feat(datepicker): wire up selected value propagation #3330
Conversation
googlebot
added
the
cla: yes
| private _value: SimpleDate; | ||
|
|
||
| // Implemented as part of ControlValueAccessor | ||
| writeValue(value: SimpleDate) { |
There was a problem hiding this comment.
I guess... I don't see much value in the : void
| } | ||
| set value(value: any) { | ||
| this._value = this._locale.parseDate(value); | ||
| let stringValue = this._value == null ? '' : this._locale.formatDate(this._value); |
src/lib/datepicker/index.ts
Outdated
| ], | ||
| exports: [ | ||
| MdCalendar, | ||
| MdCalendarTable, |
There was a problem hiding this comment.
Do you intend for the CalendarTable, MonthView, etc. to be used externally? Seems like you can remove them from the exports list.
|
addressed comments, PTAL |
|
|
||
| ngAfterContentInit() { | ||
| if (this._datepicker) { | ||
| this._datepicker.selectedChanged.subscribe((selected: SimpleDate) => { |
There was a problem hiding this comment.
Do you unsubscribe from this anywhere?
|
|
||
| @Component({ | ||
| template: ` | ||
| <input [mdDatepicker]="d" [value]="'1/1/2020'"> |
There was a problem hiding this comment.
Nit: might be easier to scan if it's just value="1/1/2020"
|
|
||
| expect(testComponent.selected).toEqual(selected); | ||
| expect(testComponent.datepickerInput.value).toEqual(selected); | ||
| })); |
There was a problem hiding this comment.
We should add tests that ensure that touched, disabled, and dirty still work, given that we're re-implementing them in the value accessor
|
comments addressed |
| testComponent.datepicker.selected = new SimpleDate(2017, 0, 1); | ||
| detectModelChanges(fixture); | ||
|
|
||
| expect(inputEl.classList).toContain('ng-dirty'); |
There was a problem hiding this comment.
It should actually not mark the control dirty based on programmatic changes. Dirtiness is a reflection of UI changes only (what the user does). We're probably calling the change callback somewhere we shouldn't be.
There was a problem hiding this comment.
This property isn't really intended to be set programmatically, it's set when the user clicks a calendar cell. Made it internal so people don't try to call it, though...
kara
added
pr: lgtm
action: merge
* enable value propagation through various components & directives * add tests * fix lint * addressed comments * addressed feedback * fix lint * make datepicker selected property internal * add test for pristine after ngModel change
* enable value propagation through various components & directives * add tests * fix lint * addressed comments * addressed feedback * fix lint * make datepicker selected property internal * add test for pristine after ngModel change
* enable value propagation through various components & directives * add tests * fix lint * addressed comments * addressed feedback * fix lint * make datepicker selected property internal * add test for pristine after ngModel change
* enable value propagation through various components & directives * add tests * fix lint * addressed comments * addressed feedback * fix lint * make datepicker selected property internal * add test for pristine after ngModel change
* enable value propagation through various components & directives * add tests * fix lint * addressed comments * addressed feedback * fix lint * make datepicker selected property internal * add test for pristine after ngModel change
* enable value propagation through various components & directives * add tests * fix lint * addressed comments * addressed feedback * fix lint * make datepicker selected property internal * add test for pristine after ngModel change
* enable value propagation through various components & directives * add tests * fix lint * addressed comments * addressed feedback * fix lint * make datepicker selected property internal * add test for pristine after ngModel change
* enable value propagation through various components & directives * add tests * fix lint * addressed comments * addressed feedback * fix lint * make datepicker selected property internal * add test for pristine after ngModel change
|
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. |
as of this PR we have a bare minimum working datepicker \o/