From d1e34be2affdecebfe28c9381ac5559746efcb72 Mon Sep 17 00:00:00 2001
From: Charlotte Wickham <cwickham@gmail.com>
Date: Tue, 8 Sep 2026 08:33:45 -0700
Subject: [PATCH] Use the browser focus ring for sidebar section toggles

The sidebar section toggles from #14826 drew their own focus ring
(`outline: 2px solid currentColor` with `outline-offset: 2px`). Every
other control in a Quarto page takes the browser's ring, so tabbing down
the sidebar stepped from a browser ring, to this one, and back again.

The comment on the rule said a bare button gets no ring from Bootstrap's
reboot. Reboot only clears the ring for `:focus:not(:focus-visible)`,
which is the mouse-click case, and these buttons carry no `btn` class,
so nothing suppressed their ring. Measured in Chromium, Firefox and
WebKit: with the rule gone the buttons compute `outline-style: auto`,
and the ring is not clipped by the sidebar.

Delete the rule, state the convention where the one `outline: revert`
rule lives, and record it in the sass-theming rules file. Extend the
focus-indicator spec to cover the section toggles.
---
 .claude/rules/formats/sass-theming.md         | 28 ++++++++++++
 .../html/bootstrap/_bootstrap-rules.scss      | 18 ++++++--
 .../website/navigation/quarto-nav.scss        |  8 ----
 .../website/bare-btn-focus/_quarto.yml        |  3 ++
 .../website/bare-btn-focus/index.qmd          | 12 +++--
 ...n.spec.ts => html-focus-indicator.spec.ts} | 45 ++++++++++++++++---
 6 files changed, 93 insertions(+), 21 deletions(-)
 rename tests/integration/playwright/tests/{html-focus-indicator-bare-btn.spec.ts => html-focus-indicator.spec.ts} (59%)

diff --git a/.claude/rules/formats/sass-theming.md b/.claude/rules/formats/sass-theming.md
index 99f421dc77..a156966942 100644
--- a/.claude/rules/formats/sass-theming.md
+++ b/.claude/rules/formats/sass-theming.md
@@ -6,6 +6,7 @@ paths:
   - "src/format/reveal/format-reveal-theme*"
   - "src/format/dashboard/format-dashboard-shared*"
   - "src/resources/formats/**/*.scss"
+  - "src/resources/projects/**/*.scss"
 ---
 
 # Sass Theming
@@ -22,3 +23,30 @@ background-color: var(--r-background-color, $body-bg);
 Read `llm-docs/sass-theming-architecture.md` for full compilation pipeline details.
 
 For the three-tier callout CSS architecture (Bootstrap, RevealJS, standalone HTML), see `llm-docs/callout-styling-html.md`.
+
+## Focus indicators
+
+Never author a focus ring. The browser's own ring adapts to the platform accent
+color, to the user's accessibility settings, and to forced-colors mode, where
+`box-shadow` is dropped entirely. An authored ring does none of that, and a
+second convention next to the first is visible as an inconsistency when a user
+tabs through a page.
+
+So the only focus rules Quarto writes are ones that undo a vendor rule which
+suppressed the browser ring:
+
+```scss
+// a bare .btn: Bootstrap's .btn:focus-visible sets outline: 0
+.code-tools-button:focus-visible {
+  outline: revert;
+}
+```
+
+A bare `<button>` with no `.btn` class needs no rule at all — nothing suppressed
+its ring. Bootstrap's reboot only clears the ring for `:focus:not(:focus-visible)`,
+which is the mouse-click case.
+
+When a control has no visible focus indicator, look for the suppression and
+delete or revert it, rather than adding a ring of your own. See
+`_bootstrap-rules.scss` for the one `outline: revert` rule, and quarto-cli#14774
+and quarto-cli#12118 for the two times this came up.
diff --git a/src/resources/formats/html/bootstrap/_bootstrap-rules.scss b/src/resources/formats/html/bootstrap/_bootstrap-rules.scss
index 06db8a53d8..e503b1d119 100644
--- a/src/resources/formats/html/bootstrap/_bootstrap-rules.scss
+++ b/src/resources/formats/html/bootstrap/_bootstrap-rules.scss
@@ -1061,9 +1061,21 @@ td code:not(.sourceCode) {
   padding: 0.7rem;
 }
 
-// These buttons are bare .btn elements (no btn-* variant class), so
-// Bootstrap's .btn:focus-visible leaves them with no focus indicator.
-// Restore the browser's ring; unlike box-shadow it survives forced colors.
+// Quarto's focus-indicator convention: never author a focus ring. The
+// browser's own ring adapts to the platform accent color, to the user's
+// accessibility settings, and to forced-colors mode; an authored ring
+// cannot. So the only focus rules Quarto writes are ones that undo a
+// vendor rule which suppressed that ring.
+//
+// This is the one such rule. These three buttons are bare .btn elements
+// (no btn-* variant class), so Bootstrap's .btn:focus-visible sets
+// outline: 0 and substitutes a box-shadow that only the variant classes
+// define — leaving no indicator at all. `revert` restores the browser's.
+//
+// A bare <button> with no .btn class needs no rule: nothing suppressed
+// its ring in the first place. Bootstrap's reboot only clears the ring
+// for :focus:not(:focus-visible), which is the mouse-click case.
+//
 // See https://github.com/quarto-dev/quarto-cli/issues/14774
 .quarto-btn-toggle:focus-visible,
 .quarto-search-button:focus-visible,
diff --git a/src/resources/projects/website/navigation/quarto-nav.scss b/src/resources/projects/website/navigation/quarto-nav.scss
index aa3ba4065f..cd4619936d 100644
--- a/src/resources/projects/website/navigation/quarto-nav.scss
+++ b/src/resources/projects/website/navigation/quarto-nav.scss
@@ -409,14 +409,6 @@ $sidebar-section-bottom-margin: 0.2em;
   padding-left: 0.5rem;
 }
 
-// A bare button gets no focus ring from Bootstrap's reboot, so state one
-// explicitly rather than depend on the UA default (quarto-cli#14774).
-.sidebar-item .sidebar-item-toggle:focus-visible,
-.sidebar-item button.sidebar-item-text:focus-visible {
-  outline: 2px solid currentColor;
-  outline-offset: 2px;
-}
-
 .sidebar-item .sidebar-item-toggle .bi {
   // The dongle for opening and closing sections
   font-size: 0.7rem;
diff --git a/tests/docs/playwright/website/bare-btn-focus/_quarto.yml b/tests/docs/playwright/website/bare-btn-focus/_quarto.yml
index 32a3a3fa63..7ccd0f4a78 100644
--- a/tests/docs/playwright/website/bare-btn-focus/_quarto.yml
+++ b/tests/docs/playwright/website/bare-btn-focus/_quarto.yml
@@ -15,5 +15,8 @@ website:
     contents:
       - index.qmd
       - about.qmd
+      - section: "Group"
+        contents:
+          - about.qmd
 
 format: html
diff --git a/tests/docs/playwright/website/bare-btn-focus/index.qmd b/tests/docs/playwright/website/bare-btn-focus/index.qmd
index 9ea709a618..343b121d45 100644
--- a/tests/docs/playwright/website/bare-btn-focus/index.qmd
+++ b/tests/docs/playwright/website/bare-btn-focus/index.qmd
@@ -2,6 +2,12 @@
 title: "Home"
 ---
 
-Regression test for #14774: below 992px the secondary nav shows the
-sidebar toggle and sidebar search buttons. Both are bare Bootstrap
-`.btn` elements and must keep a visible keyboard focus indicator.
+Regression tests for Quarto's focus-indicator convention.
+
+Below 992px the secondary nav shows the sidebar toggle and sidebar search
+buttons. Both are bare Bootstrap `.btn` elements and must keep a visible
+keyboard focus indicator (#14774).
+
+At full width the sidebar shows a collapsible "Group" section. Its toggle
+is a bare `<button>` with no `.btn` class, and it takes the browser's own
+focus ring with no Quarto rule involved (#14826).
diff --git a/tests/integration/playwright/tests/html-focus-indicator-bare-btn.spec.ts b/tests/integration/playwright/tests/html-focus-indicator.spec.ts
similarity index 59%
rename from tests/integration/playwright/tests/html-focus-indicator-bare-btn.spec.ts
rename to tests/integration/playwright/tests/html-focus-indicator.spec.ts
index 87a4556042..cd4cecb13b 100644
--- a/tests/integration/playwright/tests/html-focus-indicator-bare-btn.spec.ts
+++ b/tests/integration/playwright/tests/html-focus-indicator.spec.ts
@@ -1,13 +1,21 @@
 import { expect, Locator, Page, test } from "@playwright/test";
 import { getUrl } from "../src/utils";
 
-// Regression tests for #14774. Quarto emits three buttons that carry the
-// Bootstrap `btn` class with no `btn-*` variant class: the code tools
-// button, the sidebar toggle, and the sidebar search button. Bootstrap's
-// .btn:focus-visible removes the native focus ring (outline: 0) and
-// substitutes a box-shadow that only the variant classes define, so these
-// buttons took keyboard focus with no visible indicator. Quarto restores
-// the browser's native ring with `outline: revert`.
+// Every Quarto control must show a visible indicator on keyboard focus
+// (WCAG 2.2 SC 2.4.7). Quarto's convention is to use the browser's own
+// ring and never author one, so these tests assert only that an outline
+// is drawn — not which outline. See `.claude/rules/formats/sass-theming.md`.
+//
+// Three buttons carry the Bootstrap `btn` class with no `btn-*` variant
+// class: the code tools button, the sidebar toggle, and the sidebar search
+// button. Bootstrap's .btn:focus-visible removes the native ring
+// (outline: 0) and substitutes a box-shadow that only the variant classes
+// define, so these took keyboard focus with no indicator at all. Quarto
+// restores the browser's ring with `outline: revert` (#14774).
+//
+// The sidebar section toggles are bare `<button>` elements with no `btn`
+// class, so nothing suppresses their ring and Quarto writes no rule for
+// them (#14826).
 
 // Move focus with real Tab presses so the button matches :focus-visible —
 // the fix only applies to keyboard focus, and programmatic locator.focus()
@@ -74,3 +82,26 @@ test.describe("website secondary nav buttons", () => {
     });
   }
 });
+
+test.describe("website sidebar section toggle", () => {
+  // The left sidebar is display: none below the lg breakpoint (992px), so
+  // its section toggles are only focusable at full width.
+  test.use({ viewport: { width: 1400, height: 900 } });
+
+  test("shows a focus indicator on keyboard focus", async ({
+    page,
+    browserName,
+  }) => {
+    await page.goto(getUrl("website/bare-btn-focus/_site/index.html"), {
+      waitUntil: "load",
+    });
+
+    const button = page.locator(
+      "#quarto-sidebar button.sidebar-item-toggle",
+    ).first();
+    await expect(button).toBeVisible();
+    expect(await tabUntilFocused(page, browserName, button)).toBe(true);
+
+    await expect(button).not.toHaveCSS("outline-style", "none");
+  });
+});