Skip to content

fix(datepicker): don't render invalid ranges and clean up range display logic #19111

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions src/material/datepicker/calendar-body.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,22 +336,24 @@ describe('MatCalendarBody', () => {
expect(cells[27].classList).toContain(inRangeClass);
});

it('should be able to mark a date both as the range start and end', () => {
it('should not to mark a date as both the start and end', () => {
testComponent.startValue = 1;
testComponent.endValue = 1;
fixture.detectChanges();

expect(cells[0].classList).toContain(startClass);
expect(cells[0].classList).toContain(endClass);
expect(cells[0].classList).not.toContain(startClass);
expect(cells[0].classList).not.toContain(inRangeClass);
expect(cells[0].classList).not.toContain(endClass);
});

it('should be able to mark a date both as the comparison range start and end', () => {
it('should not mark a date as both the comparison start and end', () => {
testComponent.comparisonStart = 1;
testComponent.comparisonEnd = 1;
fixture.detectChanges();

expect(cells[0].classList).toContain(comparisonStartClass);
expect(cells[0].classList).toContain(comparisonEndClass);
expect(cells[0].classList).not.toContain(comparisonStartClass);
expect(cells[0].classList).not.toContain(inComparisonClass);
expect(cells[0].classList).not.toContain(comparisonEndClass);
});

it('should not mark a date as the range end if it comes before the start', () => {
Expand All @@ -361,7 +363,7 @@ describe('MatCalendarBody', () => {

expect(cells[0].classList).not.toContain(endClass);
expect(cells[0].classList).not.toContain(inRangeClass);
expect(cells[1].classList).toContain(startClass);
expect(cells[1].classList).not.toContain(startClass);
});

it('should not mark a date as the comparison range end if it comes before the start', () => {
Expand All @@ -371,7 +373,7 @@ describe('MatCalendarBody', () => {

expect(cells[0].classList).not.toContain(comparisonEndClass);
expect(cells[0].classList).not.toContain(inComparisonClass);
expect(cells[1].classList).toContain(comparisonStartClass);
expect(cells[1].classList).not.toContain(comparisonStartClass);
});

it('should not show a range if there is no start', () => {
Expand Down Expand Up @@ -473,7 +475,7 @@ describe('MatCalendarBody', () => {
dispatchMouseEvent(cells[2], 'mouseenter');
fixture.detectChanges();

expect(cells[5].classList).toContain(startClass);
expect(cells[5].classList).not.toContain(startClass);
expect(cells[5].classList).not.toContain(previewStartClass);
expect(cells.some(cell => cell.classList.contains(inPreviewClass))).toBe(false);
});
Expand Down
47 changes: 28 additions & 19 deletions src/material/datepicker/calendar-body.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,23 +215,22 @@ export class MatCalendarBody implements OnChanges, OnDestroy {

/** Gets whether a value is the start of the main range. */
_isRangeStart(value: number) {
return value === this.startValue;
return isStart(value, this.startValue, this.endValue);
}

/** Gets whether a value is the end of the main range. */
_isRangeEnd(value: number) {
return this.startValue && value >= this.startValue && value === this.endValue;
return isEnd(value, this.startValue, this.endValue);
}

/** Gets whether a value is within the currently-selected range. */
_isInRange(value: number): boolean {
return this.isRange && this.startValue !== null && this.endValue !== null &&
value >= this.startValue && value <= this.endValue;
return isInRange(value, this.startValue, this.endValue, this.isRange);
}

/** Gets whether a value is the start of the comparison range. */
_isComparisonStart(value: number) {
return value === this.comparisonStart;
return isStart(value, this.comparisonStart, this.comparisonEnd);
}

/** Whether the cell is a start bridge cell between the main and comparison ranges. */
Expand Down Expand Up @@ -268,36 +267,27 @@ export class MatCalendarBody implements OnChanges, OnDestroy {

/** Gets whether a value is the end of the comparison range. */
_isComparisonEnd(value: number) {
return this.comparisonStart && value >= this.comparisonStart &&
value === this.comparisonEnd;
return isEnd(value, this.comparisonStart, this.comparisonEnd);
}

/** Gets whether a value is within the current comparison range. */
_isInComparisonRange(value: number) {
return this.comparisonStart && this.comparisonEnd &&
value >= this.comparisonStart &&
value <= this.comparisonEnd;
return isInRange(value, this.comparisonStart, this.comparisonEnd, this.isRange);
}

/** Gets whether a value is the start of the preview range. */
_isPreviewStart(value: number) {
return value === this.previewStart && this.previewEnd && value < this.previewEnd;
return isStart(value, this.previewStart, this.previewEnd);
}

/** Gets whether a value is the end of the preview range. */
_isPreviewEnd(value: number) {
return value === this.previewEnd && this.previewStart && value > this.previewStart;
return isEnd(value, this.previewStart, this.previewEnd);
}

/** Gets whether a value is inside the preview range. */
_isInPreview(value: number) {
if (!this.isRange) {
return false;
}

const {previewStart, previewEnd} = this;
return previewStart !== null && previewEnd !== null && previewStart !== previewEnd &&
value >= previewStart && value <= previewEnd;
return isInRange(value, this.previewStart, this.previewEnd, this.isRange);
}

/**
Expand Down Expand Up @@ -372,3 +362,22 @@ export class MatCalendarBody implements OnChanges, OnDestroy {
function isTableCell(node: Node): node is HTMLTableCellElement {
return node.nodeName === 'TD';
}

/** Checks whether a value is the start of a range. */
function isStart(value: number, start: number | null, end: number | null): boolean {
return end !== null && start !== end && value < end && value === start;
}

/** Checks whether a value is the end of a range. */
function isEnd(value: number, start: number | null, end: number | null): boolean {
return start !== null && start !== end && value >= start && value === end;
}

/** Checks whether a value is inside of a range. */
function isInRange(value: number,
start: number | null,
end: number | null,
rangeEnabled: boolean): boolean {
return rangeEnabled && start !== null && end !== null && start !== end &&
value >= start && value <= end;
}
6 changes: 3 additions & 3 deletions src/material/datepicker/calendar.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<mat-month-view
*ngSwitchCase="'month'"
[(activeDate)]="activeDate"
[selected]="selected"
[selected]="_getDisplaySelection()"
[dateFilter]="dateFilter"
[maxDate]="maxDate"
[minDate]="minDate"
Expand All @@ -17,7 +17,7 @@
<mat-year-view
*ngSwitchCase="'year'"
[(activeDate)]="activeDate"
[selected]="selected"
[selected]="_getDisplaySelection()"
[dateFilter]="dateFilter"
[maxDate]="maxDate"
[minDate]="minDate"
Expand All @@ -28,7 +28,7 @@
<mat-multi-year-view
*ngSwitchCase="'multi-year'"
[(activeDate)]="activeDate"
[selected]="selected"
[selected]="_getDisplaySelection()"
[dateFilter]="dateFilter"
[maxDate]="maxDate"
[minDate]="minDate"
Expand Down
5 changes: 5 additions & 0 deletions src/material/datepicker/calendar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,11 @@ export class MatCalendar<D> implements AfterContentInit, AfterViewChecked, OnDes
this.currentView = view;
}

/** Gets the selection that should be displayed to the user. */
_getDisplaySelection(): DateRange<D> | D | null {
return this._model.isValid() ? this._model.selection : null;
}

/**
* @param obj The object to check.
* @returns The given object if it is both a date instance and valid, otherwise null.
Expand Down
56 changes: 45 additions & 11 deletions src/material/datepicker/date-selection-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import {FactoryProvider, Injectable, Optional, SkipSelf, OnDestroy} from '@angular/core';
import {DateAdapter} from '@angular/material/core';
import {Observable, Subject} from 'rxjs';

/** A class representing a range of dates. */
Expand Down Expand Up @@ -50,7 +51,8 @@ export abstract class MatDateSelectionModel<S, D = ExtractDateTypeFromSelection<

protected constructor(
/** The current selection. */
readonly selection: S) {
readonly selection: S,
protected _adapter: DateAdapter<D>) {
this.selection = selection;
}

Expand All @@ -68,18 +70,25 @@ export abstract class MatDateSelectionModel<S, D = ExtractDateTypeFromSelection<
this._selectionChanged.complete();
}

protected _isValidDateInstance(date: D): boolean {
return this._adapter.isDateInstance(date) && this._adapter.isValid(date);
}

/** Adds a date to the current selection. */
abstract add(date: D | null): void;

/** Checks whether the current selection is valid. */
abstract isValid(): boolean;

/** Checks whether the current selection is complete. */
abstract isComplete(): boolean;
}

/** A selection model that contains a single date. */
@Injectable()
export class MatSingleDateSelectionModel<D> extends MatDateSelectionModel<D | null, D> {
constructor() {
super(null);
constructor(adapter: DateAdapter<D>) {
super(null, adapter);
}

/**
Expand All @@ -90,6 +99,11 @@ export class MatSingleDateSelectionModel<D> extends MatDateSelectionModel<D | nu
super.updateSelection(date, this);
}

/** Checks whether the current selection is valid. */
isValid(): boolean {
return this.selection != null && this._isValidDateInstance(this.selection);
}

/**
* Checks whether the current selection is complete. In the case of a single date selection, this
* is true if the current selection is not null.
Expand All @@ -102,8 +116,8 @@ export class MatSingleDateSelectionModel<D> extends MatDateSelectionModel<D | nu
/** A selection model that contains a date range. */
@Injectable()
export class MatRangeDateSelectionModel<D> extends MatDateSelectionModel<DateRange<D>, D> {
constructor() {
super(new DateRange<D>(null, null));
constructor(adapter: DateAdapter<D>) {
super(new DateRange<D>(null, null), adapter);
}

/**
Expand All @@ -126,6 +140,26 @@ export class MatRangeDateSelectionModel<D> extends MatDateSelectionModel<DateRan
super.updateSelection(new DateRange<D>(start, end), this);
}

/** Checks whether the current selection is valid. */
isValid(): boolean {
const {start, end} = this.selection;

// Empty ranges are valid.
if (start == null && end == null) {
return true;
}

// Complete ranges are only valid if both dates are valid and the start is before the end.
if (start != null && end != null) {
return this._isValidDateInstance(start) && this._isValidDateInstance(end) &&
this._adapter.compareDate(start, end) <= 0;
}

// Partial ranges are valid if the start/end is valid.
return (start == null || this._isValidDateInstance(start)) &&
(end == null || this._isValidDateInstance(end));
}

/**
* Checks whether the current selection is complete. In the case of a date range selection, this
* is true if the current selection has a non-null `start` and `end`.
Expand All @@ -137,27 +171,27 @@ export class MatRangeDateSelectionModel<D> extends MatDateSelectionModel<DateRan

/** @docs-private */
export function MAT_SINGLE_DATE_SELECTION_MODEL_FACTORY(
parent: MatSingleDateSelectionModel<unknown>) {
return parent || new MatSingleDateSelectionModel();
parent: MatSingleDateSelectionModel<unknown>, adapter: DateAdapter<unknown>) {
return parent || new MatSingleDateSelectionModel(adapter);
}

/** Used to provide a single selection model to a component. */
export const MAT_SINGLE_DATE_SELECTION_MODEL_PROVIDER: FactoryProvider = {
provide: MatDateSelectionModel,
deps: [[new Optional(), new SkipSelf(), MatDateSelectionModel]],
deps: [[new Optional(), new SkipSelf(), MatDateSelectionModel], DateAdapter],
useFactory: MAT_SINGLE_DATE_SELECTION_MODEL_FACTORY,
};


/** @docs-private */
export function MAT_RANGE_DATE_SELECTION_MODEL_FACTORY(
parent: MatSingleDateSelectionModel<unknown>) {
return parent || new MatRangeDateSelectionModel();
parent: MatSingleDateSelectionModel<unknown>, adapter: DateAdapter<unknown>) {
return parent || new MatRangeDateSelectionModel(adapter);
}

/** Used to provide a range selection model to a component. */
export const MAT_RANGE_DATE_SELECTION_MODEL_PROVIDER: FactoryProvider = {
provide: MatDateSelectionModel,
deps: [[new Optional(), new SkipSelf(), MatDateSelectionModel]],
deps: [[new Optional(), new SkipSelf(), MatDateSelectionModel], DateAdapter],
useFactory: MAT_RANGE_DATE_SELECTION_MODEL_FACTORY,
};
Loading