Skip to content

Commit

Permalink
fix(datepicker): ExpressionChangedAfterItHasBeenCheckedError when swi…
Browse files Browse the repository at this point in the history 
…tching between months

Reuses data structures in the datepicker data model instead of building
new objects each time the user navigates to a different month.
As a consequence, DOM elements are no longer destroyed when changing month.

This fixes the following issue which occurred when BrowserAnimationsModule was
used: when using the keyboard to navigate from one month to another, the
focused element was destroyed during the change detection of the keyboard
event, which synchronously triggered the focusout event in Chrome, leading
to a change in the data model (and that change is still part of the data
model only when BrowserAnimationsModule is used, because that module changes
the behavior of Angular to not destroy views and their data model as quickly
as when this module is not loaded), and this was causing the
ExpressionChangedAfterItHasBeenCheckedError error.

Fixes #2408
Closes #2462
  • Loading branch information
@divdavem @maxokorokov
divdavem authored and maxokorokov committed Jun 19, 2018
1 parent d7e3649 commit b47f981
Show file tree
Hide file tree
Showing 4 changed files with 303 additions and 131 deletions.
31 changes: 12 additions & 19 deletions src/datepicker/datepicker-service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,29 +158,22 @@ describe('ngb-datepicker-service', () => {
expect(getDayCtx(5).disabled).toBe(true); // 6 MAY
});

it(`should rebuild month when 'min/maxDates' change and visible`, () => {
it(`should update month when 'min/maxDates' change and visible`, () => {
// MAY 2017
service.focus(new NgbDate(2017, 5, 5));
expect(model.months.length).toBe(1);
expect(model.minDate).toBeUndefined();
expect(model.maxDate).toBeUndefined();

const month = model.months[0];
const date = month.weeks[0].days[0].date;

// MIN -> 5 MAY, 2017
service.minDate = new NgbDate(2017, 5, 5);
expect(model.months.length).toBe(1);
expect(model.months[0]).not.toBe(month);
expect(getDayCtx(0).disabled).toBe(true);
expect(getDay(0).date).not.toBe(date);

// MAX -> 10 MAY, 2017
service.maxDate = new NgbDate(2017, 5, 10);
expect(model.months.length).toBe(1);
expect(model.months[0]).not.toBe(month);
expect(model.months[0].weeks[4].days[0].context.disabled).toBe(true);
expect(model.months[0].weeks[0].days[0].date).not.toBe(date);
});
});

Expand Down Expand Up @@ -233,18 +226,19 @@ describe('ngb-datepicker-service', () => {
expect(model.months[0].weekdays[0]).toBe(4);
});

it(`should rebuild months when 'firstDayOfWeek' changes`, () => {
it(`should update months when 'firstDayOfWeek' changes`, () => {
service.focus(new NgbDate(2017, 5, 5));
expect(model.months.length).toBe(1);
expect(model.firstDayOfWeek).toBe(1);

const month = model.months[0];
const date = month.weeks[0].days[0].date;
const oldFirstDate = getDay(0).date.toString();
expect(oldFirstDate).toBe('2017-5-1');

service.firstDayOfWeek = 3;
expect(model.months.length).toBe(1);
expect(model.months[0]).not.toBe(month);
expect(getDay(0).date).not.toBe(date);
expect(model.firstDayOfWeek).toBe(3);
const newFirstDate = getDay(0).date.toString();
expect(newFirstDate).toBe('2017-4-26');
});
});

Expand Down Expand Up @@ -905,17 +899,16 @@ describe('ngb-datepicker-service', () => {
expect(day.context.disabled).toBe(true);
});

it(`should rebuild months when 'markDisabled changes'`, () => {
it(`should update months when 'markDisabled changes'`, () => {
// MAY 2017
service.markDisabled = (_) => true;
service.focus(new NgbDate(2017, 5, 1));

const month = model.months[0];
const date = month.weeks[0].days[0].date;
expect(getDay(0).context.disabled).toBe(true);

service.markDisabled = (_) => true;
expect(model.months[0]).not.toBe(month);
expect(getDay(0).date).not.toBe(date);
service.markDisabled = (_) => false;

expect(getDay(0).context.disabled).toBe(false);
});
});

Expand Down
271 changes: 189 additions & 82 deletions src/datepicker/datepicker-tools.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
import {NgbDate} from './ngb-date';
import {NgbCalendar, NgbCalendarGregorian} from './ngb-calendar';
import {TestBed} from '@angular/core/testing';
import {DatepickerViewModel, NgbMarkDisabled} from './datepicker-view-model';
import {DatepickerViewModel, NgbMarkDisabled, MonthViewModel} from './datepicker-view-model';
import {NgbDatepickerI18n, NgbDatepickerI18nDefault} from './datepicker-i18n';
import {DatePipe} from '@angular/common';

Expand Down Expand Up @@ -248,92 +248,199 @@ describe(`datepicker-tools`, () => {
expect(months.length).toBe(2);
});

it(`should not rebuild existing months by default`, () => {
const may = new NgbDate(2017, 5, 5);
const june = new NgbDate(2017, 6, 5);
const storeMonthsDataStructure = (months: MonthViewModel[]) => {
return months.map(month => {
const storage = {weeks: month.weeks, weekdays: month.weekdays};
const weeks = month.weeks;
for (let weekIndex = 0, weeksLength = weeks.length; weekIndex < weeksLength; weekIndex++) {
const currentWeek = weeks[weekIndex];
storage[`weeks[${weekIndex}]`] = currentWeek;
const days = currentWeek.days;
storage[`weeks[${weekIndex}].days`] = days;
for (let dayIndex = 0, daysLength = days.length; dayIndex < daysLength; dayIndex++) {
const currentDay = days[dayIndex];
storage[`weeks[${weekIndex}].days[${dayIndex}]`] = currentDay;
}
}
return storage;
});
};

const customMatchers: jasmine.CustomMatcherFactories = {
toHaveTheSameMonthDataStructureAs: function(util, customEqualityTesters) {
return {
compare(actualMonthsStorage, expectedMonthsStorage) {
try {
const monthsNumber = actualMonthsStorage.length;
if (expectedMonthsStorage.length !== monthsNumber) {
throw 'the number of months';
};
for (let i = 0; i < monthsNumber; i++) {
const storage1 = actualMonthsStorage[i];
const storage2 = expectedMonthsStorage[i];
const keys1 = Object.keys(storage1);
const keys2 = Object.keys(storage2);
if (!util.equals(keys2, keys1, customEqualityTesters)) {
throw `the set of keys in months[${i}]: ${keys1} != ${keys2}`;
}
for (const key of keys1) {
if (storage1[key] !== storage2[key]) {
throw `months[${i}].${key}`;
}
}
}
return {
pass: true, message: 'Expected different months data structures, but the same data structure was found.'
}
} catch (e) {
return {
pass: false,
message: typeof e === 'string' ?
`Expected the same months data structure, but a difference was found in ${e}` :
`${e}`
};
}
}
};
}
};

beforeEach(function() { jasmine.addMatchers(customMatchers); });

// one same month
it(`should reuse the same data structure (force = false)`, () => {
let state = { displayMonths: 1, firstDayOfWeek: 1, months: [] } as DatepickerViewModel;
state.months = buildMonths(calendar, may, state, i18n, false);
let newMonths = buildMonths(calendar, may, state, i18n, false);

expect(state.months.length).toBe(1);
expect(newMonths.length).toBe(1);
expect(state.months[0]).toBe(newMonths[0]);

// one new month
state = { displayMonths: 1, firstDayOfWeek: 1, months: [] } as DatepickerViewModel;
state.months = buildMonths(calendar, may, state, i18n, false);
newMonths = buildMonths(calendar, june, state, i18n, false);

expect(state.months.length).toBe(1);
expect(newMonths.length).toBe(1);
expect(state.months[0]).not.toBe(newMonths[0]);

// two same months
state = { displayMonths: 2, firstDayOfWeek: 1, months: [] } as DatepickerViewModel;
state.months = buildMonths(calendar, may, state, i18n, false);
newMonths = buildMonths(calendar, may, state, i18n, false);

expect(state.months.length).toBe(2);
expect(newMonths.length).toBe(2);
expect(state.months[0]).toBe(newMonths[0]);
expect(state.months[1]).toBe(newMonths[1]);

// two months, one overlaps
state = { displayMonths: 2, firstDayOfWeek: 1, months: [] } as DatepickerViewModel;
state.months = buildMonths(calendar, may, state, i18n, false);
newMonths = buildMonths(calendar, june, state, i18n, false);

expect(state.months.length).toBe(2);
expect(newMonths.length).toBe(2);
expect(state.months[0]).not.toBe(newMonths[0]);
expect(state.months[1]).not.toBe(newMonths[1]);
expect(state.months[1]).toBe(newMonths[0]); // june reused
});
let months = buildMonths(calendar, new NgbDate(2017, 5, 5), state, i18n, false);
expect(months).toBe(state.months);
expect(months.length).toBe(1);
let monthsStructure = storeMonthsDataStructure(months);

months = buildMonths(calendar, new NgbDate(2018, 5, 5), state, i18n, false);
expect(months).toBe(state.months);
expect(months.length).toBe(1);
expect(storeMonthsDataStructure(months))['toHaveTheSameMonthDataStructureAs'](monthsStructure);

state.displayMonths = 2;
months = buildMonths(calendar, new NgbDate(2018, 5, 5), state, i18n, false);
expect(months).toBe(state.months);
expect(months.length).toBe(2);
monthsStructure.push(...storeMonthsDataStructure([months[1]]));
expect(storeMonthsDataStructure(months))['toHaveTheSameMonthDataStructureAs'](monthsStructure);

// next month
months = buildMonths(calendar, new NgbDate(2018, 6, 5), state, i18n, false);
expect(months).toBe(state.months);
expect(months.length).toBe(2);
// the structures should be swapped:
monthsStructure.push(monthsStructure.shift());
expect(storeMonthsDataStructure(months))['toHaveTheSameMonthDataStructureAs'](monthsStructure);

it(`should rebuild existing months with 'rebuild=false'`, () => {
const may = new NgbDate(2017, 5, 5);
const june = new NgbDate(2017, 6, 5);
// previous month
months = buildMonths(calendar, new NgbDate(2018, 5, 5), state, i18n, false);
expect(months).toBe(state.months);
expect(months.length).toBe(2);
// the structures should be swapped (again):
monthsStructure.push(monthsStructure.shift());
expect(storeMonthsDataStructure(months))['toHaveTheSameMonthDataStructureAs'](monthsStructure);

state.displayMonths = 5;
months = buildMonths(calendar, new NgbDate(2018, 5, 5), state, i18n, false);
expect(months).toBe(state.months);
expect(months.length).toBe(5);
monthsStructure.push(...storeMonthsDataStructure(months.slice(2)));
expect(storeMonthsDataStructure(months))['toHaveTheSameMonthDataStructureAs'](monthsStructure);

// go to two months after, the 3 last months are reused as is
months = buildMonths(calendar, new NgbDate(2018, 7, 5), state, i18n, false);
expect(months).toBe(state.months);
expect(months.length).toBe(5);
monthsStructure.unshift(...monthsStructure.splice(2, 3));
expect(storeMonthsDataStructure(months))['toHaveTheSameMonthDataStructureAs'](monthsStructure);

// go to two months before, the 3 first months are reused as is
months = buildMonths(calendar, new NgbDate(2018, 5, 5), state, i18n, false);
expect(months).toBe(state.months);
expect(months.length).toBe(5);
monthsStructure.push(...monthsStructure.splice(0, 3));
expect(storeMonthsDataStructure(months))['toHaveTheSameMonthDataStructureAs'](monthsStructure);

// completely change the dates, nothing is shifted in monthsStructure
months = buildMonths(calendar, new NgbDate(2018, 10, 5), state, i18n, false);
expect(months).toBe(state.months);
expect(months.length).toBe(5);
expect(storeMonthsDataStructure(months))['toHaveTheSameMonthDataStructureAs'](monthsStructure);

// keep 2 months
state.displayMonths = 2;
months = buildMonths(calendar, new NgbDate(2018, 11, 5), state, i18n, false);
expect(months).toBe(state.months);
expect(months.length).toBe(2);
monthsStructure = monthsStructure.slice(1, 3);
expect(storeMonthsDataStructure(months))['toHaveTheSameMonthDataStructureAs'](monthsStructure);
});

// one same month
it(`should reuse the same data structure (force = true)`, () => {
let state = { displayMonths: 1, firstDayOfWeek: 1, months: [] } as DatepickerViewModel;
state.months = buildMonths(calendar, may, state, i18n, true);
let newMonths = buildMonths(calendar, may, state, i18n, true);

expect(state.months.length).toBe(1);
expect(newMonths.length).toBe(1);
expect(state.months[0]).not.toBe(newMonths[0]);

// one new month
state = { displayMonths: 1, firstDayOfWeek: 1, months: [] } as DatepickerViewModel;
state.months = buildMonths(calendar, may, state, i18n, true);
newMonths = buildMonths(calendar, june, state, i18n, true);

expect(state.months.length).toBe(1);
expect(newMonths.length).toBe(1);
expect(state.months[0]).not.toBe(newMonths[0]);

// two same months
state = { displayMonths: 2, firstDayOfWeek: 1, months: [] } as DatepickerViewModel;
state.months = buildMonths(calendar, may, state, i18n, true);
newMonths = buildMonths(calendar, may, state, i18n, true);

expect(state.months.length).toBe(2);
expect(newMonths.length).toBe(2);
expect(state.months[0]).not.toBe(newMonths[0]);
expect(state.months[1]).not.toBe(newMonths[1]);

// two months, one overlaps
state = { displayMonths: 2, firstDayOfWeek: 1, months: [] } as DatepickerViewModel;
state.months = buildMonths(calendar, may, state, i18n, true);
newMonths = buildMonths(calendar, june, state, i18n, true);

expect(state.months.length).toBe(2);
expect(newMonths.length).toBe(2);
expect(state.months[0]).not.toBe(newMonths[0]);
expect(state.months[1]).not.toBe(newMonths[1]);
expect(state.months[1]).not.toBe(newMonths[0]);
let months = buildMonths(calendar, new NgbDate(2017, 5, 5), state, i18n, true);
expect(months).toBe(state.months);
expect(months.length).toBe(1);
let monthsStructure = storeMonthsDataStructure(months);

months = buildMonths(calendar, new NgbDate(2018, 5, 5), state, i18n, true);
expect(months).toBe(state.months);
expect(months.length).toBe(1);
expect(storeMonthsDataStructure(months))['toHaveTheSameMonthDataStructureAs'](monthsStructure);

state.displayMonths = 2;
months = buildMonths(calendar, new NgbDate(2018, 5, 5), state, i18n, true);
expect(months).toBe(state.months);
expect(months.length).toBe(2);
monthsStructure.push(...storeMonthsDataStructure([months[1]]));
expect(storeMonthsDataStructure(months))['toHaveTheSameMonthDataStructureAs'](monthsStructure);

// next month
months = buildMonths(calendar, new NgbDate(2018, 6, 5), state, i18n, true);
expect(months).toBe(state.months);
expect(months.length).toBe(2);
expect(storeMonthsDataStructure(months))['toHaveTheSameMonthDataStructureAs'](monthsStructure);

// previous month
months = buildMonths(calendar, new NgbDate(2018, 5, 5), state, i18n, true);
expect(months).toBe(state.months);
expect(months.length).toBe(2);
expect(storeMonthsDataStructure(months))['toHaveTheSameMonthDataStructureAs'](monthsStructure);

state.displayMonths = 5;
months = buildMonths(calendar, new NgbDate(2018, 5, 5), state, i18n, true);
expect(months).toBe(state.months);
expect(months.length).toBe(5);
monthsStructure.push(...storeMonthsDataStructure(months.slice(2)));
expect(storeMonthsDataStructure(months))['toHaveTheSameMonthDataStructureAs'](monthsStructure);

// go to two months after
months = buildMonths(calendar, new NgbDate(2018, 7, 5), state, i18n, true);
expect(months).toBe(state.months);
expect(months.length).toBe(5);
expect(storeMonthsDataStructure(months))['toHaveTheSameMonthDataStructureAs'](monthsStructure);

// go to two months before
months = buildMonths(calendar, new NgbDate(2018, 5, 5), state, i18n, true);
expect(months).toBe(state.months);
expect(months.length).toBe(5);
expect(storeMonthsDataStructure(months))['toHaveTheSameMonthDataStructureAs'](monthsStructure);

// completely change the dates
months = buildMonths(calendar, new NgbDate(2018, 10, 5), state, i18n, true);
expect(months).toBe(state.months);
expect(months.length).toBe(5);
expect(storeMonthsDataStructure(months))['toHaveTheSameMonthDataStructureAs'](monthsStructure);

// keep 2 months
state.displayMonths = 2;
months = buildMonths(calendar, new NgbDate(2018, 11, 5), state, i18n, true);
expect(months).toBe(state.months);
expect(months.length).toBe(2);
monthsStructure = monthsStructure.slice(0, 2);
expect(storeMonthsDataStructure(months))['toHaveTheSameMonthDataStructureAs'](monthsStructure);
});
});

Expand Down
Loading

0 comments on commit b47f981

Please sign in to comment.