Skip to content

Commit cd22ab6

Browse files
authored
fix(material/timepicker): valueChanges emitting on init (#32434)
Fixes that the `valueChanges` stream from reactive forms was emitting on init for the timepicker input. It was because we had an `effect` that was meant to trigger `min`/`max` revalidation. I also ended up consolidating all the validation updates into a single effect so we don't have multiple sources of changes. Fixes #32423.
1 parent 2494f0a commit cd22ab6

File tree

2 files changed

+57
-35
lines changed

2 files changed

+57
-35
lines changed

src/material/timepicker/timepicker-input.ts

Lines changed: 43 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ export class MatTimepickerInput<D>
9797
private _timepickerSubscription: OutputRefSubscription | undefined;
9898
private _validator: ValidatorFn;
9999
private _lastValueValid = true;
100+
private _minValid = true;
101+
private _maxValid = true;
100102
private _lastValidDate: D | null = null;
101103

102104
/** Value of the `aria-activedescendant` attribute. */
@@ -173,8 +175,7 @@ export class MatTimepickerInput<D>
173175

174176
const renderer = inject(Renderer2);
175177
this._validator = this._getValidator();
176-
this._respondToValueChanges();
177-
this._respondToMinMaxChanges();
178+
this._updateFormsState();
178179
this._registerTimepicker();
179180
this._localeSubscription = this._dateAdapter.localeChanges.subscribe(() => {
180181
if (!this._hasFocus()) {
@@ -334,24 +335,37 @@ export class MatTimepickerInput<D>
334335
}
335336
}
336337

337-
/** Sets up the code that watches for changes in the value and adjusts the input. */
338-
private _respondToValueChanges(): void {
338+
/** Sets up the code that keeps the input state in sync with the forms module. */
339+
private _updateFormsState(): void {
339340
effect(() => {
340-
const value = this._dateAdapter.deserialize(this.value());
341-
const wasValid = this._lastValueValid;
342-
this._lastValueValid = this._isValid(value);
341+
const {
342+
_dateAdapter: adapter,
343+
_lastValueValid: prevValueValid,
344+
_minValid: prevMinValid,
345+
_maxValid: prevMaxValid,
346+
} = this;
347+
const value = adapter.deserialize(this.value());
348+
const min = this.min();
349+
const max = this.max();
350+
const valueValid = (this._lastValueValid = this._isValid(value));
351+
this._minValid = !min || !value || !valueValid || adapter.compareTime(min, value) <= 0;
352+
this._maxValid = !max || !value || !valueValid || adapter.compareTime(max, value) >= 0;
353+
const stateChanged =
354+
prevValueValid !== valueValid ||
355+
prevMinValid !== this._minValid ||
356+
prevMaxValid !== this._maxValid;
343357

344358
// Reformat the value if it changes while the user isn't interacting.
345359
if (!this._hasFocus()) {
346360
this._formatValue(value);
347361
}
348362

349-
if (value && this._lastValueValid) {
363+
if (value && valueValid) {
350364
this._lastValidDate = value;
351365
}
352366

353367
// Trigger the validator if the state changed.
354-
if (wasValid !== this._lastValueValid) {
368+
if (stateChanged) {
355369
this._validatorOnChange?.();
356370
}
357371
});
@@ -366,16 +380,6 @@ export class MatTimepickerInput<D>
366380
});
367381
}
368382

369-
/** Sets up the logic that adjusts the input if the min/max changes. */
370-
private _respondToMinMaxChanges(): void {
371-
effect(() => {
372-
// Read the min/max so the effect knows when to fire.
373-
this.min();
374-
this.max();
375-
this._validatorOnChange?.();
376-
});
377-
}
378-
379383
/**
380384
* Assigns a value set by the user to the input's model.
381385
* @param selection Time selected by the user that should be assigned.
@@ -441,24 +445,28 @@ export class MatTimepickerInput<D>
441445
this._lastValueValid
442446
? null
443447
: {'matTimepickerParse': {'text': this._elementRef.nativeElement.value}},
444-
control => {
445-
const controlValue = this._dateAdapter.getValidDateOrNull(
446-
this._dateAdapter.deserialize(control.value),
447-
);
448-
const min = this.min();
449-
return !min || !controlValue || this._dateAdapter.compareTime(min, controlValue) <= 0
448+
control =>
449+
this._minValid
450450
? null
451-
: {'matTimepickerMin': {'min': min, 'actual': controlValue}};
452-
},
453-
control => {
454-
const controlValue = this._dateAdapter.getValidDateOrNull(
455-
this._dateAdapter.deserialize(control.value),
456-
);
457-
const max = this.max();
458-
return !max || !controlValue || this._dateAdapter.compareTime(max, controlValue) >= 0
451+
: {
452+
'matTimepickerMin': {
453+
'min': this.min(),
454+
'actual': this._dateAdapter.getValidDateOrNull(
455+
this._dateAdapter.deserialize(control.value),
456+
),
457+
},
458+
},
459+
control =>
460+
this._maxValid
459461
? null
460-
: {'matTimepickerMax': {'max': max, 'actual': controlValue}};
461-
},
462+
: {
463+
'matTimepickerMax': {
464+
'max': this.max(),
465+
'actual': this._dateAdapter.getValidDateOrNull(
466+
this._dateAdapter.deserialize(control.value),
467+
),
468+
},
469+
},
462470
])!;
463471
}
464472
}

src/material/timepicker/timepicker.spec.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1225,6 +1225,20 @@ describe('MatTimepicker', () => {
12251225
expectSameTime(eventValue, controlValue);
12261226
subscription.unsubscribe();
12271227
});
1228+
1229+
it('should not emit toValueOnChanges on init', () => {
1230+
const fixture = TestBed.createComponent(TimepickerWithForms);
1231+
const spy = jasmine.createSpy('valueChanges');
1232+
const subscription = fixture.componentInstance.control.valueChanges.subscribe(spy);
1233+
fixture.detectChanges();
1234+
expect(spy).not.toHaveBeenCalled();
1235+
1236+
typeInElement(getInput(fixture), '1:37 PM');
1237+
fixture.detectChanges();
1238+
1239+
expect(spy).toHaveBeenCalled();
1240+
subscription.unsubscribe();
1241+
});
12281242
});
12291243

12301244
describe('timepicker toggle', () => {

0 commit comments

Comments
 (0)