feat(button-group): add roving focus and move the group ARIA to the host

[rkaraivanov]

Description

The single selection modes expose radio semantics but had no keyboard handling: every toggle button was its own tab stop and the arrow keys did nothing, which the radiogroup role does not allow. The group now runs a roving tab index through addRovingFocusController, with the selected button - or the first enabled one - as the single tab stop, and arrow navigation that carries the selection along, skips disabled buttons and wraps. The alignment picks the axis and the horizontal one follows the writing direction. The multiple selection mode keeps a tab stop per button.

The group resolves the tab stop and publishes it on ButtonGroupContext; each button applies it to its own host, because its shadow root delegates focus and an inner tabindex="-1" would leave the host in the tab order. syncSelection becomes syncState, covering both selected and disabled.

The role and the disabled state move from an element of the shadow root onto the host through addInternalsController, matching the other container components, so that an author's aria-label names the group.

Also collapses the two selection event emitters into one writer, resolves the tab stop once per interaction instead of once per emitted event, and drops a dead selected-items reset and a slot guard the slot controller already provides.

Type of Change

• New feature (non-breaking change that adds functionality)

Checklist

• My code follows the project's coding standards
• I have tested my changes locally

[Copilot]

🟡 Changes recommended

Resolve the missing aria-orientation and restore tabindex when group ownership ends.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds roving focus, keyboard navigation, and host-level ARIA semantics to single-selection button groups.

Changes:

• Adds selection-aware tab stops and arrow-key navigation.
• Moves group roles and disabled state to the host.
• Updates context, tests, and changelog.

File summaries

File	Description
src/internals/context.ts	Extends button-group context state.
src/components/button-group/toggle-button.ts	Applies group tab-stop state.
src/components/button-group/button-group.ts	Implements roving focus and host ARIA semantics.
src/components/button-group/button-group.spec.ts	Adds accessibility and keyboard-navigation coverage.
CHANGELOG.md	Documents the new behavior.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

Five moderate findings remain in focus, context, and tabindex lifecycle handling.

Review details

Suppressed comments (5)

src/components/button-group/button-group.ts:307

• When a non-tab-stop button is moved from this group to another still-connected parent, disconnectedCallback never runs. If _tabStop remains unchanged, _updateTabStop() does not publish, so the moved button keeps the group's tabindex="-1" and remains excluded from the tab order outside the group. Republish the context or otherwise update/restore buttons whenever slot membership changes, even when the selected tab stop itself is unchanged.

  private _handleSlotChange(): void {
    this._enforceSingleSelection();
    this._updateTabStop();

src/components/button-group/button-group.ts:268

• getRoot(this).activeElement resolves the active element in the containing root, not in this component's shadow root. When the group is used inside another shadow root, that value is the igc-button-group host rather than the focused slotted toggle button, so _getFocusedButton() returns null and all arrow navigation becomes a no-op. Resolve the focused item from the group's own focus chain (for example, by checking each toggle button's delegated shadow-root focus) instead.

    const button = getRoot(this).activeElement?.closest(
      IgcToggleButtonComponent.tagName
    );

src/components/button-group/toggle-button.ts:80

• This cleanup breaks detach/reattach of a non-tab-stop button. The async context is retained across the detach, but reconnecting does not trigger willUpdate; if _tabStop did not change, the provider never republishes, so the button returns without tabindex="-1" and becomes a second tab stop. Reconcile the managed tabindex on reconnect (or otherwise republish the group state after reattachment).

  public override disconnectedCallback(): void {
    if (this._context.value) {
      this.removeAttribute('tabindex');
    }

src/components/button-group/toggle-button.ts:80

• The group overwrites the host's tabindex while the button is slotted, but this unconditional removal loses any tabindex supplied by the author before the button joined the group. After removal, a button that started with tabindex="-1" (or another explicit value) is no longer in its original tab order, contradicting the standalone behavior described below. Preserve and restore the pre-group value instead of always removing the attribute.

  public override disconnectedCallback(): void {
    if (this._context.value) {
      this.removeAttribute('tabindex');
    }

src/components/button-group/toggle-button.ts:82

• A button moved from one group to another remains bound to the first AsyncContextConsumer: its hostConnected() returns early when a consumer already exists, so it never requests the new group's context. After this callback removes the old tabindex, the new group cannot publish its roving tab-stop state to the button, and state callbacks still target the old group. The context membership must be refreshed on reconnect/reparenting.

  public override disconnectedCallback(): void {
    if (this._context.value) {
      this.removeAttribute('tabindex');
    }

    super.disconnectedCallback();

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Fix the stale-context tabindex mutation after disconnection; also correct the contradictory test comment.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

src/components/button-group/button-group.spec.ts:1076

• This test comment contradicts the expected value and the ARIA default: a radiogroup with no aria-orientation is treated as horizontal, not vertical. Please correct the comment so it does not document the opposite behavior.

      // A `radiogroup` that announces no orientation is taken to be vertical.

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Two moderate keyboard-navigation issues remain unresolved.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/components/button-group/button-group.ts:268

• When the group itself is inside another shadow root, getRoot(this).activeElement resolves to the group host rather than the focused toggle button (the added nested-shadow-root test exercises this case). Consequently closest('igc-toggle-button') returns null and every arrow key becomes a no-op. Resolve the focused button from the group's assigned buttons (for example with :focus) before falling back to the containing root.

src/components/button-group/button-group.ts:188

• The custom skip callback replaces the key-bindings controller's default input/textarea/select skip, but only checks the group state. Arrow events from any slotted descendant that is not one of the group's direct buttons therefore reach this controller, are preventDefault()ed by bindingDefaults, and then no-op because _getFocusedButton() returns null; this breaks native keyboard behavior for nested controls (and for the explicitly unsupported nested toggle buttons). Restrict the callback to events whose composed path contains one of _buttons.

        skip: () => this.disabled || this._isMultiple,
        bindingDefaults: { preventDefault: true, repeat: true },

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Lite