Fix mwc-radio group synchronization when not created/connected simult…

…aneously

Selection groups are keyed by root (document or shadow root). Before, we created our selection group controller at construction. This is fine when creating and connecting at the same time (e.g. in HTML, or in a simple lit template), but fails otherwise, because we get the wrong root. Now we defer creating our controller until connectedCallback, so that we always get the correct root.

Fixes #282

Supersedes #279

PiperOrigin-RevId: 274220064

CHANGELOG.md

@@ -25,6 +25,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 ### Fixed
 
 - Fixed `mwc-dialog`'s issues with working on older browsers.
+- `<mwc-radio>` groups are now correctly synchronized when stamped using a
+  lit-html `map` or `repeat`, and any other time the radio is not created and
+  connected at the same time ([#282](https://github.com/material-components/material-components-web-components/issues/282)).
 
 ## [0.9.0] - 2019-09-26
 

packages/radio/src/mwc-radio-base.ts

@@ -62,7 +62,9 @@ export class RadioBase extends FormElement {
     if (this.formElement) {
       this.formElement.checked = checked;
     }
-    this._selectionController.update(this);
+    if (this._selectionController !== undefined) {
+      this._selectionController.update(this);
+    }
     this.requestUpdate('checked', oldValue);
   }
 
@@ -84,23 +86,33 @@ export class RadioBase extends FormElement {
 
   protected mdcFoundation!: MDCRadioFoundation;
 
-  /* eslint-disable @typescript-eslint/no-use-before-define */
-
-  // Note if we aren't using native shadow DOM, then we don't technically need a
-  // SelectionController, because our inputs will share document-scoped native
-  // selection groups. However, it simplifies implementation and testing to use
-  // one in all cases. In particular, it means we correctly manage groups before
-  // the first update stamps the native input.
-  private _selectionController = SelectionController.getController(this);
-  /* eslint-enable @typescript-eslint/no-use-before-define */
+  private _selectionController?: SelectionController;
 
   connectedCallback() {
     super.connectedCallback();
+    // Note that we must defer creating the selection controller until the
+    // element has connected, because selection controllers are keyed by the
+    // radio's shadow root. For example, if we're stamping in a lit-html map
+    // or repeat, then we'll be constructed before we're added to a root node.
+    //
+    // Also note if we aren't using native shadow DOM, then we don't technically
+    // need a SelectionController, because our inputs will share document-scoped
+    // native selection groups. However, it simplifies implementation and
+    // testing to use one in all cases. In particular, it means we correctly
+    // manage groups before the first update stamps the native input.
+    this._selectionController = SelectionController.getController(this);
     this._selectionController.register(this);
+    // With native <input type="radio">, when a checked radio is added to the
+    // root, then it wins. Immediately update to emulate this behavior.
+    this._selectionController.update(this);
   }
 
   disconnectedCallback() {
-    this._selectionController.unregister(this);
+    // The controller is initialized in connectedCallback, so if we are in
+    // disconnectedCallback then it must be initialized.
+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    this._selectionController!.unregister(this);
+    this._selectionController = undefined;
   }
 
   focusNative() {
@@ -125,7 +137,9 @@ export class RadioBase extends FormElement {
   }
 
   private _focusHandler() {
-    this._selectionController.focus(this);
+    if (this._selectionController !== undefined) {
+      this._selectionController.focus(this);
+    }
   }
 
   private _clickHandler() {
@@ -158,7 +172,9 @@ export class RadioBase extends FormElement {
     // We might not have been able to synchronize this from the checked setter
     // earlier, if checked was set before the input was stamped.
     this.formElement.checked = this.checked;
-    this._selectionController.update(this);
+    if (this._selectionController !== undefined) {
+      this._selectionController.update(this);
+    }
   }
 }
 

packages/radio/src/test/mwc-radio.test.ts

@@ -139,5 +139,78 @@ suite('mwc-radio', () => {
       assert.isFalse(a2.checked);
       assert.isFalse(b1.checked);
     });
+
+    test('when checked before connected', () => {
+      const r1 = document.createElement('mwc-radio');
+      r1.name = 'a';
+      const r2 = document.createElement('mwc-radio');
+      r2.name = 'a';
+      const r3 = document.createElement('mwc-radio');
+      r3.name = 'a';
+
+      // r1 and r2 should both be checked, because even though they have the
+      // same name, they aren't yet connected to a root. Groups are scoped to
+      // roots, and we can't know which root a radio belongs to until it is
+      // connected to one. This matches native <input type="radio"> behavior.
+      r1.checked = true;
+      r2.checked = true;
+      assert.isTrue(r1.checked);
+      assert.isTrue(r2.checked);
+      assert.isFalse(r3.checked);
+
+      // Connecting r1 shouldn't change anything, since it's the only one in the
+      // group.
+      container.appendChild(r1);
+      assert.isTrue(r1.checked);
+      assert.isTrue(r2.checked);
+      assert.isFalse(r3.checked);
+
+      // Appending r2 should disable r1, because when a new checked radio is
+      // connected, it wins (this matches native input behavior).
+      container.appendChild(r2);
+      assert.isFalse(r1.checked);
+      assert.isTrue(r2.checked);
+      assert.isFalse(r3.checked);
+
+      // Appending r3 shouldn't change anything, because it's not checked.
+      container.appendChild(r3);
+      assert.isFalse(r1.checked);
+      assert.isTrue(r2.checked);
+      assert.isFalse(r3.checked);
+
+      // Checking r3 should uncheck r2 because it's now in the same group.
+      r3.checked = true;
+      assert.isFalse(r1.checked);
+      assert.isFalse(r2.checked);
+      assert.isTrue(r3.checked);
+    });
+
+    test('in a lit repeat', () => {
+      const values = ['a1', 'a2'];
+      render(
+          html`${
+              values.map(
+                  (value) =>
+                      html`<mwc-radio value=${value} name="a"></mwc-radio>`)}`,
+          container);
+      const [a1, a2] = container.querySelectorAll('mwc-radio');
+
+      assert.isFalse(a1.checked);
+      assert.isFalse(a2.checked);
+      assert.equal(a1.value, values[0]);
+      assert.equal(a2.value, values[1]);
+
+      a1.checked = true;
+      assert.isTrue(a1.checked);
+      assert.isFalse(a2.checked);
+
+      a2.checked = true;
+      assert.isFalse(a1.checked);
+      assert.isTrue(a2.checked);
+
+      a2.checked = false;
+      assert.isFalse(a1.checked);
+      assert.isFalse(a2.checked);
+    });
   });
 });