fix(forms): handle standalone `<form>` tag correctly in `NgControlStatusGroup` directive (#40344)
The `NgControlStatusGroup` directive is shared between template-driven and reactive form modules. In cases when only reactive forms module is present, the `NgControlStatusGroup` directive is still activated on all `<form>` elements, but if there is no other reactive directive applied (such as `formGroup`), corresponding `ControlContainer` token is missing, thus causing exceptions (since `NgControlStatusGroup` directive relies on it to determine the status). This commit updates the logic to handle the case when no `ControlContainer` is present (effectively making directive logic a noop in this case). Alternative approach (more risky) worth considering in the future is to split the `NgControlStatusGroup` into 2 directives with different set of selectors and include them into template-driven and reactive modules separately. The downside is that these directives might be activated simultaneously on the same element (e.g. `<form>`), effectively doing the work twice. Resolves #38391. PR Close #40344
This commit is contained in:
parent
7a819caef6
commit
fdbd3cae8a
|
@ -6,39 +6,39 @@
|
||||||
* found in the LICENSE file at https://angular.io/license
|
* found in the LICENSE file at https://angular.io/license
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import {Directive, Self} from '@angular/core';
|
import {Directive, Optional, Self} from '@angular/core';
|
||||||
|
|
||||||
import {AbstractControlDirective} from './abstract_control_directive';
|
import {AbstractControlDirective} from './abstract_control_directive';
|
||||||
import {ControlContainer} from './control_container';
|
import {ControlContainer} from './control_container';
|
||||||
import {NgControl} from './ng_control';
|
import {NgControl} from './ng_control';
|
||||||
|
|
||||||
export class AbstractControlStatus {
|
export class AbstractControlStatus {
|
||||||
private _cd: AbstractControlDirective;
|
private _cd: AbstractControlDirective|null;
|
||||||
|
|
||||||
constructor(cd: AbstractControlDirective) {
|
constructor(cd: AbstractControlDirective|null) {
|
||||||
this._cd = cd;
|
this._cd = cd;
|
||||||
}
|
}
|
||||||
|
|
||||||
get ngClassUntouched(): boolean {
|
get ngClassUntouched(): boolean {
|
||||||
return this._cd.control ? this._cd.control.untouched : false;
|
return this._cd?.control?.untouched ?? false;
|
||||||
}
|
}
|
||||||
get ngClassTouched(): boolean {
|
get ngClassTouched(): boolean {
|
||||||
return this._cd.control ? this._cd.control.touched : false;
|
return this._cd?.control?.touched ?? false;
|
||||||
}
|
}
|
||||||
get ngClassPristine(): boolean {
|
get ngClassPristine(): boolean {
|
||||||
return this._cd.control ? this._cd.control.pristine : false;
|
return this._cd?.control?.pristine ?? false;
|
||||||
}
|
}
|
||||||
get ngClassDirty(): boolean {
|
get ngClassDirty(): boolean {
|
||||||
return this._cd.control ? this._cd.control.dirty : false;
|
return this._cd?.control?.dirty ?? false;
|
||||||
}
|
}
|
||||||
get ngClassValid(): boolean {
|
get ngClassValid(): boolean {
|
||||||
return this._cd.control ? this._cd.control.valid : false;
|
return this._cd?.control?.valid ?? false;
|
||||||
}
|
}
|
||||||
get ngClassInvalid(): boolean {
|
get ngClassInvalid(): boolean {
|
||||||
return this._cd.control ? this._cd.control.invalid : false;
|
return this._cd?.control?.invalid ?? false;
|
||||||
}
|
}
|
||||||
get ngClassPending(): boolean {
|
get ngClassPending(): boolean {
|
||||||
return this._cd.control ? this._cd.control.pending : false;
|
return this._cd?.control?.pending ?? false;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -99,7 +99,7 @@ export class NgControlStatus extends AbstractControlStatus {
|
||||||
host: ngControlStatusHost
|
host: ngControlStatusHost
|
||||||
})
|
})
|
||||||
export class NgControlStatusGroup extends AbstractControlStatus {
|
export class NgControlStatusGroup extends AbstractControlStatus {
|
||||||
constructor(@Self() cd: ControlContainer) {
|
constructor(@Optional() @Self() cd: ControlContainer) {
|
||||||
super(cd);
|
super(cd);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -94,6 +94,13 @@ const ValueAccessorB = createControlValueAccessor('[cva-b]');
|
||||||
return TestBed.createComponent(component);
|
return TestBed.createComponent(component);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function initReactiveFormsTest<T>(
|
||||||
|
component: Type<T>, ...directives: Type<any>[]): ComponentFixture<T> {
|
||||||
|
TestBed.configureTestingModule(
|
||||||
|
{declarations: [component, ...directives], imports: [ReactiveFormsModule]});
|
||||||
|
return TestBed.createComponent(component);
|
||||||
|
}
|
||||||
|
|
||||||
// Helper method that attaches a spy to a `validate` function on a Validator class.
|
// Helper method that attaches a spy to a `validate` function on a Validator class.
|
||||||
function validatorSpyOn(validatorClass: any) {
|
function validatorSpyOn(validatorClass: any) {
|
||||||
return spyOn(validatorClass.prototype, 'validate').and.callThrough();
|
return spyOn(validatorClass.prototype, 'validate').and.callThrough();
|
||||||
|
@ -773,6 +780,49 @@ const ValueAccessorB = createControlValueAccessor('[cva-b]');
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('setting status classes', () => {
|
describe('setting status classes', () => {
|
||||||
|
it('should not assign status on standalone <form> element', () => {
|
||||||
|
@Component({
|
||||||
|
selector: 'form-comp',
|
||||||
|
template: `
|
||||||
|
<form></form>
|
||||||
|
`
|
||||||
|
})
|
||||||
|
class FormComp {
|
||||||
|
}
|
||||||
|
|
||||||
|
const fixture = initReactiveFormsTest(FormComp);
|
||||||
|
fixture.detectChanges();
|
||||||
|
|
||||||
|
const form = fixture.debugElement.query(By.css('form')).nativeElement;
|
||||||
|
// Expect no classes added to the <form> element since it has no
|
||||||
|
// reactive directives attached and only ReactiveForms module is used.
|
||||||
|
expect(sortedClassList(form)).toEqual([]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should not assign status on standalone <form> element with form control inside', () => {
|
||||||
|
@Component({
|
||||||
|
selector: 'form-comp',
|
||||||
|
template: `
|
||||||
|
<form>
|
||||||
|
<input type="text" [formControl]="control">
|
||||||
|
</form>
|
||||||
|
`
|
||||||
|
})
|
||||||
|
class FormComp {
|
||||||
|
control = new FormControl('abc');
|
||||||
|
}
|
||||||
|
const fixture = initReactiveFormsTest(FormComp);
|
||||||
|
fixture.detectChanges();
|
||||||
|
|
||||||
|
const form = fixture.debugElement.query(By.css('form')).nativeElement;
|
||||||
|
// Expect no classes added to the <form> element since it has no
|
||||||
|
// reactive directives attached and only ReactiveForms module is used.
|
||||||
|
expect(sortedClassList(form)).toEqual([]);
|
||||||
|
|
||||||
|
const input = fixture.debugElement.query(By.css('input')).nativeElement;
|
||||||
|
expect(sortedClassList(input)).toEqual(['ng-pristine', 'ng-untouched', 'ng-valid']);
|
||||||
|
});
|
||||||
|
|
||||||
it('should work with single fields', () => {
|
it('should work with single fields', () => {
|
||||||
const fixture = initTest(FormControlComp);
|
const fixture = initTest(FormControlComp);
|
||||||
const control = new FormControl('', Validators.required);
|
const control = new FormControl('', Validators.required);
|
||||||
|
|
Loading…
Reference in New Issue