fix(forms): Make radio buttons respect [attr.disabled] (#48864)

dylhunn · pkozlowski-opensource · commit 5f2a3edcf27c · 2023-02-10T11:25:12.000+01:00
`setDisabledState` is supposed to be called whenever the disabled state of a control changes, including upon control creation. However, a longstanding bug caused the method to not fire when an *enabled* control was attached. This bug was fixed in v15. This had a side effect: previously, it was possible to instantiate a reactive form control with `[attr.disabled]=true`, even though the the corresponding control was enabled in the model. (Note that the similar-looking property binding version `[disabled]=true` was always rejected, though.) This resulted in a mismatch between the model and the DOM. Now, because `setDisabledState` is always called, the value in the DOM will be immediately overwritten with the "correct" enabled value. Users should instead disable the control directly in their model. (There are many ways to do this, such as using the `{value: 'foo', disabled: true}` constructor format, or immediately calling `FooControl.disable()` in `ngOnInit`.) If this incompatibility is too breaking, you may also opt out using `FormsModule.withConfig` or `ReactiveFormsModule.withConfig` at the time you import it, via the `callSetDisabledState` option. However, there is an exceptional case: radio buttons. Because Reactive Forms models the entire group of radio buttons as a single `FormControl`, there is no way to control the disabled state for individual radios, so they can no longer be configured as disabled. In this PR, we have special cased radio buttons to ignore their first call to `setDisabledState` when in `callSetDisabledState: 'always'` mode. This preserves the old behavior. PR Close #48864
diff --git a/goldens/public-api/forms/index.md b/goldens/public-api/forms/index.md
@@ -781,6 +781,8 @@ export class RadioControlValueAccessor extends BuiltInControlValueAccessor imple
     ngOnInit(): void;
     onChange: () => void;
     registerOnChange(fn: (_: any) => {}): void;
+    // (undocumented)
+    setDisabledState(isDisabled: boolean): void;
     value: any;
     writeValue(value: any): void;
     // (undocumented)
diff --git a/packages/forms/src/directives/radio_control_value_accessor.ts b/packages/forms/src/directives/radio_control_value_accessor.ts
@@ -6,12 +6,13 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Directive, ElementRef, forwardRef, Injectable, Injector, Input, NgModule, OnDestroy, OnInit, Provider, Renderer2, ɵRuntimeError as RuntimeError} from '@angular/core';
+import {Directive, ElementRef, forwardRef, inject, Injectable, Injector, Input, NgModule, OnDestroy, OnInit, Provider, Renderer2, ɵRuntimeError as RuntimeError} from '@angular/core';
 
 import {RuntimeErrorCode} from '../errors';
 
 import {BuiltInControlValueAccessor, ControlValueAccessor, NG_VALUE_ACCESSOR} from './control_value_accessor';
 import {NgControl} from './ng_control';
+import {CALL_SET_DISABLED_STATE, setDisabledStateDefault, SetDisabledStateOption} from './shared';
 
 const RADIO_VALUE_ACCESSOR: Provider = {
   provide: NG_VALUE_ACCESSOR,
@@ -124,6 +125,8 @@ export class RadioControlValueAccessor extends BuiltInControlValueAccessor imple
   // TODO(issue/24571): remove '!'.
   _fn!: Function;
 
+  private setDisabledStateFired = false;
+
   /**
    * The registered callback function called when a change event occurs on the input element.
    * Note: we declare `onChange` here (also used as host listener) as a function with no arguments
@@ -154,6 +157,9 @@ export class RadioControlValueAccessor extends BuiltInControlValueAccessor imple
    */
   @Input() value: any;
 
+  private callSetDisabledState =
+      inject(CALL_SET_DISABLED_STATE, {optional: true}) ?? setDisabledStateDefault;
+
   constructor(
       renderer: Renderer2, elementRef: ElementRef, private _registry: RadioControlRegistry,
       private _injector: Injector) {
@@ -193,6 +199,33 @@ export class RadioControlValueAccessor extends BuiltInControlValueAccessor imple
     };
   }
 
+  /** @nodoc */
+  override setDisabledState(isDisabled: boolean): void {
+    /**
+     * `setDisabledState` is supposed to be called whenever the disabled state of a control changes,
+     * including upon control creation. However, a longstanding bug caused the method to not fire
+     * when an *enabled* control was attached. This bug was fixed in v15 in #47576.
+     *
+     * This had a side effect: previously, it was possible to instantiate a reactive form control
+     * with `[attr.disabled]=true`, even though the the corresponding control was enabled in the
+     * model. This resulted in a mismatch between the model and the DOM. Now, because
+     * `setDisabledState` is always called, the value in the DOM will be immediately overwritten
+     * with the "correct" enabled value.
+     *
+     * However, the fix also created an exceptional case: radio buttons. Because Reactive Forms
+     * models the entire group of radio buttons as a single `FormControl`, there is no way to
+     * control the disabled state for individual radios, so they can no longer be configured as
+     * disabled. Thus, we keep the old behavior for radio buttons, so that `[attr.disabled]`
+     * continues to work. Specifically, we drop the first call to `setDisabledState` if `disabled`
+     * is `false`, and we are not in legacy mode.
+     */
+    if (this.setDisabledStateFired || isDisabled ||
+        this.callSetDisabledState === 'whenDisabledForLegacyCode') {
+      this.setProperty('disabled', isDisabled);
+    }
+    this.setDisabledStateFired = true;
+  }
+
   /**
    * Sets the "value" on the radio input element and unchecks it.
    *
diff --git a/packages/forms/test/reactive_integration_spec.ts b/packages/forms/test/reactive_integration_spec.ts
@@ -3634,6 +3634,42 @@ const ValueAccessorB = createControlValueAccessor('[cva-b]');
       });
     });
 
+    describe('radio groups', () => {
+      it('should respect the attr.disabled state as an initial value', () => {
+        const fixture = initTest(RadioForm);
+        const choice = new FormControl('one');
+        const form = new FormGroup({'choice': choice});
+        fixture.componentInstance.form = form;
+        fixture.detectChanges();
+
+        const oneInput = fixture.debugElement.query(By.css('input'));
+
+        // The 'one' input is initially disabled in the view with [attr.disabled].
+        // The model thinks it is enabled. This inconsistent state was retained for backwards
+        // compatibility. (See `RadioControlValueAccessor` for details.)
+        expect(oneInput.attributes.disabled).toBe('true');
+        expect(choice.disabled).toBe(false);
+
+        // Flipping the attribute back and forth does not change the model
+        // as expected. Once the initial `disabled` value is setup, the model
+        // becomes the source of truth.
+        oneInput.nativeElement.setAttribute('disabled', 'false');
+        fixture.detectChanges();
+        expect(oneInput.attributes.disabled).toBe('false');
+        expect(choice.disabled).toBe(false);
+
+        oneInput.nativeElement.setAttribute('disabled', 'true');
+        fixture.detectChanges();
+        expect(oneInput.attributes.disabled).toBe('true');
+        expect(choice.disabled).toBe(false);
+
+        oneInput.nativeElement.setAttribute('disabled', 'false');
+        fixture.detectChanges();
+        expect(oneInput.attributes.disabled).toBe('false');
+        expect(choice.disabled).toBe(false);
+      });
+    });
+
     describe('IME events', () => {
       it('should determine IME event handling depending on platform by default', () => {
         const fixture = initTest(FormControlComp);
@@ -5579,3 +5615,18 @@ class NativeDialogForm {
   @ViewChild('form') form!: ElementRef<HTMLFormElement>;
   formGroup = new FormGroup({});
 }
+
+@Component({
+  selector: 'radio-form',
+  template: `
+  <form [formGroup]="form">
+    <input type="radio" formControlName="choice" value="one" [attr.disabled]="true"> One
+    <input type="radio" formControlName="choice" value="two"> Two
+  </form>
+  `,
+})
+export class RadioForm {
+  form = new FormGroup({
+    choice: new FormControl('one'),
+  });
+}
