mirror of
https://github.com/we-promise/sure.git
synced 2026-05-29 15:34:58 +00:00
fix(design-system): DS::Tabs a11y — WAI-ARIA tab pattern + keyboard nav (#1847)
* fix(design-system): DS::Tabs a11y — WAI-ARIA tab pattern + keyboard nav Closes #1745. DS::Tabs rendered as a bare `<nav>` + `<button>` list with no role wiring. AT users would hear "navigation, button, button, button" instead of the tab semantics. Keyboard users got no arrow-key nav between tabs. Five fixes: 1. **Role scaffolding.** `<nav>` → `role="tablist"`, `aria-orientation="horizontal"`. Each tab `<button>` → `role="tab"`, `aria-selected`, `aria-controls="panel-#{id}"`. Each panel `<div>` → `role="tabpanel"`, `id="panel-#{tab_id}"`, `aria-labelledby="#{tab_id}"`, `tabindex="0"` (so the panel itself is reachable via keyboard for in-panel content nav). 2. **Roving tabindex.** Active tab is `tabindex="0"`, inactive are `tabindex="-1"`. ArrowLeft/Right cycles focus across the tablist without leaving the widget; Tab jumps past the whole widget. Stimulus controller updates both `aria-selected` and `tabindex` on tab switch. 3. **Manual activation.** Per WAI-ARIA APG "Tabs with Manual Activation" — arrow keys MOVE focus, Enter/Space ACTIVATES the focused tab. Avoids accidental tab swaps when the user is just navigating. Important here because several tab contents trigger Turbo fetches (transactions index, account sidebar, budgets). 4. **Home/End shortcuts.** Home jumps focus to the first tab, End to the last. WAI-ARIA APG-standard. 5. **Raw palette → token.** Replace `bg-white theme-dark:bg-gray-700` on the active button with the existing `tab-item-active` utility (defined in `_generated.css` from `design/tokens/sure.tokens.json`). Single class, dual-mode. Also gate the transition behind `motion-safe:` so reduced-motion users get an instant snap. API unchanged — the slot signatures (`btns(id:, label:)`, `panels(tab_id:)`) take the same args. Caller-provided `id:` is still the public identifier; `panel-#{id}` is internal naming for the `aria-controls`/`aria-labelledby` pair. * fix(review): scope DS::Tabs DOM ids to component instance Per CodeRabbit review on #1847: raw `panel-#{tab_id}` and `id: tab_id` on buttons collide when multiple DS::Tabs widgets on the same page share generic tab ids (e.g., "all", "overview", "transactions"), breaking aria-controls / aria-labelledby associations. Scope ids via per-instance `dom_prefix` ("tabs-#{object_id}") and share the same prefix between DS::Tabs and DS::Tabs::Nav so button ids and panel labelledby/controls stay consistent. * fix(review): use <div> host for role=tablist in DS::Tabs::Nav Codex P2 follow-up on #1847: \`<nav>\` has a fixed landmark role per ARIA-in-HTML and may not be repurposed as a tablist. The current \`tag.nav class: ..., role: \"tablist\"\` produces invalid markup — some AT implementations ignore the role override, in which case the child \`role=\"tab\"\` buttons end up without a valid tablist parent and the keyboard / AT contract this PR is meant to add silently regresses. Swap the container for a neutral \`tag.div\`. Tab semantics (\`role\`, \`aria-orientation\`, keyboard nav, manual-activation pattern) are unchanged.
This commit is contained in:
committed by
GitHub
parent
7a0cafd6ba
commit
56ff8513cb
@@ -5,6 +5,7 @@ class DS::Tabs < DesignSystemComponent
|
||||
active_btn_classes: active_btn_classes,
|
||||
inactive_btn_classes: inactive_btn_classes,
|
||||
btn_classes: base_btn_classes,
|
||||
dom_prefix: dom_prefix,
|
||||
classes: unstyled? ? classes : class_names(nav_container_classes, classes)
|
||||
)
|
||||
end
|
||||
@@ -13,16 +14,35 @@ class DS::Tabs < DesignSystemComponent
|
||||
content_tag(
|
||||
:div,
|
||||
class: ("hidden" unless tab_id == active_tab),
|
||||
role: "tabpanel",
|
||||
id: panel_dom_id(tab_id),
|
||||
"aria-labelledby": tab_dom_id(tab_id),
|
||||
tabindex: "0",
|
||||
data: { id: tab_id, DS__tabs_target: "panel" },
|
||||
&block
|
||||
)
|
||||
end
|
||||
|
||||
# Scope tab/panel DOM ids to this component instance so multiple
|
||||
# `DS::Tabs` widgets on the same page (which often reuse generic
|
||||
# tab ids like "all" or "overview") don't collide and break the
|
||||
# `aria-controls` / `aria-labelledby` associations.
|
||||
def tab_dom_id(tab_id)
|
||||
"#{dom_prefix}-tab-#{tab_id}"
|
||||
end
|
||||
|
||||
def panel_dom_id(tab_id)
|
||||
"#{dom_prefix}-panel-#{tab_id}"
|
||||
end
|
||||
|
||||
VARIANTS = {
|
||||
default: {
|
||||
active_btn_classes: "bg-white theme-dark:bg-gray-700 text-primary shadow-sm",
|
||||
# `tab-item-active` is a Sure token utility (white light / gray-700 dark).
|
||||
# Swapping out the raw `bg-white theme-dark:bg-gray-700` removes the
|
||||
# last raw-palette reference in DS::Tabs.
|
||||
active_btn_classes: "tab-item-active text-primary shadow-sm",
|
||||
inactive_btn_classes: "text-secondary hover:bg-surface-inset-hover",
|
||||
base_btn_classes: "w-full inline-flex justify-center items-center text-sm font-medium px-2 py-1 rounded-md transition-colors duration-200",
|
||||
base_btn_classes: "w-full inline-flex justify-center items-center text-sm font-medium px-2 py-1 rounded-md motion-safe:transition-colors motion-safe:duration-200",
|
||||
nav_container_classes: "flex bg-surface-inset p-1 rounded-lg mb-4"
|
||||
}
|
||||
}
|
||||
@@ -48,6 +68,10 @@ class DS::Tabs < DesignSystemComponent
|
||||
end
|
||||
|
||||
private
|
||||
def dom_prefix
|
||||
@dom_prefix ||= "tabs-#{object_id}"
|
||||
end
|
||||
|
||||
def unstyled?
|
||||
variant == :unstyled
|
||||
end
|
||||
|
||||
@@ -1,6 +1,16 @@
|
||||
class DS::Tabs::Nav < DesignSystemComponent
|
||||
erb_template <<~ERB
|
||||
<%= tag.nav class: classes do %>
|
||||
<%# Neutral `<div>` host for `role="tablist"`. Per ARIA-in-HTML,
|
||||
`<nav>` has a fixed landmark role and may not be repurposed as
|
||||
a tablist — some AT implementations ignore the override and
|
||||
the child `role="tab"` elements end up parentless. The tab
|
||||
pattern is its own widget per WAI-ARIA APG; keyboard nav
|
||||
(ArrowLeft/Right, Home, End, Enter/Space) is driven by the
|
||||
Stimulus controller with the manual-activation pattern
|
||||
(focus moves first, activate on Enter/Space). %>
|
||||
<%= tag.div class: classes,
|
||||
role: "tablist",
|
||||
"aria-orientation": "horizontal" do %>
|
||||
<% btns.each do |btn| %>
|
||||
<%= btn %>
|
||||
<% end %>
|
||||
@@ -8,19 +18,25 @@ class DS::Tabs::Nav < DesignSystemComponent
|
||||
ERB
|
||||
|
||||
renders_many :btns, ->(id:, label:, classes: nil, &block) do
|
||||
is_active = id == active_tab
|
||||
content_tag(
|
||||
:button, label, id: id,
|
||||
:button, label, id: "#{dom_prefix}-tab-#{id}",
|
||||
type: "button",
|
||||
class: class_names(btn_classes, id == active_tab ? active_btn_classes : inactive_btn_classes, classes),
|
||||
data: { id: id, action: "DS--tabs#show", DS__tabs_target: "navBtn" },
|
||||
class: class_names(btn_classes, is_active ? active_btn_classes : inactive_btn_classes, classes),
|
||||
role: "tab",
|
||||
"aria-selected": is_active.to_s,
|
||||
"aria-controls": "#{dom_prefix}-panel-#{id}",
|
||||
tabindex: is_active ? "0" : "-1",
|
||||
data: { id: id, action: "click->DS--tabs#show keydown->DS--tabs#handleKeydown", DS__tabs_target: "navBtn" },
|
||||
&block
|
||||
)
|
||||
end
|
||||
|
||||
attr_reader :active_tab, :classes, :active_btn_classes, :inactive_btn_classes, :btn_classes
|
||||
attr_reader :active_tab, :classes, :active_btn_classes, :inactive_btn_classes, :btn_classes, :dom_prefix
|
||||
|
||||
def initialize(active_tab:, classes: nil, active_btn_classes: nil, inactive_btn_classes: nil, btn_classes: nil)
|
||||
def initialize(active_tab:, dom_prefix:, classes: nil, active_btn_classes: nil, inactive_btn_classes: nil, btn_classes: nil)
|
||||
@active_tab = active_tab
|
||||
@dom_prefix = dom_prefix
|
||||
@classes = classes
|
||||
@active_btn_classes = active_btn_classes
|
||||
@inactive_btn_classes = inactive_btn_classes
|
||||
|
||||
@@ -11,13 +11,19 @@ export default class extends Controller {
|
||||
const selectedTabId = btn.dataset.id;
|
||||
|
||||
this.navBtnTargets.forEach((navBtn) => {
|
||||
if (navBtn.dataset.id === selectedTabId) {
|
||||
const isSelected = navBtn.dataset.id === selectedTabId;
|
||||
if (isSelected) {
|
||||
navBtn.classList.add(...this.navBtnActiveClasses);
|
||||
navBtn.classList.remove(...this.navBtnInactiveClasses);
|
||||
} else {
|
||||
navBtn.classList.add(...this.navBtnInactiveClasses);
|
||||
navBtn.classList.remove(...this.navBtnActiveClasses);
|
||||
}
|
||||
// Roving tabindex per WAI-ARIA APG: only the active tab is in
|
||||
// the tab order. ArrowLeft/Right (see handleKeydown) moves focus
|
||||
// across the tablist; Tab moves past the widget.
|
||||
navBtn.setAttribute("aria-selected", isSelected.toString());
|
||||
navBtn.setAttribute("tabindex", isSelected ? "0" : "-1");
|
||||
});
|
||||
|
||||
this.panelTargets.forEach((panel) => {
|
||||
@@ -38,7 +44,43 @@ export default class extends Controller {
|
||||
if (this.sessionKeyValue) {
|
||||
this.#updateSessionPreference(selectedTabId);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// WAI-ARIA APG "Tabs with Manual Activation" — arrow keys move
|
||||
// focus, Enter/Space activates. Prevents accidental tab swap when
|
||||
// tabbing through, which is important here because some tab
|
||||
// contents trigger Turbo fetches.
|
||||
handleKeydown(e) {
|
||||
const navBtns = this.navBtnTargets;
|
||||
const currentIndex = navBtns.indexOf(e.target);
|
||||
if (currentIndex === -1) return;
|
||||
|
||||
let nextIndex = null;
|
||||
switch (e.key) {
|
||||
case "ArrowRight":
|
||||
nextIndex = (currentIndex + 1) % navBtns.length;
|
||||
break;
|
||||
case "ArrowLeft":
|
||||
nextIndex = (currentIndex - 1 + navBtns.length) % navBtns.length;
|
||||
break;
|
||||
case "Home":
|
||||
nextIndex = 0;
|
||||
break;
|
||||
case "End":
|
||||
nextIndex = navBtns.length - 1;
|
||||
break;
|
||||
case "Enter":
|
||||
case " ":
|
||||
e.preventDefault();
|
||||
this.show(e);
|
||||
return;
|
||||
default:
|
||||
return;
|
||||
}
|
||||
|
||||
e.preventDefault();
|
||||
navBtns[nextIndex].focus();
|
||||
}
|
||||
|
||||
#updateSessionPreference(selectedTabId) {
|
||||
fetch("/current_session", {
|
||||
|
||||
Reference in New Issue
Block a user