Ensure keyboard nav does not regress on dynamic radios

justeattakeaway · Jan 22, 2025 · 2e76f0c · 2e76f0c
1 parent 0ff858c
commit 2e76f0c
Showing 1 changed file with 15 additions and 6 deletions.
diff --git a/packages/components/pie-radio-group/src/index.ts b/packages/components/pie-radio-group/src/index.ts
@@ -135,6 +135,17 @@ export class PieRadioGroup extends FormControlMixin(RtlMixin(LitElement)) implem
         this._hasLabel = childNodes.length > 0;
     }
 
+    /**
+     * Ensures all newly added radio buttons are not tabbable and inherit the name property
+     */
+    private _handleRadioSlotChange (): void {
+        // Make all (including any newly added) radio buttons impossible to tab to
+        // This is because by default, we are able to tab to each individual radio button.
+        // This is not the behaviour we want, so applying -1 tabindex prevents it.
+        this._slottedChildren.forEach((radio) => radio.setAttribute('tabindex', '-1'));
+        this._applyNameToChildren();
+    }
+
     /**
      * Renders the label element inside a legend, wrapping the slot content.
      * @returns {TemplateResult } The template for the label slot.
@@ -167,11 +178,9 @@ export class PieRadioGroup extends FormControlMixin(RtlMixin(LitElement)) implem
             this._handleStatus();
         }
 
-        // Make all (including any newly added) radio buttons impossible to tab to
-        // This is because by default, we are able to tab to each individual radio button.
-        // This is not the behaviour we want, so applying -1 tabindex prevents it.
-        this._slottedChildren.forEach((radio) => radio.setAttribute('tabindex', '-1'));
-        this._applyNameToChildren();
+        if (_changedProperties.has('name')) {
+            this._applyNameToChildren();
+        }
     }
 
     connectedCallback (): void {
@@ -351,7 +360,7 @@ export class PieRadioGroup extends FormControlMixin(RtlMixin(LitElement)) implem
                 aria-describedby=${hasAssistiveText ? assistiveTextId : nothing}
                 class="${classMap(classes)}">
                     ${this._renderWrappedLabel()}
-                <slot @slotchange=${this._applyNameToChildren}></slot>
+                <slot @slotchange=${this._handleRadioSlotChange}></slot>
             </fieldset>
             ${hasAssistiveText ? html`
                 <pie-assistive-text