Repository navigation
fix(registry): match Radix focus details in Fortal - #328
Open
leoafarias wants to merge 2 commits into
Open
leoafarias wants to merge 2 commits into
leoafarias wants to merge 2 commits into
Conversation
Checked against the pinned @radix-ui/themes 3.3.0 CSS, five Fortal focus treatments differed from Radix. - The soft radio rings in accent-a8 (base-radio.css:165-168); the other variants keep focus-8. - The soft select rings in accent-8 (select.css:430-433). Fortal resolves both tokens to the same step, so the rendering is unchanged unless the focus color is overridden. - The slider thumb's own 1px edge stays above the focus halo (slider.css:88-93). The halo moves from the foreground layer to behind the thumb, which is where the thumb's decoration shadows sit above it. - The dashboard accent swatch rings with Radix's ThemePanel outline (theme-panel.css:36-47): 2px off the edge, gray-12 once selected, and the panel's accent-9 while focused, so focus stays visible on the selected swatch. The ring is a foreground border and takes no layout space; the panel gains 4px of horizontal padding so the scroll views that host it do not clip the first column. - A focused sortable data table header draws a focus-8 ring inside its cell, as a foreground border like the row divider, through the existing headerCell slot. The errored text field keeps its current focus treatment, which needs a design decision. Registry templates and the demo and dashboard installed copies are regenerated.
…ch ring Drop the focus ring on sortable data table headers. Non-sortable header cells have no widget-state scope of their own and read the nearest ancestor's, so a table inside a keyboard-focused FortalDisclosure, or under a Pressable, drew a ring on every static header. Remix has to give non-sortable headers an empty state scope, as body cells have, before Fortal can add the ring back. The accent swatch ring: - draws nothing when the ring width is 0 instead of dividing by it, which threw in debug builds when the offset was also 0; - is cleared by a panel inset of the ring offset plus the ring width, read from the tokens, so a larger ring is not clipped at the panel edge. The panel grows by the same amount, keeping its content 360 wide. The slider thumb pixel test now samples the center of each band, so it holds at every theme radius in light and dark mode, and the test file describes what it covers.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This branch has not been deployed
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
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.
Description
Stacked on #320 (
fix/registry-fortal-focus-ring-tokens); review and merge that first. This PR uses itsfortalFocusOutlinehelper and ring tokens and edits the same radio, select, and slider lines.Checked against the pinned
@radix-ui/themes@3.3.0CSS, four Fortal focus treatments differed from Radix:accent-a8(base-radio.css:165-168); surface and classic keepfocus-8.accent-8(select.css:430-433). Fortal resolves both tokens to the same step, so rendering is unchanged unless the focus color is overridden.slider.css:88-93); the halo now paints behind the thumb instead of over it.theme-panel.css:36-47): 2px off the edge, gray-12 when selected, the panel's accent-9 when focused, so focus is visible on the selected swatch. It is a foreground border (no layout shift) that follows the ring width and offset tokens and draws nothing at width 0. The panel insets its content by the ring's offset plus width and grows by the same amount, so the content stays 360 wide and a scroll view hosting it does not clip the first column's ring.Deferred: focus ring on sortable data-table headers. A ring cannot be added in Fortal alone: non-sortable header cells have no widget-state scope of their own and read the nearest ancestor's, so a table inside a focused
FortalDisclosure(or under aPressable) would ring every static header. The fix belongs in Remix (give non-sortable headers an empty state scope, as body cells have); Fortal re-adds the ring after that.Left unchanged: the errored text field's focus treatment (the error-8 outline is the same focused and idle), which needs a design decision.
Registry templates and the demo and dashboard installed copies are regenerated.
Validation
focus_fidelity_test.dart(24 tests, CSS cited). Against the previous code the swatch and panel tests (clip at a larger ring, content width, zero width) and the slider layering and pixel tests fail; the slider pixel test runs at all five theme radii in light and dark mode.registry_sourceflutter test(1420) anddart analyze;apps/demo(64) andapps/dashboard(84) analyze and tests.fortal:parity:check,open-code:registry:build,build_registry.dart --check,check_consumer_apps.dart,melos run ci:checks, rootdart formatcheck.Related Issues
Follow-up to #320.
Checklist
///).Breaking Change