feat(datepicker): close calendar after choose the same date again #6323
Conversation
googlebot
added
the
cla: yes
|
Though this may be the easiest way to get the functionality you want, I don't think it's really correct to emit a |
|
Nice one. Thx for advice. |
| @@ -6,5 +6,6 @@ | |||
| [maxDate]="datepicker._maxDate" | |||
| [dateFilter]="datepicker._dateFilter" | |||
| [selected]="datepicker._selected" | |||
| (selectedChange)="datepicker._selectAndClose($event)"> | |||
| (selectedChange)="datepicker._selectAndClose($event)" | |||
There was a problem hiding this comment.
This _selectAndClose method and its comment should be updated since it no longer closes
src/lib/datepicker/calendar.html
Outdated
| (selectedChange)="_dateSelected($event)"> | ||
| (selectedChange)="_dateSelected($event)" | ||
| (userSelection)="_userSelected()" | ||
| > |
There was a problem hiding this comment.
Closing bracket should be on line above
| this._onTouched(); | ||
| this.dateInput.emit(new MdDatepickerInputEvent(this, this._elementRef.nativeElement)); | ||
| this.dateChange.emit(new MdDatepickerInputEvent(this, this._elementRef.nativeElement)); | ||
| }); |
There was a problem hiding this comment.
I think they actually prefer line breaks like this to stay 4 spaces
src/lib/datepicker/month-view.ts
Outdated
| @@ -67,6 +67,9 @@ export class MdMonthView<D> implements AfterContentInit { | |||
| /** Emits when a new date is selected. */ | |||
| @Output() selectedChange = new EventEmitter<D | null>(); | |||
|
|
|||
| /** Emits when date is picked */ | |||
| @Output() userSelection = new EventEmitter(); | |||
There was a problem hiding this comment.
The comment you had above seems more appropriate for this as well:
/** Emits when any date is selected. */
Nit: Add a period to both to match other comments
There was a problem hiding this comment.
Also I think it should have a <void> type?
new EventEmitter<void>();
There was a problem hiding this comment.
+1 to @willshowell's suggestions and a couple comments on the test
| @@ -190,6 +190,26 @@ describe('MdDatepicker', () => { | |||
| }); | |||
| }); | |||
|
|
|||
| it('setting twice to the same selected value should update input and close calendar', () => { | |||
There was a problem hiding this comment.
'clicking the currently selected date should close the calendar without firing selectedChanged'
| expect(testComponent.datepickerInput.value).toEqual(new Date(2020, JAN, currentDay)); | ||
|
|
||
| let cells = document.querySelectorAll('.mat-calendar-body-cell'); | ||
| dispatchMouseEvent(cells[1], 'click'); |
There was a problem hiding this comment.
verify that selectedChanged only emits the first time
src/lib/datepicker/datepicker.ts
Outdated
| @@ -224,13 +224,12 @@ export class MdDatepicker<D> implements OnDestroy { | |||
| } | |||
|
|
|||
| /** Selects the given date and closes the currently open popup or dialog. */ | |||
There was a problem hiding this comment.
Nit: can you update this comment too?
There was a problem hiding this comment.
Ohhh... sorry, i missed it
| @@ -224,7 +224,7 @@ export class MdDatepickerInput<D> implements AfterContentInit, ControlValueAcces | |||
| ngAfterContentInit() { | |||
| if (this._datepicker) { | |||
| this._datepickerSubscription = | |||
| this._datepicker.selectedChanged.subscribe((selected: D) => { | |||
| this._datepicker.selectedChanged.subscribe((selected: D) => { | |||
There was a problem hiding this comment.
Nit: this should stay the same too. They indent 2 for function blocks and 4 for line breaks
| @@ -190,6 +190,30 @@ describe('MdDatepicker', () => { | |||
| }); | |||
| }); | |||
|
|
|||
| it('clicking the currently selected date should close the calendar' + | |||
There was a problem hiding this comment.
nit: space before close of string so result doesn't read: "calendarwithout"
|
great, and we can just ignore that screenshot test failure, it's just because the blinking input caret |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Selecting already selected date didn't close datepicker. This contains functionality to close datepicker even when already selected date is picked.