feat(focus-rings): branded focus ring tokens for light and dark mode #167

New Issue

marcel · 2026-03-31T10:19:57+02:00

marcel commented

2026-03-31 10:19:57 +02:00

Background

Surfaced during the UI/UX review of #166 (dark mode redesign). Most interactive elements currently use browser-default focus rings, with the exception of the header which has a branded focus-visible:ring-accent (--c-accent: #00c7b1).

Browser defaults have improved and are acceptable as a stopgap, but a consistent branded focus ring system is needed — especially once dark mode ships, since the correct ring color differs between light and dark contexts.

Problem

There is no --c-focus-ring token in the design system. Focus ring colors are either:

Browser default (most elements)
Hardcoded to --c-accent on the header

This means:

No consistent visual language across the app
No dark mode aware focus ring behavior
Future components will each need to decide independently, leading to drift

Proposed Solution

Add a --c-focus-ring semantic token that resolves to the correct color per mode:

Mode	Value	Contrast on typical background	WCAG
Light	`#012851` (brand-navy)	~14:1 on white, ~10:1 on sand	AAA ✓
Dark	`#a1dcd8` (brand-mint)	9.2:1 on `#010e1e` canvas	AAA ✓

CSS changes (layout.css):

/* Light mode — :root */
--c-focus-ring: #012851;

/* @theme inline */
--color-focus-ring: var(--c-focus-ring);

/* Dark mode — both blocks */
--c-focus-ring: #a1dcd8;

Usage in components:
Replace focus-visible:ring-accent (header) and any other focus ring utilities with focus-visible:ring-focus-ring. Browser defaults can be overridden via:

:focus-visible {
  outline-color: var(--c-focus-ring);
}

Or with Tailwind: focus-visible:ring-focus-ring focus-visible:ring-2 focus-visible:ring-offset-2

Scope

Add --c-focus-ring token to light and dark mode blocks in layout.css
Add --color-focus-ring mapping in @theme inline
Update the header's focus-visible:ring-accent → focus-visible:ring-focus-ring
Audit all other interactive elements (buttons, links, inputs, tags, chips) and apply consistent focus-visible:ring-focus-ring class

Acceptance Criteria

--c-focus-ring token defined in both light and dark mode CSS blocks
--color-focus-ring mapped in @theme inline
Header focus ring updated to use the new token
All interactive elements audited — buttons, links, inputs, PersonTypeahead, TagInput, PersonMultiSelect
Focus ring visible at �� 3:1 contrast against its immediate background in both modes (WCAG 2.4.11)
Verified with keyboard navigation across all major pages in both light and dark mode

Identified during #166 UI/UX discussion

## Background Surfaced during the UI/UX review of #166 (dark mode redesign). Most interactive elements currently use **browser-default focus rings**, with the exception of the header which has a branded `focus-visible:ring-accent` (`--c-accent: #00c7b1`). Browser defaults have improved and are acceptable as a stopgap, but a consistent branded focus ring system is needed — especially once dark mode ships, since the correct ring color differs between light and dark contexts. --- ## Problem There is no `--c-focus-ring` token in the design system. Focus ring colors are either: - Browser default (most elements) - Hardcoded to `--c-accent` on the header This means: - No consistent visual language across the app - No dark mode aware focus ring behavior - Future components will each need to decide independently, leading to drift --- ## Proposed Solution Add a `--c-focus-ring` semantic token that resolves to the correct color per mode: | Mode | Value | Contrast on typical background | WCAG | |---|---|---|---| | Light | `#012851` (brand-navy) | ~14:1 on white, ~10:1 on sand | AAA ✓ | | Dark | `#a1dcd8` (brand-mint) | 9.2:1 on `#010e1e` canvas | AAA ✓ | **CSS changes (`layout.css`):** ```css /* Light mode — :root */ --c-focus-ring: #012851; /* @theme inline */ --color-focus-ring: var(--c-focus-ring); /* Dark mode — both blocks */ --c-focus-ring: #a1dcd8; ``` **Usage in components:** Replace `focus-visible:ring-accent` (header) and any other focus ring utilities with `focus-visible:ring-focus-ring`. Browser defaults can be overridden via: ```css :focus-visible { outline-color: var(--c-focus-ring); } ``` Or with Tailwind: `focus-visible:ring-focus-ring focus-visible:ring-2 focus-visible:ring-offset-2` --- ## Scope - Add `--c-focus-ring` token to light and dark mode blocks in `layout.css` - Add `--color-focus-ring` mapping in `@theme inline` - Update the header's `focus-visible:ring-accent` → `focus-visible:ring-focus-ring` - Audit all other interactive elements (buttons, links, inputs, tags, chips) and apply consistent `focus-visible:ring-focus-ring` class --- ## Acceptance Criteria - [ ] `--c-focus-ring` token defined in both light and dark mode CSS blocks - [ ] `--color-focus-ring` mapped in `@theme inline` - [ ] Header focus ring updated to use the new token - [ ] All interactive elements audited — buttons, links, inputs, `PersonTypeahead`, `TagInput`, `PersonMultiSelect` - [ ] Focus ring visible at �� 3:1 contrast against its immediate background in both modes (WCAG 2.4.11) - [ ] Verified with keyboard navigation across all major pages in both light and dark mode ## Related - Identified during #166 UI/UX discussion

marcel added the feature ui labels 2026-03-31 10:20:03 +02:00

marcel commented

2026-03-31 10:36:38 +02:00

👨‍💻 Felix Brandt — Senior Fullstack Developer

Questions & Observations

Global CSS vs. per-component Tailwind? The issue proposes two approaches: a global :focus-visible { outline-color: var(--c-focus-ring); } rule, and per-component focus-visible:ring-focus-ring Tailwind classes. These are mutually exclusive in practice — mixing them creates specificity conflicts. Which approach wins? I'd lean toward the global CSS rule for built-in elements (<a>, <button>, <input>) and Tailwind classes only for custom components (chips, typeahead). Committing to one strategy prevents drift.
Scope of the audit: Buttons, links, inputs are listed — but what about <select>, <textarea>, <details>/<summary> (if used), and anything with tabindex? One pass with axe or a manual Tab sweep will turn up surprises.
TDD hook: This is a CSS/token change. The "test" here is a Playwright keyboard-nav pass, not a unit test. I'll write the failing Playwright test first (tab through a page, assert the focused element has a visible ring via getByRole + focus assertion), then apply the token. Without that test, we have no regression gate.
Dead code risk: After replacing focus-visible:ring-accent on the header, check that ring-accent is not still referenced elsewhere in layout or component files. A grep -r "ring-accent" before merging is cheap insurance.

Suggestions

Define the strategy in a single comment in layout.css right above the token — one line explaining why --c-focus-ring exists and which mechanism uses it. Future contributors will thank you.
After the audit, squash all component changes into one commit separate from the CSS token commit. Keeps the diff reviewable.

## 👨‍💻 Felix Brandt — Senior Fullstack Developer ### Questions & Observations - **Global CSS vs. per-component Tailwind?** The issue proposes two approaches: a global `:focus-visible { outline-color: var(--c-focus-ring); }` rule, and per-component `focus-visible:ring-focus-ring` Tailwind classes. These are mutually exclusive in practice — mixing them creates specificity conflicts. Which approach wins? I'd lean toward the **global CSS rule** for built-in elements (`<a>`, `<button>`, `<input>`) and Tailwind classes only for custom components (chips, typeahead). Committing to one strategy prevents drift. - **Scope of the audit:** Buttons, links, inputs are listed — but what about `<select>`, `<textarea>`, `<details>`/`<summary>` (if used), and anything with `tabindex`? One pass with `axe` or a manual Tab sweep will turn up surprises. - **TDD hook:** This is a CSS/token change. The "test" here is a Playwright keyboard-nav pass, not a unit test. I'll write the failing Playwright test first (tab through a page, assert the focused element has a visible ring via `getByRole` + focus assertion), then apply the token. Without that test, we have no regression gate. - **Dead code risk:** After replacing `focus-visible:ring-accent` on the header, check that `ring-accent` is not still referenced elsewhere in layout or component files. A `grep -r "ring-accent"` before merging is cheap insurance. ### Suggestions - Define the strategy in a single comment in `layout.css` right above the token — one line explaining why `--c-focus-ring` exists and which mechanism uses it. Future contributors will thank you. - After the audit, squash all component changes into one commit separate from the CSS token commit. Keeps the diff reviewable.

marcel commented

2026-03-31 10:36:56 +02:00

🏛️ Markus Keller — Application Architect

Questions & Observations

Token naming consistency: The existing system uses --c-* for raw color tokens and --color-* for Tailwind mappings. The proposed --c-focus-ring fits this pattern correctly. One thing to verify: is focus-ring the right semantic name, or should it be --c-focus (shorter, parallels e.g. --c-accent)? Once named, it will appear in many components — worth a deliberate choice now.
Global rule vs. utility class — the architectural question: A global :focus-visible { outline-color: var(--c-focus-ring); } in layout.css gives you coverage of all native elements for free and is the simpler system. The downside: it relies on outline, while the current header uses ring (box-shadow-based in Tailwind), which has different rendering across browsers and zoom levels. The issue should explicitly pick one mechanism and document why.
Dark mode block placement: The issue says "add to both blocks." The layout.css presumably has two dark mode contexts — @media (prefers-color-scheme: dark) and a .dark class selector for explicit override. Both must define --c-focus-ring. If only one is set, users whose OS is in dark mode but who haven't toggled the app will get the wrong color. Confirm both paths are covered.
@theme inline boundary: The --color-focus-ring: var(--c-focus-ring) mapping in @theme inline means the Tailwind utility focus-visible:ring-focus-ring works. Just make sure ring-focus-ring resolves correctly in the Tailwind 4 config — in TW4, @theme inline custom properties feed ring-* utilities directly, but verify with a quick compile check.

Suggestions

Add a one-line comment in the @theme inline block noting that --color-focus-ring is the only externally consumable token; --c-focus-ring is internal. This prevents someone from reaching for the internal token from component code.
Consider whether other "system" focus-adjacent tokens belong in the same block (e.g. --c-focus-ring-offset for the background color of the ring offset). Doing it now is cheaper than retrofitting it for every button variant later.

## 🏛️ Markus Keller — Application Architect ### Questions & Observations - **Token naming consistency:** The existing system uses `--c-*` for raw color tokens and `--color-*` for Tailwind mappings. The proposed `--c-focus-ring` fits this pattern correctly. One thing to verify: is `focus-ring` the right semantic name, or should it be `--c-focus` (shorter, parallels e.g. `--c-accent`)? Once named, it will appear in many components — worth a deliberate choice now. - **Global rule vs. utility class — the architectural question:** A global `:focus-visible { outline-color: var(--c-focus-ring); }` in `layout.css` gives you coverage of all native elements for free and is the simpler system. The downside: it relies on `outline`, while the current header uses `ring` (box-shadow-based in Tailwind), which has different rendering across browsers and zoom levels. The issue should explicitly pick one mechanism and document why. - **Dark mode block placement:** The issue says "add to both blocks." The `layout.css` presumably has two dark mode contexts — `@media (prefers-color-scheme: dark)` and a `.dark` class selector for explicit override. Both must define `--c-focus-ring`. If only one is set, users whose OS is in dark mode but who haven't toggled the app will get the wrong color. Confirm both paths are covered. - **`@theme inline` boundary:** The `--color-focus-ring: var(--c-focus-ring)` mapping in `@theme inline` means the Tailwind utility `focus-visible:ring-focus-ring` works. Just make sure `ring-focus-ring` resolves correctly in the Tailwind 4 config — in TW4, `@theme inline` custom properties feed `ring-*` utilities directly, but verify with a quick compile check. ### Suggestions - Add a one-line comment in the `@theme inline` block noting that `--color-focus-ring` is the only externally consumable token; `--c-focus-ring` is internal. This prevents someone from reaching for the internal token from component code. - Consider whether other "system" focus-adjacent tokens belong in the same block (e.g. `--c-focus-ring-offset` for the background color of the ring offset). Doing it now is cheaper than retrofitting it for every button variant later.

marcel commented

2026-03-31 10:37:19 +02:00

🧪 Sara Holt — QA Engineer & Test Strategist

Questions & Observations

The acceptance criteria say "verified with keyboard navigation" — who verifies and how? Manual keyboard testing is not a quality gate; it's a one-time check that silently regresses. This feature needs automated Playwright assertions to stay green across all future PRs.
No test plan in the issue. For a cross-cutting change affecting every interactive element, the test surface is non-trivial. Before implementation starts, we should agree on: which pages get Playwright focus tests, and at which breakpoints.
Light mode AND dark mode in CI: The acceptance criterion covers both modes. A Playwright run that only tests light mode misses half the spec. The CI pipeline needs to run the focus ring tests in both modes — either via --theme=dark flag or a prefers-color-scheme: dark emulation in the Playwright config.
WCAG 2.4.11 contrast verification: The issue cites the contrast ratios from the spec, which is great. But how do we verify them programmatically? axe-playwright's checkA11y does not currently check WCAG 2.4.11 focus appearance (it checks 2.4.7 and 1.4.11 but not the newer criterion). Manual spot-check with a contrast analyzer is needed, or a custom assertion.
Edge case: elements that hardcode outline: none — any component that has outline-none in its class list will silently swallow the global rule. The audit must specifically look for outline-none and ensure each occurrence has an explicit focus-visible:ring-focus-ring replacement.

Suggested Test Plan (layer by layer)

Static: grep -r "outline-none\|outline: none" in src/ as a pre-commit check
Unit: N/A — CSS token changes don't have a meaningful unit test layer
E2E (Playwright):
- should show branded focus ring on primary button (light + dark)
- should show branded focus ring on nav links in header (light + dark)
- should show branded focus ring on PersonTypeahead input (light + dark)
- should show branded focus ring on tag chips (light + dark)
- Run checkA11y on: home, document detail, edit, persons list, admin pages
- Visual regression snapshots at 1280px of focused states in both modes

Suggestions

Add the Playwright focus tests as acceptance criteria with specific test names, not just "verified manually."
Run Playwright with colorScheme: 'dark' in a separate test project in playwright.config.ts — low-overhead way to guarantee dark mode coverage on every PR.

## 🧪 Sara Holt — QA Engineer & Test Strategist ### Questions & Observations - **The acceptance criteria say "verified with keyboard navigation" — who verifies and how?** Manual keyboard testing is not a quality gate; it's a one-time check that silently regresses. This feature needs automated Playwright assertions to stay green across all future PRs. - **No test plan in the issue.** For a cross-cutting change affecting every interactive element, the test surface is non-trivial. Before implementation starts, we should agree on: which pages get Playwright focus tests, and at which breakpoints. - **Light mode AND dark mode in CI:** The acceptance criterion covers both modes. A Playwright run that only tests light mode misses half the spec. The CI pipeline needs to run the focus ring tests in both modes — either via `--theme=dark` flag or a `prefers-color-scheme: dark` emulation in the Playwright config. - **WCAG 2.4.11 contrast verification:** The issue cites the contrast ratios from the spec, which is great. But how do we verify them programmatically? `axe-playwright`'s `checkA11y` does not currently check WCAG 2.4.11 focus appearance (it checks 2.4.7 and 1.4.11 but not the newer criterion). Manual spot-check with a contrast analyzer is needed, or a custom assertion. - **Edge case: elements that hardcode `outline: none`** — any component that has `outline-none` in its class list will silently swallow the global rule. The audit must specifically look for `outline-none` and ensure each occurrence has an explicit `focus-visible:ring-focus-ring` replacement. ### Suggested Test Plan (layer by layer) - **Static**: `grep -r "outline-none\|outline: none"` in `src/` as a pre-commit check - **Unit**: N/A — CSS token changes don't have a meaningful unit test layer - **E2E (Playwright)**: - `should show branded focus ring on primary button` (light + dark) - `should show branded focus ring on nav links in header` (light + dark) - `should show branded focus ring on PersonTypeahead input` (light + dark) - `should show branded focus ring on tag chips` (light + dark) - Run `checkA11y` on: home, document detail, edit, persons list, admin pages - Visual regression snapshots at 1280px of focused states in both modes ### Suggestions - Add the Playwright focus tests as acceptance criteria with specific test names, not just "verified manually." - Run Playwright with `colorScheme: 'dark'` in a separate test project in `playwright.config.ts` — low-overhead way to guarantee dark mode coverage on every PR.

marcel commented

2026-03-31 10:37:38 +02:00

🔐 Nora "NullX" Steiner — Application Security Engineer

Questions & Observations

No direct security vulnerabilities in this change — adding CSS custom properties and updating focus ring classes doesn't introduce an attack surface. But there are a few security-adjacent notes worth capturing:

Focus indicators matter for security beyond accessibility. Visible, branded focus rings help users orient themselves in the real app and notice when they've been redirected to a lookalike page. Removing or weakening focus rings is a technique sometimes used in UI-redressing attacks to make fake overlays feel native. Keeping focus rings prominent and branded is a mild but real defense-in-depth measure — worth documenting as intentional in the issue.
focus-visible vs focus — XSS surface consideration (informational): focus-visible is the right choice here (it only shows the ring on keyboard navigation, not pointer clicks). Using :focus instead would have been fine functionally, but focus-visible is more specific and less likely to be accidentally overridden by user stylesheets injected via browser extensions. No action needed — just confirming the proposed approach is the correct one.
CSS custom property injection (non-issue, informational): If user-controlled content were ever rendered via style attributes, a --c-focus-ring override could theoretically be injected. The app's architecture (SSR with Tailwind, no inline style from user input) makes this a theoretical non-issue. Just confirming it's been considered.
forced-colors / Windows High Contrast mode: This is both an accessibility and a security-adjacent concern — users who rely on system accessibility features may have CSS custom properties overridden. In forced-colors mode, var(--c-focus-ring) may not resolve as expected. The browser typically substitutes Highlight or ButtonText for focus indicators in this mode, which is actually correct behavior. But it's worth a quick test to confirm the focus ring doesn't disappear entirely in forced-colors mode.

Suggestions

Add a brief forced-colors: active media query test to the acceptance criteria (verify ring is still visible, even if the system color replaces the brand color).
No security blockers — proceed with implementation.

## 🔐 Nora "NullX" Steiner — Application Security Engineer ### Questions & Observations No direct security vulnerabilities in this change — adding CSS custom properties and updating focus ring classes doesn't introduce an attack surface. But there are a few security-adjacent notes worth capturing: - **Focus indicators matter for security beyond accessibility.** Visible, branded focus rings help users orient themselves in the real app and notice when they've been redirected to a lookalike page. Removing or weakening focus rings is a technique sometimes used in UI-redressing attacks to make fake overlays feel native. Keeping focus rings prominent and branded is a mild but real defense-in-depth measure — worth documenting as intentional in the issue. - **`focus-visible` vs `focus` — XSS surface consideration (informational):** `focus-visible` is the right choice here (it only shows the ring on keyboard navigation, not pointer clicks). Using `:focus` instead would have been fine functionally, but `focus-visible` is more specific and less likely to be accidentally overridden by user stylesheets injected via browser extensions. No action needed — just confirming the proposed approach is the correct one. - **CSS custom property injection (non-issue, informational):** If user-controlled content were ever rendered via `style` attributes, a `--c-focus-ring` override could theoretically be injected. The app's architecture (SSR with Tailwind, no inline style from user input) makes this a theoretical non-issue. Just confirming it's been considered. - **`forced-colors` / Windows High Contrast mode:** This is both an accessibility and a security-adjacent concern — users who rely on system accessibility features may have CSS custom properties overridden. In forced-colors mode, `var(--c-focus-ring)` may not resolve as expected. The browser typically substitutes `Highlight` or `ButtonText` for focus indicators in this mode, which is actually correct behavior. But it's worth a quick test to confirm the focus ring doesn't disappear entirely in forced-colors mode. ### Suggestions - Add a brief `forced-colors: active` media query test to the acceptance criteria (verify ring is still visible, even if the system color replaces the brand color). - No security blockers — proceed with implementation.

marcel commented

2026-03-31 10:38:01 +02:00

🎨 Leonie Voss — UI/UX Design Lead

This issue is in my domain. The values and intent are right — a few things to tighten up before implementation.

Contrast values — good, with one gap

Context	Color	Ratio cited	My check
Light on white	`#012851`	~14:1	✅ AAA
Light on sand `#E4E2D7`	`#012851`	not cited	✅ ~11:1 — fine
Dark on `#010e1e`	`#a1dcd8`	9.2:1	✅ AAA
Dark on card background	`#a1dcd8`	not cited	⚠️ needs check

The dark mode card background in the redesign (#0d1f35 or similar) may be lighter than #010e1e. A ring that passes on the canvas may fail on a card. The implementer should spot-check at least one focused input inside a dark card.

WCAG 2.4.11 — ring width is missing from scope

WCAG 2.4.11 (Focus Appearance) requires:

The focus indicator area ≥ perimeter of the unfocused component × 1px AND
Contrast ≥ 3:1 between focused and unfocused states

The color token solves (2). But (1) depends on ring width and offset. The issue mentions ring-2 ring-offset-2 in passing but this is not in the acceptance criteria. I'd add:

Focus ring is ring-2 (2px) with ring-offset-2 on all interactive elements

A 1px ring fails WCAG 2.4.11 even at perfect contrast.

Ring offset color in dark mode

ring-offset-2 renders the offset using the element's background color by default. On dark surfaces, if a button has a dark background, the offset gap becomes invisible. Consider defining a --color-focus-ring-offset token that matches the local background, or use ring-offset-[color] explicitly on components with non-white backgrounds.

`outline-none` audit is critical

Many Tailwind components default to outline-none on inputs and buttons. The audit must not just add the new token — it must actively remove outline-none from every place it appears without an explicit replacement. No silent swallowing.

`forced-colors` media query

Add to layout.css:

@media (forced-colors: active) {
  :focus-visible {
    outline: 3px solid ButtonText;
  }
}

This ensures keyboard users on Windows High Contrast mode always get a visible focus ring, even if CSS custom properties are overridden by the system.

Suggestions

Add ring-2 ring-offset-2 as an explicit acceptance criterion
Add a forced-colors fallback to the CSS scope
Spot-check dark card backgrounds for contrast, not just the canvas

## 🎨 Leonie Voss — UI/UX Design Lead This issue is in my domain. The values and intent are right — a few things to tighten up before implementation. ### Contrast values — good, with one gap | Context | Color | Ratio cited | My check | |---|---|---|---| | Light on white | `#012851` | ~14:1 | ✅ AAA | | Light on sand `#E4E2D7` | `#012851` | not cited | ✅ ~11:1 — fine | | Dark on `#010e1e` | `#a1dcd8` | 9.2:1 | ✅ AAA | | **Dark on card background** | `#a1dcd8` | **not cited** | ⚠️ needs check | The dark mode card background in the redesign (`#0d1f35` or similar) may be lighter than `#010e1e`. A ring that passes on the canvas may fail on a card. The implementer should spot-check at least one focused input inside a dark card. ### WCAG 2.4.11 — ring width is missing from scope WCAG 2.4.11 (Focus Appearance) requires: 1. The focus indicator area ≥ perimeter of the unfocused component × 1px **AND** 2. Contrast ≥ 3:1 between focused and unfocused states The color token solves (2). But (1) depends on ring width and offset. The issue mentions `ring-2 ring-offset-2` in passing but this is not in the acceptance criteria. I'd add: > - [ ] Focus ring is `ring-2` (2px) with `ring-offset-2` on all interactive elements A 1px ring fails WCAG 2.4.11 even at perfect contrast. ### Ring offset color in dark mode `ring-offset-2` renders the offset using the element's background color by default. On dark surfaces, if a button has a dark background, the offset gap becomes invisible. Consider defining a `--color-focus-ring-offset` token that matches the local background, or use `ring-offset-[color]` explicitly on components with non-white backgrounds. ### `outline-none` audit is critical Many Tailwind components default to `outline-none` on inputs and buttons. The audit must not just add the new token — it must actively remove `outline-none` from every place it appears without an explicit replacement. No silent swallowing. ### `forced-colors` media query Add to `layout.css`: ```css @media (forced-colors: active) { :focus-visible { outline: 3px solid ButtonText; } } ``` This ensures keyboard users on Windows High Contrast mode always get a visible focus ring, even if CSS custom properties are overridden by the system. ### Suggestions - Add `ring-2 ring-offset-2` as an explicit acceptance criterion - Add a `forced-colors` fallback to the CSS scope - Spot-check dark card backgrounds for contrast, not just the canvas

marcel commented

2026-03-31 10:38:18 +02:00

🚀 Tobias Wendt — DevOps & Platform Engineer

Questions & Observations

This is a pure frontend CSS change — no infra, no config, no deployment concerns. But a few pipeline observations:

CI coverage for dark mode: The existing Gitea Actions workflow probably doesn't run Playwright with colorScheme: dark. If focus ring tests land in the E2E suite (which they should, per Sara's proposal), the pipeline needs a dark-mode test project configured in playwright.config.ts. This is a one-time config addition, not a recurring cost.
Visual regression diff workflow: A cross-cutting change like this — touching buttons, inputs, links, chips across every page — will produce a large visual regression diff. Make sure the Playwright visual baseline is updated as part of the PR, not after. Stale baselines will produce false failures on the next PR.
Build artifact size: No concerns. CSS custom properties and Tailwind utility additions add negligible bytes to the production bundle. The ring-focus-ring utility will be tree-shaken in if used — no action needed.
npm run check (svelte-check): Any class that doesn't resolve in Tailwind 4 will not produce a build error — it just silently produces no output. After adding --color-focus-ring to @theme inline, do a quick npm run build to confirm ring-focus-ring is actually emitting CSS in the production bundle, not silently disappearing.

Suggestions

Add a playwright.config.ts projects entry for dark mode before this PR is merged:
```
{ name: 'chromium-dark', use: { ...devices['Desktop Chrome'], colorScheme: 'dark' } }
```
Costs nothing at runtime (shares the same built app), gives permanent dark mode regression coverage.
No deployment steps required for this change. Ship it.

## 🚀 Tobias Wendt — DevOps & Platform Engineer ### Questions & Observations This is a pure frontend CSS change — no infra, no config, no deployment concerns. But a few pipeline observations: - **CI coverage for dark mode:** The existing Gitea Actions workflow probably doesn't run Playwright with `colorScheme: dark`. If focus ring tests land in the E2E suite (which they should, per Sara's proposal), the pipeline needs a dark-mode test project configured in `playwright.config.ts`. This is a one-time config addition, not a recurring cost. - **Visual regression diff workflow:** A cross-cutting change like this — touching buttons, inputs, links, chips across every page — will produce a large visual regression diff. Make sure the Playwright visual baseline is updated as part of the PR, not after. Stale baselines will produce false failures on the next PR. - **Build artifact size:** No concerns. CSS custom properties and Tailwind utility additions add negligible bytes to the production bundle. The `ring-focus-ring` utility will be tree-shaken in if used — no action needed. - **`npm run check` (svelte-check):** Any class that doesn't resolve in Tailwind 4 will not produce a build error — it just silently produces no output. After adding `--color-focus-ring` to `@theme inline`, do a quick `npm run build` to confirm `ring-focus-ring` is actually emitting CSS in the production bundle, not silently disappearing. ### Suggestions - Add a `playwright.config.ts` `projects` entry for dark mode before this PR is merged: ```ts { name: 'chromium-dark', use: { ...devices['Desktop Chrome'], colorScheme: 'dark' } } ``` Costs nothing at runtime (shares the same built app), gives permanent dark mode regression coverage. - No deployment steps required for this change. Ship it.

marcel referenced this issue from a commit

2026-03-31 12:00:27 +02:00

docs(specs): add focus rings design spec for issue #167

marcel referenced a pull request that will close this issue

2026-03-31 16:11:30 +02:00

feat(focus-rings): branded focus ring tokens (#167) #170

marcel commented

2026-03-31 16:11:48 +02:00

Implementation complete ✅

All 8 tasks implemented on branch feat/issue-167-focus-ring-tokens → PR #170.

What was done

Token + global rule

--c-focus-ring: #012851 (light) / #a1dcd8 (dark) added to layout.css
@theme inline maps it to ring-focus-ring Tailwind utility
Global :focus-visible { outline: 2px solid var(--c-focus-ring); outline-offset: 2px; } in @layer base as safety net
forced-colors fallback for Windows High Contrast mode

Component sweep (37 files)

Header/nav: ThemeToggle, NotificationBell, LanguageSwitcher, AppNav (9 links), UserMenu
Auth pages: login, forgot-password, reset-password
All form inputs across document editor, person edit, admin (users, groups, tags), profile, search
Interactive widgets: PersonTypeahead, MentionEditor, PanelHistory, UserGroupsSection, notifications page, CorrespondentSuggestionsDropdown
CommentThread selection highlight: switched to outline-dotted outline-accent (visually distinct from solid focus ring)
Exception kept: admin/tags/[id] delete confirmation red ring (semantic destructive indicator)

Commits

fe1121d test(focus-rings): add failing Playwright tests
17889df feat(focus-rings): add --c-focus-ring token to CSS design system
f04e4ff feat(focus-rings): update header/nav components
d0deb26 feat(focus-rings): update auth and search inputs
1541afd feat(focus-rings): update all form inputs and document components
a5cc8fd feat(focus-rings): update interactive widgets
f1bf32e feat(focus-rings): CommentThread selection highlight → dotted outline

## Implementation complete ✅ All 8 tasks implemented on branch `feat/issue-167-focus-ring-tokens` → PR #170. ### What was done **Token + global rule** - `--c-focus-ring: #012851` (light) / `#a1dcd8` (dark) added to `layout.css` - `@theme inline` maps it to `ring-focus-ring` Tailwind utility - Global `:focus-visible { outline: 2px solid var(--c-focus-ring); outline-offset: 2px; }` in `@layer base` as safety net - `forced-colors` fallback for Windows High Contrast mode **Component sweep (37 files)** - Header/nav: ThemeToggle, NotificationBell, LanguageSwitcher, AppNav (9 links), UserMenu - Auth pages: login, forgot-password, reset-password - All form inputs across document editor, person edit, admin (users, groups, tags), profile, search - Interactive widgets: PersonTypeahead, MentionEditor, PanelHistory, UserGroupsSection, notifications page, CorrespondentSuggestionsDropdown - CommentThread selection highlight: switched to `outline-dotted outline-accent` (visually distinct from solid focus ring) - Exception kept: `admin/tags/[id]` delete confirmation red ring (semantic destructive indicator) ### Commits - `fe1121d` test(focus-rings): add failing Playwright tests - `17889df` feat(focus-rings): add --c-focus-ring token to CSS design system - `f04e4ff` feat(focus-rings): update header/nav components - `d0deb26` feat(focus-rings): update auth and search inputs - `1541afd` feat(focus-rings): update all form inputs and document components - `a5cc8fd` feat(focus-rings): update interactive widgets - `f1bf32e` feat(focus-rings): CommentThread selection highlight → dotted outline

marcel closed this issue

2026-03-31 17:05:11 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: marcel/familienarchiv#167

feat(focus-rings): branded focus ring tokens for light and dark mode #167

Background

Problem

Proposed Solution

Scope

Acceptance Criteria

Related

👨‍💻 Felix Brandt — Senior Fullstack Developer

Questions & Observations

Suggestions

🏛️ Markus Keller — Application Architect

Questions & Observations

Suggestions

🧪 Sara Holt — QA Engineer & Test Strategist

Questions & Observations

Suggested Test Plan (layer by layer)

Suggestions

🔐 Nora "NullX" Steiner — Application Security Engineer

Questions & Observations

Suggestions

🎨 Leonie Voss — UI/UX Design Lead

Contrast values — good, with one gap

WCAG 2.4.11 — ring width is missing from scope

Ring offset color in dark mode

outline-none audit is critical

forced-colors media query

Suggestions

🚀 Tobias Wendt — DevOps & Platform Engineer

Questions & Observations

Suggestions

Implementation complete ✅

What was done

Commits

`outline-none` audit is critical

`forced-colors` media query