Skip to content

Commit

Permalink
refactor(datepicker): improve code readability
Browse files Browse the repository at this point in the history
- update public comments
- use `calendar.getPrev` instead of `calendar.getNext` where possible
- rename `state.focusDate` to `state.focusedDate`
  • Loading branch information
maxokorokov committed Nov 15, 2019
1 parent e284110 commit b35f759
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 45 deletions.
@@ -1,10 +1,5 @@
import {Component, Injectable} from '@angular/core';
import {
NgbCalendar,
NgbDatepicker,
NgbDatepickerKeyboardService,
NgbDateStruct
} from '@ng-bootstrap/ng-bootstrap';
import {NgbCalendar, NgbDatepicker, NgbDatepickerKeyboardService, NgbDateStruct} from '@ng-bootstrap/ng-bootstrap';

const Key = {
PageUp: 'PageUp',
Expand All @@ -19,10 +14,10 @@ export class CustomKeyboardService extends NgbDatepickerKeyboardService {
const state = dp.state;
switch (event.code) {
case Key.PageUp:
dp.focusDate(calendar.getNext(state.focusDate, event.altKey ? 'y' : 'm', -1));
dp.focusDate(calendar.getPrev(state.focusedDate, event.altKey ? 'y' : 'm'));
break;
case Key.PageDown:
dp.focusDate(calendar.getNext(state.focusDate, event.altKey ? 'y' : 'm', 1));
dp.focusDate(calendar.getNext(state.focusedDate, event.altKey ? 'y' : 'm'));
break;
case Key.End:
dp.focusDate(event.altKey ? state.maxDate : state.lastDate);
Expand Down
12 changes: 6 additions & 6 deletions src/datepicker/datepicker-integration.spec.ts
Expand Up @@ -121,10 +121,10 @@ describe('ngb-datepicker integration', () => {
// tslint:disable-next-line:deprecation
switch (event.which) {
case Key.PageUp:
service.focusDate(calendar.getNext(state.focusDate, event.altKey ? 'y' : 'm', -1));
service.focusDate(calendar.getPrev(state.focusedDate, event.altKey ? 'y' : 'm', 1));
break;
case Key.PageDown:
service.focusDate(calendar.getNext(state.focusDate, event.altKey ? 'y' : 'm', 1));
service.focusDate(calendar.getNext(state.focusedDate, event.altKey ? 'y' : 'm', 1));
break;
default:
super.processKey(event, service, calendar);
Expand Down Expand Up @@ -155,19 +155,19 @@ describe('ngb-datepicker integration', () => {
dp = fixture.debugElement.query(By.css('ngb-datepicker')).injector.get(NgbDatepicker);
ngbCalendar = fixture.debugElement.query(By.css('ngb-datepicker')).injector.get(NgbCalendar as Type<NgbCalendar>);

spyOn(ngbCalendar, 'getNext');
spyOn(ngbCalendar, 'getPrev');
});

it('should allow customize keyboard navigation', () => {
dp.onKeyDown(<any>{which: Key.PageUp, altKey: true, preventDefault: () => {}, stopPropagation: () => {}});
expect(ngbCalendar.getNext).toHaveBeenCalledWith(startDate, 'y', -1);
expect(ngbCalendar.getPrev).toHaveBeenCalledWith(startDate, 'y', 1);
dp.onKeyDown(<any>{which: Key.PageUp, shiftKey: true, preventDefault: () => {}, stopPropagation: () => {}});
expect(ngbCalendar.getNext).toHaveBeenCalledWith(startDate, 'm', -1);
expect(ngbCalendar.getPrev).toHaveBeenCalledWith(startDate, 'm', 1);
});

it('should allow access to default keyboard navigation', () => {
dp.onKeyDown(<any>{which: Key.ArrowUp, altKey: true, preventDefault: () => {}, stopPropagation: () => {}});
expect(ngbCalendar.getNext).toHaveBeenCalledWith(startDate, 'd', -7);
expect(ngbCalendar.getPrev).toHaveBeenCalledWith(startDate, 'd', 7);
});
});
});
Expand Down
25 changes: 14 additions & 11 deletions src/datepicker/datepicker-keyboard-service.spec.ts
Expand Up @@ -15,7 +15,7 @@ describe('ngb-datepicker-keyboard-service', () => {
let calendar: NgbCalendar;
let mock: Partial<NgbDatepicker>;
let processKey = function(e: KeyboardEvent) { service.processKey(e, mock as NgbDatepicker, calendar); };
let state: NgbDatepickerState = Object.assign({focusDate: {day: 1, month: 1, year: 2018}});
let state: NgbDatepickerState = Object.assign({focusedDate: {day: 1, month: 1, year: 2018}});

beforeEach(() => {
TestBed.configureTestingModule(
Expand All @@ -28,40 +28,43 @@ describe('ngb-datepicker-keyboard-service', () => {
spyOn(mock, 'focusDate');
spyOn(mock, 'focusSelect');
spyOn(calendar, 'getNext');
spyOn(calendar, 'getPrev');
});

it('should be instantiated', () => { expect(service).toBeTruthy(); });

it('should move focus by 1 day or 1 week with "Arrow" keys', () => {
processKey(event(Key.ArrowUp));
expect(calendar.getNext).toHaveBeenCalledWith(state.focusDate, 'd', -7);
expect(calendar.getPrev).toHaveBeenCalledWith(state.focusedDate, 'd', 7);

processKey(event(Key.ArrowDown));
expect(calendar.getNext).toHaveBeenCalledWith(state.focusDate, 'd', 7);
expect(calendar.getNext).toHaveBeenCalledWith(state.focusedDate, 'd', 7);

processKey(event(Key.ArrowLeft));
expect(calendar.getNext).toHaveBeenCalledWith(state.focusDate, 'd', -1);
expect(calendar.getPrev).toHaveBeenCalledWith(state.focusedDate, 'd', 1);

processKey(event(Key.ArrowRight));
expect(calendar.getNext).toHaveBeenCalledWith(state.focusDate, 'd', 1);
expect(calendar.getNext).toHaveBeenCalledWith(state.focusedDate, 'd', 1);

expect(calendar.getNext).toHaveBeenCalledTimes(4);
expect(calendar.getPrev).toHaveBeenCalledTimes(2);
expect(calendar.getNext).toHaveBeenCalledTimes(2);
});

it('should move focus by 1 month or year "PgUp" and "PageDown"', () => {
processKey(event(Key.PageUp));
expect(calendar.getNext).toHaveBeenCalledWith(state.focusDate, 'm', -1);
expect(calendar.getPrev).toHaveBeenCalledWith(state.focusedDate, 'm', 1);

processKey(event(Key.PageDown));
expect(calendar.getNext).toHaveBeenCalledWith(state.focusDate, 'm', 1);
expect(calendar.getNext).toHaveBeenCalledWith(state.focusedDate, 'm', 1);

processKey(event(Key.PageUp, true));
expect(calendar.getNext).toHaveBeenCalledWith(state.focusDate, 'y', -1);
expect(calendar.getPrev).toHaveBeenCalledWith(state.focusedDate, 'y', 1);

processKey(event(Key.PageDown, true));
expect(calendar.getNext).toHaveBeenCalledWith(state.focusDate, 'y', 1);
expect(calendar.getNext).toHaveBeenCalledWith(state.focusedDate, 'y', 1);

expect(calendar.getNext).toHaveBeenCalledTimes(4);
expect(calendar.getPrev).toHaveBeenCalledTimes(2);
expect(calendar.getNext).toHaveBeenCalledTimes(2);
});

it('should select focused date with "Space" and "Enter"', () => {
Expand Down
14 changes: 7 additions & 7 deletions src/datepicker/datepicker-keyboard-service.ts
Expand Up @@ -6,7 +6,7 @@ import {Key} from '../util/key';
/**
* A service that represents the keyboard navigation.
*
* The default navigation is documented in the overview.
* Default keyboard shortcuts [are documented in the overview](#/components/datepicker/overview#keyboard-shortcuts)
*/
@Injectable({providedIn: 'root'})
export class NgbDatepickerKeyboardService {
Expand All @@ -18,10 +18,10 @@ export class NgbDatepickerKeyboardService {
// tslint:disable-next-line:deprecation
switch (event.which) {
case Key.PageUp:
datepicker.focusDate(calendar.getNext(state.focusDate, event.shiftKey ? 'y' : 'm', -1));
datepicker.focusDate(calendar.getPrev(state.focusedDate, event.shiftKey ? 'y' : 'm', 1));
break;
case Key.PageDown:
datepicker.focusDate(calendar.getNext(state.focusDate, event.shiftKey ? 'y' : 'm', 1));
datepicker.focusDate(calendar.getNext(state.focusedDate, event.shiftKey ? 'y' : 'm', 1));
break;
case Key.End:
datepicker.focusDate(event.shiftKey ? state.maxDate : state.lastDate);
Expand All @@ -30,16 +30,16 @@ export class NgbDatepickerKeyboardService {
datepicker.focusDate(event.shiftKey ? state.minDate : state.firstDate);
break;
case Key.ArrowLeft:
datepicker.focusDate(calendar.getNext(state.focusDate, 'd', -1));
datepicker.focusDate(calendar.getPrev(state.focusedDate, 'd', 1));
break;
case Key.ArrowUp:
datepicker.focusDate(calendar.getNext(state.focusDate, 'd', -calendar.getDaysPerWeek()));
datepicker.focusDate(calendar.getPrev(state.focusedDate, 'd', calendar.getDaysPerWeek()));
break;
case Key.ArrowRight:
datepicker.focusDate(calendar.getNext(state.focusDate, 'd', 1));
datepicker.focusDate(calendar.getNext(state.focusedDate, 'd', 1));
break;
case Key.ArrowDown:
datepicker.focusDate(calendar.getNext(state.focusDate, 'd', calendar.getDaysPerWeek()));
datepicker.focusDate(calendar.getNext(state.focusedDate, 'd', calendar.getDaysPerWeek()));
break;
case Key.Enter:
case Key.Space:
Expand Down
8 changes: 4 additions & 4 deletions src/datepicker/datepicker.spec.ts
Expand Up @@ -1265,12 +1265,12 @@ describe('ngb-datepicker', () => {
expect(dp.model.lastDate).toEqual(NgbDate.from({year: 2016, month: 8, day: 31}));
});

it('should provide an defensive copy of focusDate', () => {
it('should provide an defensive copy of focusedDate', () => {
dp.onKeyDown(<KeyboardEvent>{});
expect(mockState.focusDate).toEqual(NgbDate.from({year: 2016, month: 8, day: 1}));
Object.assign(mockState, {focusDate: undefined});
expect(mockState.focusedDate).toEqual(NgbDate.from({year: 2016, month: 8, day: 1}));
Object.assign(mockState, {focusedDate: undefined});
dp.onKeyDown(<KeyboardEvent>{});
expect(mockState.focusDate).toEqual(NgbDate.from({year: 2016, month: 8, day: 1}));
expect(dp.model.focusDate).toEqual(NgbDate.from({year: 2016, month: 8, day: 1}));
});
});
});
Expand Down
19 changes: 10 additions & 9 deletions src/datepicker/datepicker.ts
Expand Up @@ -62,34 +62,35 @@ export interface NgbDatepickerNavigateEvent {
}

/**
* An object that represents a part of the state of the datepicker.
* Required to override datepicker services i.e. the datepicker-keyboard-service.
* An interface that represents the readonly public state of the datepicker.
*
* Accessible via the `datepicker.state` getter
*/
export interface NgbDatepickerState {
/**
* The minDate provided as input.
* The earliest date that can be displayed or selected
*/
readonly minDate: NgbDate;

/**
* The maxDate provided as input.
* The latest date that can be displayed or selected
*/
readonly maxDate: NgbDate;

/**
* The first date of current month.
* The first visible date of currently displayed months
*/
readonly firstDate: NgbDate;

/**
* The last date of current month.
* The last visible date of currently displayed months
*/
readonly lastDate: NgbDate;

/**
* The focused date.
* The date currently focused by the datepicker
*/
readonly focusDate?: NgbDate;
readonly focusedDate: NgbDate;
}

/**
Expand Down Expand Up @@ -340,7 +341,7 @@ export class NgbDatepicker implements OnDestroy,
}

/**
* Returns a copy of the state of the datepicker.
* Returns the readonly public state of the datepicker
*/
get state(): NgbDatepickerState { return this._publicState; }

Expand Down

0 comments on commit b35f759

Please sign in to comment.