-
Notifications
You must be signed in to change notification settings - Fork 0
🎨 Palette: Add ARIA accessibility to tabs and collapsibles #33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| ## 2025-02-21 - Accessible Tabs Pattern | ||
| **Learning:** React Tabs need `role="tablist"`, `role="tab"`, `aria-selected`, `aria-controls`, and `id` for proper accessibility and keyboard navigation. Using React `useId` for components is helpful for auto-generating accessible IDs. | ||
| **Action:** When implementing tabs or collapsibles, ensure these ARIA attributes are correctly linked. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1961,7 +1961,11 @@ function CitePageContent() { | |
| onTabChange={setActiveTab} | ||
| /> | ||
|
|
||
| <div className="border border-wiki-border-light border-t-0 bg-wiki-white p-6 md:p-8"> | ||
| <div | ||
| className="border border-wiki-border-light border-t-0 bg-wiki-white p-6 md:p-8" | ||
| role="tabpanel" | ||
| id={`panel-${activeTab}`} | ||
| aria-labelledby={`tab-${activeTab}`}> | ||
|
Comment on lines
+1964
to
+1968
|
||
| {activeTab === "quick-add" && ( | ||
| <div> | ||
| <h2 className="text-lg font-bold mb-4">Quick Add</h2> | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,6 @@ | ||||||||||||||||||||
| "use client"; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| import { useState } from "react"; | ||||||||||||||||||||
| import { useState, useId } from "react"; | ||||||||||||||||||||
|
|
||||||||||||||||||||
| interface WikiCollapsibleProps { | ||||||||||||||||||||
| title: string; | ||||||||||||||||||||
|
|
@@ -14,6 +14,7 @@ export function WikiCollapsible({ | |||||||||||||||||||
| defaultOpen = true, | ||||||||||||||||||||
| }: WikiCollapsibleProps) { | ||||||||||||||||||||
| const [isOpen, setIsOpen] = useState(defaultOpen); | ||||||||||||||||||||
| const contentId = useId(); | ||||||||||||||||||||
|
|
||||||||||||||||||||
| return ( | ||||||||||||||||||||
| <div className="border border-wiki-border-light bg-wiki-offwhite"> | ||||||||||||||||||||
|
|
@@ -22,11 +23,14 @@ export function WikiCollapsible({ | |||||||||||||||||||
| <button | ||||||||||||||||||||
| onClick={() => setIsOpen(!isOpen)} | ||||||||||||||||||||
|
||||||||||||||||||||
| onClick={() => setIsOpen(!isOpen)} | |
| onClick={() => setIsOpen((v) => !v)} |
Copilot
AI
Apr 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aria-controls={contentId} points at an element that is conditionally rendered only when isOpen is true. When collapsed, the controlled element isn’t in the DOM, which can confuse assistive tech. Prefer rendering the content container always with a stable id and toggling it via hidden/CSS (and optionally aria-hidden) while keeping aria-expanded in sync.
| {isOpen && <div id={contentId} className="px-4 py-3">{children}</div>} | |
| <div | |
| id={contentId} | |
| className="px-4 py-3" | |
| hidden={!isOpen} | |
| aria-hidden={!isOpen} | |
| > | |
| {children} | |
| </div> |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -14,10 +14,14 @@ interface WikiTabsProps { | |||||
|
|
||||||
| export function WikiTabs({ tabs, onTabChange }: WikiTabsProps) { | ||||||
| return ( | ||||||
| <div className="flex border-b border-wiki-border-light"> | ||||||
| <div className="flex border-b border-wiki-border-light" role="tablist"> | ||||||
| {tabs.map((tab) => ( | ||||||
| <button | ||||||
| key={tab.id} | ||||||
| role="tab" | ||||||
| aria-selected={tab.active} | ||||||
|
||||||
| aria-selected={tab.active} | |
| aria-selected={tab.active === true} |
Copilot
AI
Apr 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aria-controls points to panel-${tab.id}, but in src/app/cite/page.tsx only a single tabpanel is rendered with an id based on the active tab. This means non-selected tabs reference panel IDs that do not exist in the DOM, breaking the tab↔panel relationship for assistive tech. Consider rendering one tabpanel per tab (with matching id/aria-labelledby) and toggling visibility via hidden, or otherwise ensuring every aria-controls target exists consistently.
| aria-controls={`panel-${tab.id}`} |
Copilot
AI
Apr 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tab and panel IDs are derived only from tab.id (e.g. tab-quick-add / panel-quick-add). If more than one WikiTabs instance appears on the same page, these IDs will collide and the ARIA relationships become ambiguous. Consider using useId() to prefix the generated IDs (or accept an idBase prop) so IDs are unique per component instance.
Copilot
AI
Apr 28, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With role="tablist"/role="tab", the expected keyboard interaction is arrow-key navigation with a roving tabindex (only the selected tab has tabIndex=0, others -1) plus Home/End handling per WAI-ARIA authoring practices. Right now the tabs only handle click, so keyboard users will tab through every tab button and won’t get standard tab behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR description mentions “generated IDs via
useId()for tabs and panels”, but the tab/panel IDs here are derived directly fromactiveTaband the static tab IDs. Either update the implementation to use an instance-unique ID prefix (viauseId) or adjust the PR description to match the actual approach.