fix(radio): repair group membership, derived tab stops and group propagation - #2344
Merged
Conversation
…agation The group of a radio was resolved by a tree walk over the whole root node on every read, and the tab stop was patched on the radio that changed, so a group could not react when a member left it. Removing or renaming the checked radio stranded the radios that stayed on `tabindex="-1"`. Each radio now registers under its root node and name for its whole life-cycle, and every mutation derives the tab stop from the state of the full group. - Removing the checked radio, or moving it to another group with `name`, hands the tab stop back to the radios that stay. This applies to plain name-based groups and not only to the ones under an `igc-radio-group`. - A form reset derives the tab stop again, because the reset restores the default state without the `checked` setter. - `igc-radio-group` adopts the radios that are slotted at run time - `name`, `defaultValue` and a pending `value` all apply to them - and reports the pending `value` while it holds no radios. - The group reflects `alignment` as `aria-orientation`, keeps its `role`, and no longer reports an empty group as disabled. - A group read is the size of the group instead of the size of the root node, and the four overlapping arrays of `getGroup` give way to one member query.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes radio/radio-group behavior around group membership and roving tab-index by introducing a lifecycle-aware group registry (keyed by root node + name) and deriving tab stops from full group state instead of patching only the radio that changed.
Changes:
- Introduces a radio group registry/controller to keep group membership stable across lifecycle changes and to re-derive roving tab stops on mutations.
- Updates radio validation and radio component group queries to use the new group membership mechanism (removing the previous DOM tree-walk implementation).
- Improves
igc-radio-groupbehavior for dynamic slotted radios (adoption ofname/defaultValue/pendingvalue) and adjusts ARIA/state syncing.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/internals/controllers/slot.ts | Makes slot querying resilient when renderRoot is not yet created. |
| src/components/radio/validators.ts | Switches required validation to use the new group membership query. |
| src/components/radio/utils.ts | Removes the old DOM tree-walk based group query helper. |
| src/components/radio/radio.ts | Integrates the new group controller for membership + roving tab index derivation. |
| src/components/radio/radio.spec.ts | Adds tests covering membership changes and tab-stop restoration scenarios. |
| src/components/radio/controller.ts | Adds the new root+name keyed radio group registry/controller implementation. |
| src/components/radio-group/radio-group.ts | Improves dynamic child adoption, pending value behavior, and ARIA/state syncing. |
| src/components/radio-group/radio-group.spec.ts | Adds tests for dynamic children adoption, custom states/layout, and ARIA behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- The tab stop comes from the enabled checked radio, and each change of `disabled` derives it again. - `igc-radio-group` keeps its value only while it is pending, so the removal of the checked radio no longer selects the radio of an old value. - The `checked` setter derives the group state only when the selection changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The group of a radio was resolved by a tree walk over the whole root node on every read, and the tab stop was patched on the radio that changed, so a group could not react when a member left it. Removing or renaming the checked radio stranded the radios that stayed on
tabindex="-1". Each radio now registers under its root node and name for its whole life cycle, and every mutation derives the tab stop from the state of the full group.name, hands the tab stop back to the radios that stay. This applies to plain name-based groups and not only to the ones under anigc-radio-group.checkedsetter.igc-radio-groupadopts the radios that are slotted at run time -name,defaultValueand a pendingvalueall apply to them - and reports the pendingvaluewhile it holds no radios.alignmentasaria-orientation, keeps itsrole, and no longer reports an empty group as disabled.getGroupgive way to one member query.Type of Change
Checklist