From 56ff8513cb9cb224d1fb9938c58b997c04c2f05c Mon Sep 17 00:00:00 2001
From: Guillem Arias Fauste <accounts@gariasf.com>
Date: Wed, 20 May 2026 18:19:17 +0200
Subject: [PATCH] =?UTF-8?q?fix(design-system):=20DS::Tabs=20a11y=20?=
 =?UTF-8?q?=E2=80=94=20WAI-ARIA=20tab=20pattern=20+=20keyboard=20nav=20(#1?=
 =?UTF-8?q?847)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* 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.
---
 app/components/DS/tabs.rb            | 28 +++++++++++++++--
 app/components/DS/tabs/nav.rb        | 28 +++++++++++++----
 app/components/DS/tabs_controller.js | 46 ++++++++++++++++++++++++++--
 3 files changed, 92 insertions(+), 10 deletions(-)

diff --git a/app/components/DS/tabs.rb b/app/components/DS/tabs.rb
index a2c0ff95e..5a0f735be 100644
--- a/app/components/DS/tabs.rb
+++ b/app/components/DS/tabs.rb
@@ -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
diff --git a/app/components/DS/tabs/nav.rb b/app/components/DS/tabs/nav.rb
index 8e795588d..eb77672fa 100644
--- a/app/components/DS/tabs/nav.rb
+++ b/app/components/DS/tabs/nav.rb
@@ -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
diff --git a/app/components/DS/tabs_controller.js b/app/components/DS/tabs_controller.js
index 259765aa7..5089ea63e 100644
--- a/app/components/DS/tabs_controller.js
+++ b/app/components/DS/tabs_controller.js
@@ -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", {