Skip to content

Commit

Permalink
fix(material/datepicker): don't invoke change handler when filter is …
Browse files Browse the repository at this point in the history
…swapped out if result is the same (#20970)

Doesn't invoke the `ControlValueAccessor` change function when a new date filter
is assigned, if the result wouldn't have change the validation state.

Fixes #20967.
  • Loading branch information
crisbeto authored Nov 24, 2020
1 parent 78fc115 commit 975fbb3
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 5 deletions.
52 changes: 52 additions & 0 deletions src/material/datepicker/date-range-input.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {BACKSPACE} from '@angular/cdk/keycodes';
import {MatDateRangeInput} from './date-range-input';
import {MatDateRangePicker} from './date-range-picker';
import {MatStartDate, MatEndDate} from './date-range-input-parts';
import {Subscription} from 'rxjs';

describe('MatDateRangeInput', () => {
function createComponent<T>(
Expand Down Expand Up @@ -317,6 +318,57 @@ describe('MatDateRangeInput', () => {
expect(end.errors?.matDatepickerFilter).toBeTruthy();
});

it('should should revalidate when a new date filter function is assigned', () => {
const fixture = createComponent(StandardRangePicker);
fixture.detectChanges();
const {start, end} = fixture.componentInstance.range.controls;
const date = new Date(2020, 2, 2);
start.setValue(date);
end.setValue(date);
fixture.detectChanges();

const spy = jasmine.createSpy('change spy');
const subscription = new Subscription();
subscription.add(start.valueChanges.subscribe(spy));
subscription.add(end.valueChanges.subscribe(spy));

fixture.componentInstance.dateFilter = () => false;
fixture.detectChanges();
expect(spy).toHaveBeenCalledTimes(2);

fixture.componentInstance.dateFilter = () => true;
fixture.detectChanges();
expect(spy).toHaveBeenCalledTimes(4);

subscription.unsubscribe();
});

it('should not dispatch the change event if a new filter function with the same result ' +
'is assigned', () => {
const fixture = createComponent(StandardRangePicker);
fixture.detectChanges();
const {start, end} = fixture.componentInstance.range.controls;
const date = new Date(2020, 2, 2);
start.setValue(date);
end.setValue(date);
fixture.detectChanges();

const spy = jasmine.createSpy('change spy');
const subscription = new Subscription();
subscription.add(start.valueChanges.subscribe(spy));
subscription.add(end.valueChanges.subscribe(spy));

fixture.componentInstance.dateFilter = () => false;
fixture.detectChanges();
expect(spy).toHaveBeenCalledTimes(2);

fixture.componentInstance.dateFilter = () => false;
fixture.detectChanges();
expect(spy).toHaveBeenCalledTimes(2);

subscription.unsubscribe();
});

it('should throw if there is no start input', () => {
expect(() => {
const fixture = createComponent(RangePickerNoStart);
Expand Down
13 changes: 12 additions & 1 deletion src/material/datepicker/date-range-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,19 @@ export class MatDateRangeInput<D> implements MatFormFieldControl<DateRange<D>>,
@Input()
get dateFilter() { return this._dateFilter; }
set dateFilter(value: DateFilterFn<D>) {
const start = this._startInput;
const end = this._endInput;
const wasMatchingStart = start && start._matchesFilter(start.value);
const wasMatchingEnd = end && end._matchesFilter(start.value);
this._dateFilter = value;
this._revalidate();

if (start && start._matchesFilter(start.value) !== wasMatchingStart) {
start._validatorOnChange();
}

if (end && end._matchesFilter(end.value) !== wasMatchingEnd) {
end._validatorOnChange();
}
}
private _dateFilter: DateFilterFn<D>;

Expand Down
9 changes: 7 additions & 2 deletions src/material/datepicker/datepicker-input-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,7 @@ export abstract class MatDatepickerInputBase<S, D = ExtractDateTypeFromSelection
private _filterValidator: ValidatorFn = (control: AbstractControl): ValidationErrors | null => {
const controlValue = this._dateAdapter.getValidDateOrNull(
this._dateAdapter.deserialize(control.value));
const dateFilter = this._getDateFilter();
return !dateFilter || !controlValue || dateFilter(controlValue) ?
return !controlValue || this._matchesFilter(controlValue) ?
null : {'matDatepickerFilter': true};
}

Expand Down Expand Up @@ -392,6 +391,12 @@ export abstract class MatDatepickerInputBase<S, D = ExtractDateTypeFromSelection
return false;
}

/** Gets whether a value matches the current date filter. */
_matchesFilter(value: D | null): boolean {
const filter = this._getDateFilter();
return !filter || filter(value);
}

// Accept `any` to avoid conflicts with other directives on `<input>` that
// may accept different types.
static ngAcceptInputType_value: any;
Expand Down
6 changes: 5 additions & 1 deletion src/material/datepicker/datepicker-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,12 @@ export class MatDatepickerInput<D> extends MatDatepickerInputBase<D | null, D>
@Input('matDatepickerFilter')
get dateFilter() { return this._dateFilter; }
set dateFilter(value: DateFilterFn<D | null>) {
const wasMatchingValue = this._matchesFilter(this.value);
this._dateFilter = value;
this._validatorOnChange();

if (this._matchesFilter(this.value) !== wasMatchingValue) {
this._validatorOnChange();
}
}
private _dateFilter: DateFilterFn<D | null>;

Expand Down
42 changes: 41 additions & 1 deletion src/material/datepicker/datepicker.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1506,6 +1506,45 @@ describe('MatDatepicker', () => {
expect(cells[0].classList).toContain('mat-calendar-body-disabled');
expect(cells[1].classList).not.toContain('mat-calendar-body-disabled');
});

it('should revalidate when a new function is assigned', fakeAsync(() => {
const classList = fixture.debugElement.query(By.css('input'))!.nativeElement.classList;
testComponent.date = new Date(2017, JAN, 1);
testComponent.filter = () => true;
fixture.detectChanges();
flush();
fixture.detectChanges();

expect(classList).not.toContain('ng-invalid');

testComponent.filter = () => false;
fixture.detectChanges();
flush();
fixture.detectChanges();

expect(classList).toContain('ng-invalid');
}));

it('should not dispatch the change event if a new function with the same result is assigned',
fakeAsync(() => {
const spy = jasmine.createSpy('change spy');
const subscription = fixture.componentInstance.model.valueChanges?.subscribe(spy);
testComponent.filter = () => false;
fixture.detectChanges();
flush();
fixture.detectChanges();

expect(spy).toHaveBeenCalledTimes(1);

testComponent.filter = () => false;
fixture.detectChanges();
flush();
fixture.detectChanges();

expect(spy).toHaveBeenCalledTimes(1);
subscription?.unsubscribe();
}));

});

describe('datepicker with change and input events', () => {
Expand Down Expand Up @@ -2342,8 +2381,9 @@ class DatepickerWithMinAndMaxValidation {
})
class DatepickerWithFilterAndValidation {
@ViewChild('d') datepicker: MatDatepicker<Date>;
@ViewChild(NgModel) model: NgModel;
date: Date;
filter = (date: Date) => date.getDate() != 1;
filter = (date: Date | null) => date?.getDate() != 1;
}


Expand Down

0 comments on commit 975fbb3

Please sign in to comment.