Skip to content

Commit c848d24

Browse files
authored
fix(material/timepicker): assign form control value before emitting events (#31981)
Fixes that we were emitting some events before the form control value was assigned. Fixes #31950.
1 parent b61d884 commit c848d24

File tree

4 files changed

+89
-9
lines changed

4 files changed

+89
-9
lines changed

goldens/material/timepicker/index.api.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ export class MatTimepickerInput<D> implements ControlValueAccessor, Validator, O
100100
registerOnValidatorChange(fn: () => void): void;
101101
setDisabledState(isDisabled: boolean): void;
102102
readonly timepicker: InputSignal<MatTimepicker<D>>;
103+
_timepickerValueAssigned(value: D | null): void;
103104
validate(control: AbstractControl): ValidationErrors | null;
104105
readonly value: ModelSignal<D | null>;
105106
writeValue(value: any): void;

src/material/timepicker/timepicker-input.ts

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,14 @@ export class MatTimepickerInput<D> implements ControlValueAccessor, Validator, O
317317
}
318318
}
319319

320+
/** Called by the timepicker to sync up the user-selected value. */
321+
_timepickerValueAssigned(value: D | null) {
322+
if (!this._dateAdapter.sameTime(value, this.value())) {
323+
this._assignUserSelection(value, true);
324+
this._formatValue(value);
325+
}
326+
}
327+
320328
/** Sets up the code that watches for changes in the value and adjusts the input. */
321329
private _respondToValueChanges(): void {
322330
effect(() => {
@@ -346,12 +354,6 @@ export class MatTimepickerInput<D> implements ControlValueAccessor, Validator, O
346354
const timepicker = this.timepicker();
347355
timepicker.registerInput(this);
348356
timepicker.closed.subscribe(() => this._onTouched?.());
349-
timepicker.selected.subscribe(({value}) => {
350-
if (!this._dateAdapter.sameTime(value, this.value())) {
351-
this._assignUserSelection(value, true);
352-
this._formatValue(value);
353-
}
354-
});
355357
});
356358
}
357359

@@ -371,8 +373,10 @@ export class MatTimepickerInput<D> implements ControlValueAccessor, Validator, O
371373
* @param propagateToAccessor Whether the value should be propagated to the ControlValueAccessor.
372374
*/
373375
private _assignUserSelection(selection: D | null, propagateToAccessor: boolean) {
376+
let toAssign: D | null;
377+
374378
if (selection == null || !this._isValid(selection)) {
375-
this.value.set(selection);
379+
toAssign = selection;
376380
} else {
377381
// If a datepicker and timepicker are writing to the same object and the user enters an
378382
// invalid time into the timepicker, we may end up clearing their selection from the
@@ -384,12 +388,15 @@ export class MatTimepickerInput<D> implements ControlValueAccessor, Validator, O
384388
const hours = adapter.getHours(selection);
385389
const minutes = adapter.getMinutes(selection);
386390
const seconds = adapter.getSeconds(selection);
387-
this.value.set(target ? adapter.setTime(target, hours, minutes, seconds) : selection);
391+
toAssign = target ? adapter.setTime(target, hours, minutes, seconds) : selection;
388392
}
389393

394+
// Propagate to the form control before emitting to `valueChange`.
390395
if (propagateToAccessor) {
391-
this._onChange?.(this.value());
396+
this._onChange?.(toAssign);
392397
}
398+
399+
this.value.set(toAssign);
393400
}
394401

395402
/** Formats the current value and assigns it to the input. */

src/material/timepicker/timepicker.spec.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,6 +1156,75 @@ describe('MatTimepicker', () => {
11561156
expect(input.disabled).toBe(true);
11571157
expect(fixture.componentInstance.input.disabled()).toBe(true);
11581158
});
1159+
1160+
it('should emit to valueChange before assigning control value when typing', () => {
1161+
const fixture = TestBed.createComponent(TimepickerWithForms);
1162+
const control = fixture.componentInstance.control;
1163+
let eventValue: Date | null = null;
1164+
let controlValue: Date | null = null;
1165+
fixture.detectChanges();
1166+
1167+
const subscription = fixture.componentInstance.input.value.subscribe(value => {
1168+
eventValue = value;
1169+
controlValue = control.value;
1170+
});
1171+
1172+
typeInElement(getInput(fixture), '1:37 PM');
1173+
fixture.detectChanges();
1174+
1175+
expect(eventValue).toBeTruthy();
1176+
expect(controlValue).toBeTruthy();
1177+
expectSameTime(eventValue, controlValue);
1178+
subscription.unsubscribe();
1179+
});
1180+
1181+
it('should emit to valueChange before assigning control value when clicking an option', () => {
1182+
const fixture = TestBed.createComponent(TimepickerWithForms);
1183+
const control = fixture.componentInstance.control;
1184+
let eventValue: Date | null = null;
1185+
let controlValue: Date | null = null;
1186+
fixture.detectChanges();
1187+
1188+
const subscription = fixture.componentInstance.input.value.subscribe(value => {
1189+
eventValue = value;
1190+
controlValue = control.value;
1191+
});
1192+
1193+
getInput(fixture).click();
1194+
fixture.detectChanges();
1195+
getOptions()[5].click();
1196+
fixture.detectChanges();
1197+
fixture.detectChanges();
1198+
1199+
expect(eventValue).toBeTruthy();
1200+
expect(controlValue).toBeTruthy();
1201+
expectSameTime(eventValue, controlValue);
1202+
subscription.unsubscribe();
1203+
});
1204+
1205+
it('should emit to selected event before assigning control value when clicking an option', () => {
1206+
const fixture = TestBed.createComponent(TimepickerWithForms);
1207+
const control = fixture.componentInstance.control;
1208+
let eventValue: Date | null = null;
1209+
let controlValue: Date | null = null;
1210+
fixture.detectChanges();
1211+
1212+
const subscription = fixture.componentInstance.timepicker.selected.subscribe(event => {
1213+
eventValue = event.value;
1214+
controlValue = control.value;
1215+
});
1216+
1217+
getInput(fixture).click();
1218+
fixture.detectChanges();
1219+
getOptions()[5].click();
1220+
fixture.detectChanges();
1221+
fixture.detectChanges();
1222+
1223+
expect(eventValue).toBeTruthy();
1224+
expect(controlValue).toBeTruthy();
1225+
expectSameTime(eventValue, controlValue);
1226+
subscription.unsubscribe();
1227+
});
11591228
});
11601229

11611230
describe('timepicker toggle', () => {
@@ -1410,6 +1479,7 @@ class TimepickerTwoWayBinding {
14101479
})
14111480
class TimepickerWithForms {
14121481
@ViewChild(MatTimepickerInput) input: MatTimepickerInput<Date>;
1482+
@ViewChild(MatTimepicker) timepicker: MatTimepicker<Date>;
14131483
readonly control = new FormControl<Date | null>(null, [Validators.required]);
14141484
readonly min = signal<Date | null>(null);
14151485
readonly max = signal<Date | null>(null);

src/material/timepicker/timepicker.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,8 @@ export class MatTimepicker<D> implements OnDestroy, MatOptionParentComponent {
296296
current.deselect(false);
297297
}
298298
});
299+
// Notify the input first so it can sync up the form control before emitting to `selected`.
300+
this._input()?._timepickerValueAssigned(option.value);
299301
this.selected.emit({value: option.value, source: this});
300302
this._input()?.focus();
301303
}

0 commit comments

Comments
 (0)