Retire --shadow-focus for the sanctioned focus outline (#261) - #1807
Conversation
DS Track A2. `--shadow-focus` packed a 3px accent halo in front of `--shadow-soft`, so its one consumer — `.chat-composer-shell-delta:focus-within` — painted a companion ring on top of the accent border swap. That is the second focus affordance the shared `:focus-visible` treatment is written to prevent (GATES Gate 3). The composer now uses `outline: 2px solid var(--focus)` at `outline-offset: 2px`, and both theme declarations of the token are deleted. Focus no longer overrides `box-shadow`. The retired token carried `--shadow-soft` as its second layer, so leaving the resting elevation alone keeps the pill seated at the same height focused as at rest. Measured in Chromium: light `solid 2px rgb(29, 111, 184)`, dark `solid 2px rgb(116, 189, 240)`, box-shadow byte-identical resting vs focused in both themes. Corrects the premise inherited from HANDOVER-2026-08-07 A2: this is not a visible production focus change. `chatComposerShell` is imported only by `calculators/search-detail.tsx` and its mockup twin; production `/calculators` renders `chatComposerShellBase` + `answer-footer-search-pill`, and `CalculatorSearchHome` is reached only from two unrouted mockup exports. Probing all 37 static production routes in Chromium found zero renders of the class; the single live render is `/mockups/calculators-search`, where the look was taken. The inherited note was right that a `--include=*.tsx` grep misses the consumer (it is in CSS) but wrong about its reach. Corrections recorded in the handover's own "Measured wrong so far" list, per that file's convention. Guard: design-token-contract.test.ts rejects both a `--shadow-focus:` declaration and a `var(--shadow-focus)` consumer, across globals.css and the v2 layer, and is mutation-verified against both. Deliberately not the whole-file substring check used for `--shadow-tight`, because the composer rule names the retired token in a comment on purpose. legacyShadowAliases 127 -> 125 (soft 71 -> 69), globals.css per-path pin tightened 3 -> 1: the deleted declarations' VALUE ended in `var(--shadow-soft)`, so they scored as two `soft` aliases — the indirection GATES documents. Left un-ratcheted as pre-existing slack unrelated to this diff: rawPaddingLiterals 67 -> 65, rawGapLiterals 34 -> 32, layoutTransitionExceptions 12 -> 11. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012spLo9ry4zjoDyZi7j89kT
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
📝 WalkthroughWalkthroughThe PR removes ChangesShadow focus token retirement
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. Comment |
Handoff record for PR #1807 (DS Track A2, --shadow-focus retirement). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012spLo9ry4zjoDyZi7j89kT
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 448a0d084c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/design-system/TOKENS.md (1)
112-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winKeep exact focus values in the owner file.
Lines 3-5 state that token values live only in the owner file. The new row repeats
outline: 2px solid var(--focus)andoutline-offset: 2px. Keep this row at the role and behavior level, and link todocs/design-system.mdorsrc/app/globals.css.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/design-system/TOKENS.md` around lines 112 - 115, Update the --shadow-focus disposition in the design-system token table to describe the focus behavior at the role level without repeating exact outline values. Link readers to the owning documentation or stylesheet, such as docs/design-system.md or the globals.css owner, while preserving the migration status and gate details.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/globals.css`:
- Around line 1470-1480: Update the comment above
.chat-composer-shell-delta:focus-within so it acknowledges the focus-state
border-color change, stating that the outline is the only additional focus layer
or explicitly mentioning the border swap; leave the CSS rule unchanged.
In `@tests/design-token-contract.test.ts`:
- Around line 179-195: Update the retired-token assertions in the “keeps the
retired --shadow-focus token deleted in every scope” test to use
whitespace-aware matching for declarations and var() consumers, including
optional fallback arguments. Add mutation cases covering `--shadow-focus :`,
`var( --shadow-focus )`, and `var(--shadow-focus, fallback)` so each form is
rejected.
---
Nitpick comments:
In `@docs/design-system/TOKENS.md`:
- Around line 112-115: Update the --shadow-focus disposition in the
design-system token table to describe the focus behavior at the role level
without repeating exact outline values. Link readers to the owning documentation
or stylesheet, such as docs/design-system.md or the globals.css owner, while
preserving the migration status and gate details.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 58cb1908-ef17-44e2-a312-1b254f09f913
📒 Files selected for processing (10)
docs/branch-review-ledger.mddocs/design-system.mddocs/design-system/GATES.mddocs/design-system/HANDOVER-2026-08-07.mddocs/design-system/SPEC.mddocs/design-system/TOKENS.mddocs/outstanding-issues.mdscripts/design-system-contract-baseline.jsonsrc/app/globals.csstests/design-token-contract.test.ts
Unblock snapshot (PR #1807)
What I changed:
Residual risk / what still blocks merge:
If you want, I can continue to the next PR now and return to #1807 after any reviewer action is posted. |
Summary
--shadow-focus(DS Track A2, ledger#261). The token packed a 3px accent halo in front of--shadow-soft, so its one consumer —.chat-composer-shell-delta:focus-within— painted a companion focus ring on top of the accent border swap. That is the second focus affordance the shared:focus-visibletreatment inglobals.cssis written to prevent (GATES Gate 3). The composer now uses the sanctionedoutline: 2px solid var(--focus)atoutline-offset: 2px, and both theme declarations of the token are deleted.box-shadow. The retired token carried--shadow-softas its own second layer, so leaving the resting elevation alone keeps the pill seated at exactly the same height focused as at rest — the outline is the only thing that changes.tests/design-token-contract.test.tsrejecting both a--shadow-focus:declaration and avar(--shadow-focus)consumer, acrossglobals.cssand the v2 layer. Deliberately not the whole-file substring check used for--shadow-tight: the composer rule names the retired token in a comment on purpose, so the next author reaching for a focus halo finds the reason it is gone.legacyShadowAliases127 → 125 with theglobals.cssper-path pin 3 → 1. The deleted declarations' value ended invar(--shadow-soft), so they scored as twosoftaliases — the indirection GATES already documents. Left un-ratcheted because it is pre-existing slack unrelated to this diff:rawPaddingLiterals67 → 65,rawGapLiterals34 → 32,layoutTransitionExceptions12 → 11.HANDOVER-2026-08-07Track A2, which called it "a visible focus-state change on the search composer". It is not a visible production change.chatComposerShellis imported only bycalculators/search-detail.tsxand its mockup twin; production/calculatorsrenderschatComposerShellBase+answer-footer-search-pillinstead, andCalculatorSearchHomeis reached only from two unrouted mockup exports. Probing all 37 static production routes in Chromium found zero renders of the class; the single live render is/mockups/calculators-search. The inherited note was right that a--include=*.tsxgrep misses the consumer — it is in CSS — but wrong about its reach. The correction is recorded in that handover's own "Measured wrong so far" list, per the file's stated convention of not rewriting its body.TOKENS.md,SPEC.md,design-system.mdand the Gate 3 evidence note, refreshes the now-stale alias breakdown inGATES.mdto the measured 125 (soft69,elevated17,hover17,card12,lux8,lift2), and archives#261in the outstanding-issues ledger.Verification
npm run verify:pr-localEvery step passed except one pre-existing failure unrelated to this diff:
tests/pr-handoff-stop.test.ts > emits handoff context only when the marker file exists. Baselined rather than assumed — with these changes stashed, the untouched basee8b61d8fails the same single assertion (Tests 1 failed | 10 passed (11)). Because the runner stops there,buildandcheck:rag:fixtureswere re-run directly afterwards and both passed:npm run check:design-system-contract— the gate this ledger row names./mockups/calculators-search— the only route that renders the class:redeclares --shadow-focus, reinstating thevar()consumer fails withconsumes --shadow-focus.UI verification not run:
npm run verify:uiandnpm run verify:phone-chromewere not run. This container's Chromium is revision 1194 against the repo's pinned Playwright revision 1234 — the#255drift, wheredocs/testing.mdsays to delegate browser proof to CI Production UI rather than force a mismatched binary. The Chromium evidence above is therefore reported as an inspection, not as gate evidence. No production phone chrome renders the changed class (zero of 37 production routes), so the phone-chrome risk this row anticipated does not arise.Risk and rollout
/mockups/calculators-search, which 404s in production; the correctness benefit is that any future production use ofchatComposerShellinherits the sanctioned focus treatment instead of a companion ring. Residual risk is a surface adopting the class between this PR and a Chromium CI run — covered by the Production UI job.git revertthis commit. The token declarations and thebox-shadow: var(--shadow-focus)consumer return together; the contract guard is in the same commit, so it reverts with them and cannot fail against a restored token.npm run buildand not left running.Notes
#261is archived with the measured outcome, including the reach correction and the three un-ratcheted slack metrics left for whoever picks up#262.GATES.mdstays planned. This removes the one companion ring the rule knew about; nothing yet stops the next one.Generated by Claude Code
Summary by CodeRabbit