💍 fix: Unclip the Keyboard Highlight Ring in Select Popovers - #125
Open
dustinhealy wants to merge 3 commits into
Open
💍 fix: Unclip the Keyboard Highlight Ring in Select Popovers#125dustinhealy wants to merge 3 commits into
dustinhealy wants to merge 3 commits into
Conversation
Inline @clickhouse/click-ui in vitest server deps and exclude .claude/** from test discovery. click-ui 0.9.1 ships ESM that imports .css files, which Node's loader cannot parse, so Vitest must process the package through Vite.
click-ui renders the select option list at the trigger width while the popover border-box loses 2px to its 1px borders, so the overflow hidden popover clips the outermost 2px of the options on the right. The keyboard-highlight ring click-ui draws on the active option (2px outline at -2px offset) falls entirely inside that clipped band on the right edge, so the ring rendered on three sides only. Inset the ring by the overhang with a scoped override on the click-ui modality attribute pair, following the existing scoped focus overrides in styles.css.
dustinhealy
changed the title
…oken click-ui paints the keyboard-highlight ring on popover options with its own accent token, which the app theme never overrides, so the ring rendered near-black in light mode while every other focus ring uses the outline token. The options are not buttons, so the global focus-visible override never reaches them; the existing scoped popover rule now also sets the outline color to var(--cui-color-outline), matching the app focus convention in both themes.
dustinhealy
marked this pull request as ready for review
August 11, 2026 23:04
Contributor
Author
|
@codex review |
dustinhealy
marked this pull request as draft
August 14, 2026 15:05
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
dustinhealy
marked this pull request as ready for review
August 14, 2026 15:23
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Keyboard-navigating the options of any click-ui Select shows the highlight ring on only three sides; the right edge is cut off by the popover.
Root cause (measured in the browser): click-ui's Select popover content is
overflow: hiddenwith 1px borders, so its inner clip box is 2px narrower than the popover, but the option list and rows render at the full trigger width and overhang the clip box by exactly 2px on the right. click-ui's keyboard-highlight rule draws a 2px outline atoutline-offset: -2pxon the active option, placing the ring in the outermost 2px band, so the right band falls entirely inside the clipped overhang. Pixel sampling on the highlighted row confirmed zero outline pixels between the option's visible right edge and the popover border. Systemic to every click-ui Select in the app and theme-independent.Fix: a scoped override in
styles.css, following the existing scoped focus-override convention, insets the ring past the overhang:[data-input-modality='keyboard'] [data-highlighted] { outline-offset: -4px !important; }. The selector pair only matches click-ui popover descendants because the input-modality attribute lives on the click-ui popover content element; other click-ui popovers (e.g. the user-menu dropdown) also pick up the inset, verified visually with no regression (their rings stay complete and comfortably inside the menu). The width mismatch itself is a click-ui bug worth reporting upstream; this override keeps the ring fully visible regardless.Change Type
Testing
Verified in the browser at 1280x800 with keyboard modality: the highlighted option's ring now renders on all four sides (computed
outline-offset: -4px, outline pixels present at the right band inside the popover), checked on multiple selects (Title method, Title timing). jsdom cannot assert outline geometry, so the proof is visual plus the pixel sampling; no unit test carries signal for this rule.Full gates green: bunx tsc --noEmit, bunx eslint src/ --max-warnings 0, bunx vitest run (799/799), bun run build, prettier on the touched file.
Before/after proof:
Test Configuration:
Checklist