fix(datepicker): calendar should update when input changes #3824
Conversation
googlebot
added
the
cla: yes
mmalerba
force-pushed
the
dp3
branch
2 times, most recently
from
edeb7d4
to
6320891
Compare
src/lib/datepicker/datepicker.ts
Outdated
| @@ -116,7 +111,7 @@ export class MdDatepicker implements OnDestroy { | |||
|
|
|||
| constructor(private _dialog: MdDialog, private _overlay: Overlay, | |||
| private _viewContainerRef: ViewContainerRef, private _locale: CalendarLocale, | |||
| @Optional() private _dir: Dir) {} | |||
| private _changeDetectorRef: ChangeDetectorRef, @Optional() private _dir: Dir) {} | |||
There was a problem hiding this comment.
Are you using this _changeDetectorRef anywhere? Couldn't find it.
There was a problem hiding this comment.
nope, must've been leftover from previous iteration
src/lib/datepicker/datepicker.ts
Outdated
| @@ -134,6 +139,7 @@ export class MdDatepicker implements OnDestroy { | |||
| throw new MdError('An MdDatepicker can only be associated with a single input.'); | |||
| } | |||
| this._datepickerInput = input; | |||
| this._datepickerInput.valueChange.subscribe((value: SimpleDate) => this._selected = value); | |||
There was a problem hiding this comment.
Are you unsubscribing anywhere?
| this._renderer.setElementProperty(this._elementRef.nativeElement, 'value', | ||
| date ? this._locale.formatDate(date) : ''); | ||
| if (!SimpleDate.equals(oldDate, date)) { | ||
| this._valueChangeEmitter.emit(date); |
There was a problem hiding this comment.
If you had something like this:
<input [value]="val" (valueChange)="doSomething()" [mdDatepicker]="dp">Would setting the value programmatically fire the doSomething() method? It looks like valueChange is emitted in both directions.
There was a problem hiding this comment.
Yes, isn't that what we want? Otherwise if I set the value programmatically the calendar won't be updated to reflect the change in the input
There was a problem hiding this comment.
Yeah, I see how it's solving that problem. Just concerned that users will be confused because typically when you have a model to view change, you don't expect the view to model handler to fire as well.
There was a problem hiding this comment.
Yeah I admit its a little weird and I had to add that equals check to stop it from infinite looping, maybe we can talk about if there's a better way
src/lib/datepicker/datepicker.ts
Outdated
| @@ -123,6 +121,19 @@ export class MdDatepicker implements OnDestroy { | |||
| if (this._popupRef) { | |||
| this._popupRef.dispose(); | |||
| } | |||
| if (this._inputSubscription) { | |||
|
|
|||
There was a problem hiding this comment.
Did you mean to unsubscribe here?
There was a problem hiding this comment.
sorry... i'm an idiot ><
* fix(datepicker): calendar selected date should change when input changes * startAt fix * fix tests * address comments * fix unsubscribe * make valueChange on datepicker-input internal
* fix(datepicker): calendar selected date should change when input changes * startAt fix * fix tests * address comments * fix unsubscribe * make valueChange on datepicker-input internal
* fix(datepicker): calendar selected date should change when input changes * startAt fix * fix tests * address comments * fix unsubscribe * make valueChange on datepicker-input internal
* fix(datepicker): calendar selected date should change when input changes * startAt fix * fix tests * address comments * fix unsubscribe * make valueChange on datepicker-input internal
* fix(datepicker): calendar selected date should change when input changes * startAt fix * fix tests * address comments * fix unsubscribe * make valueChange on datepicker-input internal
|
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. |
No description provided.