Repository navigation
Stop every focus move from re-checking the whole page's styles on macOS - #1038
Conversation
Tailwind's group-focus-within compiles to :is(:where(.group):focus-within *), which WebKit can't key its invalidation on, so every focus move re-checked the whole document: about 45 ms each with a long transcript open. Keeping the whole group inside :where(), the form the dark variant already uses, keys the rule on .group. The rules match the same elements and compute the same styles.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Title check |  |
The title clearly describes the main objective: preventing full-page style recalculation during focus moves on macOS. It is specific and related to the selector change. |
| Description check | |
The description includes all required sections, explains the change and motivation, documents UI impact, and reports validation results with the checklist completed. |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
@zcy22606 looks good! thank you |
What changed
src/styles/index.cssredefines thegroup-focus-withinvariant as&:where(.group:focus-within *), so the whole group condition sits inside:where(). This is the same form the file already uses for thedarkvariant. No component changes.Why
Tailwind v4's built-in
group-focus-withincompiles toWebKit's style invalidation can't key this rule on
.group, because.groupis wrapped in a:where()nested inside the:is(). So whenever focus moves anywhere (clicking a session row, a new composer taking focus, tabbing), WebKit re-checks the style of the whole document. With a long transcript open, the Safari Web Inspector timeline showed about 45 ms of style recalculation per focus move. I measured this in a build of MonoCode that I run day to day, not on a fresh checkout of main.With
&:where(.group:focus-within *)the rule is keyed on.group:focus-within. WebKit then only invalidates the.groupelements whose focus-within state changed, and the same focus move costs next to nothing.This is the same kind of fix as #985, where
:has()inside agroup-has-*variant made every composer keystroke re-check the whole page.Styles are unchanged
The selector matches exactly the same elements:
:where()/:is()only affect specificity, not matching. There are 5 uses on main (git grep group-focus-within):Sidebar.tsxsession card: model name (max-w,pr,opacity) and the archive button (pointer-events,opacity)AutomationsView.tsx"Remove trigger" button (opacity)GitChangesPanel.tsxdirectory and file row actions (display)Two things change in the compiled CSS, and neither changes a computed value:
opacity-0,max-w-0,pr-0,pointer-events-none,hidden) at (0,1,0), which the variant still beats because it comes later in the file (next point), or (b)group-hover:*/focus-visible:opacity-100, which set the same value. No non-utility rule in the built CSS sets these properties on these elements. I scanned every rule in the build output that setsopacity/display/pointer-events/max-width/paddingwith a selector that isn't a plain utility.dark:*) instead of beforegroup-hover:*, so they still come after the base utilities they override.I diffed
npx vite buildoutput before and after the change (written to /tmp, outside the repo). The only difference is these 5 rules, rewritten and moved. I then loaded both builds in Playwright (WebKit and Chromium) on the exact class lists above and compared the computedopacity,display,pointer-events,max-width,padding-right,background-colorandcolorin idle, group focus-within, group hover, hover + focus-within, self-hover and keyboard-focus states. Both engines gave 24/24 identical states. (Playwright's WebKit is a trunk build, so I used it to check the styles are the same, not to measure the slowdown.)UI
No visual change. The computed styles match in every state checked (see above).
Validation
npm run check:web: vitest 483 files / 5363 tests passed (13 skipped),tsc --noEmitclean. Rust untouched, socheck:rustnot run.npx prettier --check src/styles/index.csspasses.Checklist
npm run check(check:web; no Rust files changed)Summary by CodeRabbit
group-focus-withinstyling, allowing elements within a focused group to change appearance when the group or one of its descendants receives focus. This makes it easier to provide visible focus feedback for grouped interface elements, such as forms and navigation controls.