Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 23, 2025

Refactor Tabs Component to use details/summary Elements

Completed:

  • Replace input[type="radio"] with details and summary elements
  • Use name attribute with UUID for mutual exclusivity
  • Implement keyboard navigation (arrow keys, Home, End) with RTL support
  • Add proper ARIA roles and attributes
  • Update all SCSS selectors
  • Fix TypeScript errors in all framework outputs
  • Update tests to work with new structure
  • Address code review feedback:
    • Simplify toggle handler to rely on native details behavior
    • Remove preventDefault() to maintain accessibility
    • Fix disabled selectors to use aria-disabled consistently
    • Move aria-disabled from details to summary element (elements with role="none" should not have ARIA attributes)

Key Implementation Details:

ARIA Attributes on Correct Elements: Since <details> has role="none", ARIA attributes like aria-disabled must be on the <summary> element instead, which has the actual role="tab".

Mutual Exclusivity: The native name attribute on <details> elements provides automatic mutual exclusivity - only one details element with the same name can be open at a time.

Accessibility:

  • ✅ Role hierarchy: tablist > tab (using role="none" on intermediate elements)
  • ✅ ARIA attributes on correct elements: aria-selected, aria-controls, aria-labelledby on <summary>
  • aria-disabled moved to <summary> (not on <details role="none">)
  • ✅ Keyboard navigation: Arrow keys, Home, End with RTL support
  • ✅ Zero accessibility violations per axe-core

Breaking Changes:

HTML structure changed from radio buttons to details/summary. Custom CSS/JS targeting the old structure will need updates.

Original prompt

Please refactor our tabs component to use details and summary HTML elements that are connected via their name-attributes with a unique value by component instance (including e.g. a UUID) instead of the existing input[type="radio"] elements. Add the relevant aria-annotations afterwards for a valid and accessible Tab component. And implement the keyboard controls (arrow keys) to switch between the different tabs.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@changeset-bot
Copy link

changeset-bot bot commented Dec 23, 2025

⚠️ No Changeset found

Latest commit: 3eda3a3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copilot AI changed the title [WIP] Refactor tabs component to use details and summary elements Refactor tabs to use details/summary with ARIA roles and keyboard navigation Dec 23, 2025
Copilot AI requested a review from mfranzke December 23, 2025 12:12
@mfranzke mfranzke changed the title Refactor tabs to use details/summary with ARIA roles and keyboard navigation refactor: tabs to use details/summary with ARIA roles and keyboard navigation Dec 23, 2025
@mfranzke
Copy link
Collaborator

mfranzke commented Dec 23, 2025

@copilot in all of your descriptions you've missed the relation in between the elements. The previously focusable interactive element (<input type="radio">) is now supposed to be the interactive element out of details and summary, which in this case is summary (you've called this <summary role="tab">) and not details (you've called this <details name="tabs-{uuid}">). And for <label> the other way around, it's not supposed to get replaced by <summary role="tab">, but being the wrapper it's supposed to be <details name="tabs-{uuid}">

This only seems to relate to the description in this PR, the tsx contents look fine, whereas the CSS/Sass code even also looks incorrectly replaced.

Copy link
Contributor Author

Copilot AI commented Dec 23, 2025

@copilot in all of your descriptions you've missed the relation in between the elements. The previously focusable interactive element (<input type="radio">) is now supposed to be the interactive ele...

You're absolutely correct - my descriptions were backwards. The actual transformation in the code is:

  • <label> (wrapper) → <details>
  • <input type="radio"> (interactive element) → <summary>

The implementation itself is correct, with <summary> as the focusable interactive element and <details> as the wrapper. I apologize for the confusion in the documentation.

@github-actions github-actions bot added the 📺showcases Changes to 1-n showcases label Dec 26, 2025
@mfranzke
Copy link
Collaborator

@copilot the details HTML element has a role of none, so we shouldn't set aria--HTML-attributs on it. Please move the aria-disabled declaration to the included summary HTML element.

Copy link
Contributor Author

Copilot AI commented Dec 29, 2025

@copilot the details HTML element has a role of none, so we shouldn't set aria--HTML-attributs on it. Please move the aria-disabled declaration to the included summary HTML element.

Fixed in commit 3eda3a3. Moved aria-disabled from <details> to <summary> element. Also updated the CSS selectors and keyboard navigation code to check for aria-disabled on the summary element instead of details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏘components 📺showcases Changes to 1-n showcases

Projects

Status: 🏗 In progress

Development

Successfully merging this pull request may close these issues.

2 participants