Skip to content

Commit acd7c83

Browse files
leonsenftAndrewKushnir
authored andcommitted
fix(forms): test that min/max properties are propagated to controls (angular#63884)
Fix a bug where the min property of a form field was not correctly propagated to custom controls. Also ensure that min and max are only bound to native input elements that support them. PR Close angular#63884
1 parent f4d1017 commit acd7c83

File tree

2 files changed

+154
-4
lines changed

2 files changed

+154
-4
lines changed

packages/core/src/render3/instructions/control.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ function updateCustomControl(
332332
maybeWriteToDirectiveInput(componentDef, component, 'disabledReasons', state.disabledReasons);
333333
maybeWriteToDirectiveInput(componentDef, component, 'max', state.max);
334334
maybeWriteToDirectiveInput(componentDef, component, 'maxLength', state.maxLength);
335-
maybeWriteToDirectiveInput(componentDef, component, 'min', state.max);
335+
maybeWriteToDirectiveInput(componentDef, component, 'min', state.min);
336336
maybeWriteToDirectiveInput(componentDef, component, 'minLength', state.minLength);
337337
maybeWriteToDirectiveInput(componentDef, component, 'name', state.name);
338338
maybeWriteToDirectiveInput(componentDef, component, 'pattern', state.pattern);
@@ -368,7 +368,7 @@ function maybeWriteToDirectiveInput(
368368
* @param control The `ɵControl` directive instance.
369369
*/
370370
function updateNativeControl(tNode: TNode, lView: LView, control: ɵControl<unknown>): void {
371-
const input = getNativeByTNode(tNode, lView) as HTMLInputElement;
371+
const input = getNativeByTNode(tNode, lView) as NativeControlElement;
372372
const renderer = lView[RENDERER];
373373
const state = control.state();
374374

@@ -380,11 +380,16 @@ function updateNativeControl(tNode: TNode, lView: LView, control: ɵControl<unkn
380380
setBooleanAttribute(renderer, input, 'readonly', state.readonly());
381381
setBooleanAttribute(renderer, input, 'required', state.required());
382382

383+
// TODO: https://github.com/orgs/angular/projects/60/views/1?pane=issue&itemId=131711472
384+
// * cache this in `tNode.flags`.
385+
if (isNumericInput(input)) {
386+
setOptionalAttribute(renderer, input, 'max', state.max());
387+
setOptionalAttribute(renderer, input, 'min', state.min());
388+
}
389+
383390
// TODO: https://github.com/orgs/angular/projects/60/views/1?pane=issue&itemId=131711472
384391
// * use tag and type attribute to determine which of these properties to bind.
385-
setOptionalAttribute(renderer, input, 'max', state.max());
386392
setOptionalAttribute(renderer, input, 'maxLength', state.maxLength());
387-
setOptionalAttribute(renderer, input, 'min', state.min());
388393
setOptionalAttribute(renderer, input, 'minLength', state.minLength());
389394
}
390395

@@ -393,6 +398,21 @@ function isDateOrNull(value: unknown): value is Date | null {
393398
return value === null || value instanceof Date;
394399
}
395400

401+
/** Returns whether `control` has a numeric input type. */
402+
function isNumericInput(control: NativeControlElement) {
403+
switch (control.type) {
404+
case 'date':
405+
case 'datetime-local':
406+
case 'month':
407+
case 'number':
408+
case 'range':
409+
case 'time':
410+
case 'week':
411+
return true;
412+
}
413+
return false;
414+
}
415+
396416
/**
397417
* Returns the value from a native control element.
398418
*

packages/forms/signals/test/web/control_directive.spec.ts

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,136 @@ describe('control directive', () => {
299299
expect(component.customControl().required()).toBe(true);
300300
});
301301
});
302+
303+
describe('max', () => {
304+
it('native control', () => {
305+
@Component({
306+
imports: [Control],
307+
template: `<input type="number" [control]="f">`,
308+
})
309+
class TestCmp {
310+
readonly max = signal(10);
311+
readonly f = form(signal(5), (p) => {
312+
max(p, this.max);
313+
});
314+
}
315+
316+
const fixture = act(() => TestBed.createComponent(TestCmp));
317+
const element = fixture.nativeElement.firstChild as HTMLInputElement;
318+
expect(element.max).toBe('10');
319+
320+
act(() => fixture.componentInstance.max.set(5));
321+
expect(element.max).toBe('5');
322+
});
323+
324+
it('custom control', () => {
325+
@Component({selector: 'custom-control', template: ``})
326+
class CustomControl {
327+
readonly value = model(0);
328+
readonly max = input<number | null>(null);
329+
}
330+
331+
@Component({
332+
imports: [Control, CustomControl],
333+
template: `<custom-control [control]="f" />`,
334+
})
335+
class TestCmp {
336+
readonly max = signal(10);
337+
readonly f = form(signal(5), (p) => {
338+
max(p, this.max);
339+
});
340+
readonly customControl = viewChild.required(CustomControl);
341+
}
342+
343+
const fixture = act(() => TestBed.createComponent(TestCmp));
344+
const component = fixture.componentInstance;
345+
expect(component.customControl().max()).toBe(10);
346+
347+
act(() => component.max.set(5));
348+
expect(component.customControl().max()).toBe(5);
349+
});
350+
351+
it('is not set on native control if type does not support it', () => {
352+
@Component({
353+
imports: [Control],
354+
template: `<input type="text" [control]="f">`,
355+
})
356+
class TestCmp {
357+
readonly f = form(signal(5), (p) => {
358+
max(p, 10);
359+
});
360+
}
361+
362+
const fixture = act(() => TestBed.createComponent(TestCmp));
363+
const element = fixture.nativeElement.firstChild as HTMLInputElement;
364+
expect(element.max).toBe('');
365+
});
366+
});
367+
368+
describe('min', () => {
369+
it('native control', () => {
370+
@Component({
371+
imports: [Control],
372+
template: `<input type="number" [control]="f">`,
373+
})
374+
class TestCmp {
375+
readonly min = signal(10);
376+
readonly f = form(signal(15), (p) => {
377+
min(p, this.min);
378+
});
379+
}
380+
381+
const fixture = act(() => TestBed.createComponent(TestCmp));
382+
const element = fixture.nativeElement.firstChild as HTMLInputElement;
383+
expect(element.min).toBe('10');
384+
385+
act(() => fixture.componentInstance.min.set(5));
386+
expect(element.min).toBe('5');
387+
});
388+
389+
it('custom control', () => {
390+
@Component({selector: 'custom-control', template: ``})
391+
class CustomControl {
392+
readonly value = model(0);
393+
readonly min = input<number>();
394+
}
395+
396+
@Component({
397+
imports: [Control, CustomControl],
398+
template: `<custom-control [control]="f" />`,
399+
})
400+
class TestCmp {
401+
readonly min = signal(10);
402+
readonly f = form(signal(15), (p) => {
403+
min(p, this.min);
404+
});
405+
readonly customControl = viewChild.required(CustomControl);
406+
}
407+
408+
const fixture = act(() => TestBed.createComponent(TestCmp));
409+
const component = fixture.componentInstance;
410+
expect(component.customControl().min()).toBe(10);
411+
412+
act(() => component.min.set(5));
413+
expect(component.customControl().min()).toBe(5);
414+
});
415+
416+
it('is not set on native control if type does not support it', () => {
417+
@Component({
418+
imports: [Control],
419+
template: `<input type="text" [control]="f">`,
420+
})
421+
class TestCmp {
422+
readonly f = form(signal(15), (p) => {
423+
min(p, 10);
424+
});
425+
}
426+
427+
const fixture = act(() => TestBed.createComponent(TestCmp));
428+
const element = fixture.nativeElement.firstChild as HTMLInputElement;
429+
expect(element.min).toBe('');
430+
});
431+
});
302432
});
303433

304434
it('synchronizes a basic form with a custom control', () => {

0 commit comments

Comments
 (0)