Refine flyout visuals, motion and surface tint - #16
Merged
Conversation
Use opaque themed elevation, compact spacing, and interruption-safe point-to-point transitions so the popover reads as a precise desktop overlay.
Tighten row rhythm, align semantic interaction states, and unify context and submenu transitions for clearer keyboard-driven navigation.
Drive shell width and canvas spacing from interruption-safe visual state so collapse reversals preserve geometry, opacity, and reduced-motion policy.
Stabilize virtualized row geometry, strengthen selection semantics, and stage content reveal without delaying keyboard or action handling.
Keep primary and trigger segments aligned across states, expose keyboard focus on the trigger, and route disabled and loading behavior through the full group.
Route flyouts and floating panels through theme-driven Acrylic and Mica surfaces. Preserve opaque blur-off fallbacks while giving menus, palettes, and popovers more breathing room.
Keep existing subtle-stroke token semantics while sourcing glass edges from the dedicated control-stroke color. Let explicit FloatingSidebar background refinements override the transparent inner default.
Flyout surfaces each carried their own geometry: menus hardcoded a 6px radius, popover and the editor popovers used theme.radius, select clamped to 8px, and the palette used radius_lg, with rows at a flat 4px unrelated to any container. Left insets, type sizes, and intra-row gaps drifted the same way. Add FlyoutTokens to derive all of it from theme tokens: container radius from radius_lg, row radius as radius minus inset so corners stay concentric, one 12px left rail, two type steps from theme.typography, and two intra-row gaps. Apply across popover, hover card, popup and context menu, select popup, command palette, editor popovers, and the collapsed sidebar submenu. Drop StyledExt::popover_style, which hand-rolled the flyout material in parallel with SurfacePreset and had drifted: it missed the noise overlay and the opaque fallback used when backdrop blur is off. SurfacePreset ::apply_material is now public as its replacement. Also fixes a 4px command palette height error and a duplicated magnifier glyph in its search row.
The basic popover separated its header from its metadata with a rule, while the gap between the two groups sat within 10% of the gap between the metadata rows themselves. Nothing read as grouped, and the rule was doing work that spacing should do. Drop the divider and set three distinct steps: the title and its timestamp pair at 18.5px, metadata rows at 26.5px, and the group break at 34.5px. Vertical insets come out symmetric at 18px as a result.
muted.foreground is tuned against the window background, but flyouts sit above it — noticeably lighter in dark mode — so secondary text inside a flyout lost roughly a stop. All 22 bundled dark themes measured sub-AA there, Default Dark at 3.74:1 against a 4.5:1 floor. Add theme::contrast with WCAG math that composites through translucent surfaces, and derive flyout_secondary_foreground from it: keep the theme's token wherever it already passes, otherwise bisect lightness away from the material for the smallest correction that clears AA. Hue, saturation and alpha are preserved, so a theme's character survives, and the correction is a pure function of the theme, so custom themes get it too. Scoped to flyouts deliberately: cards and other raised surfaces share muted.foreground and are left alone. This promotes bundled_themes_meet_flyout_text_contrast_floor from an ignored known-failure to a passing test.
Every elevated surface resolved its color from fixed Fluent constants: flyout from acrylic_default (#2C2C2C/#FCFCFC), panel from mica_base (#202020/#F3F3F3), card from pure white. Under a tinted theme the app chrome carried the theme while its menus, panels and cards stayed neutral grey, so flyouts read as foreign cards dropped onto the window. Derive all of them the same way instead: take the theme anchor color and blend it toward the mode's elevation pole — white in dark mode, where surfaces rise toward light, black in light mode, where materials sit a step under a white window. Blend fractions are calibrated so the neutral Default theme reproduces the old Fluent values exactly, so it renders pixel-identical while tinted themes keep their hue. Collapsing onto the plain Popover and Sidebar sources was measured and rejected: popover resolves equal to background in every bundled theme, so the Default Dark menu would have dropped to #0A0A0A and vanished into the window. The variants are renamed for what they now derive — ElevatedPopover, ElevatedSidebar, ElevatedBackground — and no constant-backed color source remains. Drop the 8 Fluent color constants, their ThemeMaterial fields and their schema wiring. Third-party theme files carrying those keys still parse; ThemeConfig has no deny_unknown_fields, so the keys are simply ignored. Opacity and blur-radius material tokens stay, still consumed. Also fixes the flyout text hierarchy, which shared one value across three roles: section headers, disabled labels and enabled subtitles all rendered at luma 128, while a disabled row's own shortcut and icon sat at 142 — brighter than the label they belonged to. Adds flyout_disabled_foreground and routes every disabled branch through it, giving a three-step ladder (250/142/92 in Default Dark) that holds its hue across themes. Tests now derive the material through the renderer's own flyout_base_color so they cannot drift from the painted surface. That caught a real regression: Alduin's deliberately dim foreground fell to 4.40:1 on its brighter tinted material, so flyout_primary_foreground now takes the same minimal AA correction as the secondary role, a no-op for the other 21 themes.
Interaction and motion guidance used by the flyout polish passes. Written for SwiftUI, so it applies here as principles — direct manipulation, material treatment, typography, Reduce Motion — rather than as APIs.
Four surfaces had no motion at all despite being fully wired for it. The submenu instrumented 20 Entering frames while its pixels showed a one-frame pop: opacity and transform wrappers have no effect on an anchored() subtree unless it renders through deferred(). The context menu only animated because it happened to do so already. Wrapping the submenu the same way makes the identical animation code render, and explains how it had been escaping its parent's overflow_hidden clip. Hover card, select popup and command palette simply had no exit path. Every exit was also being cut off partway. Presence close windows ran on the fade token (83ms) while close animations ran 187ms, so surfaces unmounted at 44% of their curve and popped. Both now read one exit token, with a 33ms unmount grace because presence timers and the animation clock do not start on the same frame. Consolidate ThemeMotion from 18 fields to 10: four durations (fade, exit, enter, emphasis), one spring, four easings. normal and slow durations had zero consumers; fast_dismiss_easing was dead and byte-identical to fast_invoke; the mild and medium springs measured within three frames of each other at 60Hz and collapse to one. Exits use the standard curve, not Fluent's accelerate, which measured a 5.1 per-frame step into its final frame and reads as a snap. Every flyout now enters and exits on the same grammar, sliding from its trigger rather than its own center, with exit at roughly 0.9x enter. Measured by frame diff at 60fps CFR. Submenu 1-frame pop to 182ms open and 232ms close; hover card to 166/216ms; select popup to 249ms both ways; popover close from a truncated 99ms to a complete 215ms.
The dialog layer half-had this already: close_dialog deferred unmount, but only when the chrome itself animated. The command palette opts out of chrome animation, so it fell through to instant finalize and popped out in a single frame. The design conflated 'chrome animates' with 'defer unmount'. Separate the two. Dialog::defer_close opts a host into staying mounted through the exit window with a non-animating chrome, so content can own its own exit visuals. Hosts that do not opt in take the old path unchanged, and reduced motion always unmounts immediately. Content learns it is closing through the new ClosingScope and is_layer_closing. The flag is pushed during layout as well as paint: entity children build their trees in the layout pass, so a paint-only scope is invisible to a child view's render. The deferral window is its own ceiling — an exit_duration timer finalizes unconditionally, so there is no completion signal that can be dropped and leak a mounted view. The sheet had the same defect and takes the same shape. Fixes two chrome bugs the contract exposed: both dialog wrapper layers rendered presence.progress(1.0), which is 0 during Exiting, so a non-animating dialog went fully invisible for the whole deferral window. Timing moves onto the shared motion tokens, replacing the dialog's bespoke close duration that left only 20ms of slack over its own exit animation. Measured at 60fps: palette close from a 1-frame pop to a complete 232ms fade, sheet close from instant unmount to a 183ms slide mirroring its enter, dialogs unchanged in kind with their marginal tail-clip resolved. The palette renders the opaque fallback material while closing, because backdrop blur ignores element opacity and a wrapper fade over a blurred subtree does not fade at all. Remove that swap once the renderer honors element opacity.
The animation pass left popover, context menu, submenu, select and hover card running roughly ninety lines each of open/close wiring that had converged on identical behavior. Identical was the precondition for extracting it; before that pass they genuinely differed. Add flyout_presence for the shared enter/exit windows and unmount grace, FlyoutSlide for the slide geometry, and flyout_motion for the one open and close motion. Slide distances stay per-surface parameters rather than being normalized to a single value, because the goal here is to remove duplication without retuning anything. The command palette deliberately stays off the helper. Its motion is dialog-hosted — a mount-only enter and a defer_close-window exit driven by is_layer_closing, with no presence of its own — so folding it in would mean escape hatches for a surface that does not share the shape. Every surface re-recorded at 60fps and compared against the pre-refactor figures at matched noise floors. Submenu, select and hover card open measured frame-exact; the rest sit within one frame at 60Hz, which is capture variance rather than behavior, and the formulas are unchanged. Net 165 lines removed.
Backdrop blur ignored element opacity: paint_backdrop_blur was the one paint method that never read it, and BackdropBlur carried no opacity field, so a flyout fading to zero kept painting a full-strength blurred backdrop until unmount and then dropped it in a single frame. Opacity is now a final-composite parameter on the primitive, deliberately invisible to planning, source capture, clustering and pyramid generation, so surfaces mid-fade at different opacities still share one cluster and one blur texture. Folding it into the surface alpha lets the blend unit interpolate against the framebuffer, which is what the sharp backdrop would have been lerped against anyway — same result, no extra bind. Measured against the three candidate mechanisms on a real 215ms exit: composite weight holds a 1.05 unevenness against 3.17 for radius scaling and 4.56 for a threshold skip, whose detail unevenness of 35.5 is the pop itself. Drops the command palette's blur-off closing workaround, which existed only because a wrapper fade over a blurred subtree did not fade. Verified on macOS. The DirectX/HLSL and wgpu paths compile only in CI — no local toolchain for those targets — so they are unverified here.
The slide travel was a fixed 100px regardless of panel size, so a default 350px sheet appeared already 71% of the way in and then settled the last 100px — and once the exit mirrored the enter, it left the same way. Derive travel from the panel's own extent along its placement axis, so it enters from off-screen and leaves completely. Also give the notification's dismiss timer the same 33ms unmount grace the other layer surfaces use. Presence timers and the animation clock do not start on the same frame, so an exit window equal to the animation clips its final frame.
WalkthroughThis PR introduces shared flyout tokens, contrast-aware colors, theme-based motion helpers, deferred layer closing, animated sidebar transitions, updated visual-review stories, documentation, CI handling, and synchronized GPUI references. ChangesFlyout motion and visual system
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CommandPalette
participant Dialog
participant FlyoutMotion
participant ThemeMotion
User->>CommandPalette: open, search, select, or dismiss
CommandPalette->>ThemeMotion: read enter, exit, fade, and spring tokens
CommandPalette->>FlyoutMotion: animate results and closing state
CommandPalette->>Dialog: defer close while content exits
Dialog->>FlyoutMotion: propagate layer closing state
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Repoints from the branch commit to the squashed merge on the fork's main, so the pin stays valid if the branch is deleted.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/ui/src/theme/default-theme.json (1)
38-63: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winRemove the orphaned
mica_base_*/acrylic_*keys from bothmaterialblocks.ThemeMaterialConfigno longer defines these fields, so they’re ignored on load and leavecrates/ui/src/theme/default-theme.json:38-63, 300-325out of sync with the schema.🤖 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 `@crates/ui/src/theme/default-theme.json` around lines 38 - 63, Remove all mica_base_* and acrylic_* entries from both material blocks in the default theme JSON, leaving the remaining material settings unchanged so the file matches ThemeMaterialConfig.crates/ui/src/animation.rs (1)
481-541: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the new pure motion helpers.
The new branching/derivation logic in this file is untested:
FlyoutSlide::offsetdirection/progress math,FlyoutSlide::horizontal/verticaldistances, and the invariant thatexit_duration(motion) > exit_animation's own duration (the whole point ofEXIT_UNMOUNT_GRACE_MS). These are pure functions overThemeMotion::default()and cheap to assert.🧪 Suggested tests
#[test] fn exit_window_outlives_exit_animation() { let motion = ThemeMotion::default(); assert!( super::exit_duration(&motion) > std::time::Duration::from_millis(u64::from(motion.exit_duration_ms)) ); } #[test] fn flyout_slide_offset_collapses_to_zero_at_completion() { let slide = super::FlyoutSlide::vertical(-1.0); assert_eq!(slide.offset(slide.enter_distance, 1.0), gpui::px(0.0)); assert_eq!(slide.offset(slide.enter_distance, 0.0), gpui::px(-4.0)); }As per coding guidelines, "Tests should prioritize complex logic and core functionality... and cover branching, state transitions, and edge cases."
🤖 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 `@crates/ui/src/animation.rs` around lines 481 - 541, Add tests for the pure motion helpers in the existing tests module: verify exit_duration(&ThemeMotion::default()) exceeds the duration represented by motion.exit_duration_ms, and cover FlyoutSlide::offset completion/progress behavior using a vertical slide, including zero offset at progress 1.0 and the expected initial offset at progress 0.0. Also add assertions for FlyoutSlide::horizontal and ::vertical distance derivation and direction handling where applicable.Source: Coding guidelines
🤖 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 @.agents/skills/apple-design/SKILL.md:
- Around line 1-10: Rewrite the apple-design skill guidance to match this
Rust/GPUI repository instead of SwiftUI/Digests: remove SwiftUI, iOS/macOS,
Digests-only wrappers, and nonexistent documentation references, and describe
the actual GPUI-native motion, material, interaction, and accessibility APIs
used by the project. If suitable GPUI guidance cannot be provided, remove the
skill file.
In `@crates/story/src/stories/command_palette_story.rs`:
- Around line 52-58: Hoist the platform-specific primary modifier selection into
one shared constant and reuse it from static_items and the other
shortcut-building logic instead of maintaining separate cfg styles. Extract or
reuse a shared open-and-subscribe helper for the palette variants, and update
show_loading_palette and show_empty_palette to subscribe to handle.state() so
the Last Selected panel stays current.
In `@crates/story/src/stories/floating_sidebar_story.rs`:
- Around line 93-101: Update the base duration selection in the content
transition calculation to use motion.exit_duration_ms when self.collapsed is
true and motion.enter_duration_ms otherwise. Remove the redundant
spring_duration_ms dependency so collapse respects exit timing while expansion
preserves enter timing.
- Around line 68-74: Rename the render_example parameter _window to window and
update its keyed_presence usage accordingly, preserving the existing behavior.
In `@crates/ui/src/accordion.rs`:
- Line 12: Update the accordion closing animation setup around close_anim and
chevron_close_anim to use the exit-duration-aware exit_animation(motion,
reduced_motion) pattern, matching dialog.rs, instead of cloning
standard_animation or using spring_animation for exit. Keep spring_animation
only for the entering overshoot, and preserve close_duration from
motion.exit_duration_ms so visual closing and keyed_presence unmount use the
same timing.
In `@crates/ui/src/animation.rs`:
- Around line 165-167: Update exit_duration to widen exit_duration_ms before
adding EXIT_UNMOUNT_GRACE_MS and use saturating arithmetic for the addition,
ensuring large deserialized values cannot overflow or wrap while preserving the
Duration conversion.
In `@crates/ui/src/command_palette/view.rs`:
- Line 385: Update the match-highlight styling at the text_color call to avoid
using the stroke-only list_active_border token. Use the appropriate
accent/primary foreground token or apply the same contrast-corrected flyout
foreground helper used by the surrounding flyout text styling.
- Around line 250-258: Update the selected-row styling in the command palette
row builder so every row reserves the same 1px border within the fixed
self.item_height layout. Apply border_1() unconditionally with the appropriate
transparent border color for unselected rows, while preserving
list_active_border for selected enabled rows and the existing disabled styling.
In `@crates/ui/src/dialog.rs`:
- Around line 328-340: Add a Dialog test module covering defer_close builder
behavior and close-state handling. Exercise should_defer_close and
is_layer_closing for deferred close, reduced-motion, and normal paths, asserting
the dialog remains mounted only when the configured behavior requires it and
that the closing state is reported correctly.
In `@crates/ui/src/input/popovers/hover_popover.rs`:
- Around line 205-209: Replace the prose padding token in the hover
documentation rendering with content_padding instead of item_padding_x. Apply
the same change in crates/ui/src/input/popovers/hover_popover.rs lines 205-209
and crates/ui/src/input/popovers/completion_menu.rs lines 445-450, using
FlyoutTokens::new(cx).content_padding in the completion menu site.
In `@crates/ui/src/progress/progress.rs`:
- Around line 124-130: The progress control in
crates/ui/src/progress/progress.rs lines 124-130 and the switch control in
crates/ui/src/switch.rs lines 167-173 should use animation::standard_animation
with the existing motion and reduced_motion values instead of rebuilding the
animation from enter_duration_ms and standard_easing; retain each local Duration
for its value-commit or toggle-state commit timer.
In `@crates/ui/src/select.rs`:
- Around line 941-949: Update the vertical_direction calculation near
FlyoutSlide::vertical to use -1.0 for Anchor::Top* placements, matching
select_menu_placement and the direction used by popover.rs and hover_card.rs;
preserve the existing bottom-anchor behavior and horizontal handling.
In `@crates/ui/src/sidebar_shell/mod.rs`:
- Around line 407-412: Add a short clarifying comment above the
`surface_render_width` calculation explaining that it is used only for
noise-overlay texture sizing through `render_noise_overlay_unclipped`, while the
surface’s actual layout width comes from `.size_full()` and the animated outer
width.
In `@crates/ui/src/sidebar/menu.rs`:
- Around line 432-434: Update the submenu animation setup around
submenu_transform_anim to provide distinct enter and exit animations, using
exit_animation for the collapsing transform just as chevron_close_anim does.
Apply the exit transform during the submenu closing path while retaining
spring_animation for opening, so transform timing matches the layout animation’s
close_duration.
In `@crates/ui/src/surface/flyout.rs`:
- Around line 26-45: Expose the flyout geometry constants INSET, ITEM_PADDING_X,
ITEM_GAP, ICON_GAP, ACCESSORY_GAP, SECTION_HEADER_HEIGHT, SEPARATOR_MARGIN,
CONTENT_PADDING, MIN_WIDTH, and MAX_WIDTH as Theme/schema values using their
current defaults. Update flyout layout code to read these values through
cx.theme() via the ActiveTheme trait, removing direct reliance on the fixed
constants while preserving existing defaults.
In `@crates/ui/src/theme/mod.rs`:
- Around line 43-56: Update the motion tokens doc comment above the theme
definitions to state that there are four easing curves, matching the actual
easing fields and the wording in the theme documentation.
In `@docs/docs/components/menu.md`:
- Around line 13-18: Replace fixed pixel values in the menu documentation with
the flyout token names radius, inset, item_height, and content_padding, matching
the theme-driven definitions in flyout.rs. Apply the corresponding terminology
update in docs/docs/components/menu.md at lines 13-18 and
docs/docs/components/popover.md at line 11; preserve the surrounding behavior
descriptions.
---
Outside diff comments:
In `@crates/ui/src/animation.rs`:
- Around line 481-541: Add tests for the pure motion helpers in the existing
tests module: verify exit_duration(&ThemeMotion::default()) exceeds the duration
represented by motion.exit_duration_ms, and cover FlyoutSlide::offset
completion/progress behavior using a vertical slide, including zero offset at
progress 1.0 and the expected initial offset at progress 0.0. Also add
assertions for FlyoutSlide::horizontal and ::vertical distance derivation and
direction handling where applicable.
In `@crates/ui/src/theme/default-theme.json`:
- Around line 38-63: Remove all mica_base_* and acrylic_* entries from both
material blocks in the default theme JSON, leaving the remaining material
settings unchanged so the file matches ThemeMaterialConfig.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ab6acc27-1642-4de0-8eb2-baf398fcfff7
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (58)
.agents/skills/apple-design/SKILL.mdCargo.tomlcrates/story/src/stories/command_palette_story.rscrates/story/src/stories/dropdown_button_story.rscrates/story/src/stories/floating_sidebar_story.rscrates/story/src/stories/menu_story.rscrates/story/src/stories/popover_story.rscrates/ui/src/accordion.rscrates/ui/src/animation.rscrates/ui/src/badge.rscrates/ui/src/button/dropdown_button.rscrates/ui/src/checkbox.rscrates/ui/src/closing_scope.rscrates/ui/src/collapsible.rscrates/ui/src/command_palette/mod.rscrates/ui/src/command_palette/state.rscrates/ui/src/command_palette/view.rscrates/ui/src/dialog.rscrates/ui/src/floating_sidebar/mod.rscrates/ui/src/global_state.rscrates/ui/src/hover_card.rscrates/ui/src/input/popovers/code_action_menu.rscrates/ui/src/input/popovers/completion_menu.rscrates/ui/src/input/popovers/hover_popover.rscrates/ui/src/input/popovers/mod.rscrates/ui/src/lib.rscrates/ui/src/menu/context_menu.rscrates/ui/src/menu/dropdown_menu.rscrates/ui/src/menu/menu_item.rscrates/ui/src/menu/popup_menu.rscrates/ui/src/notification.rscrates/ui/src/popover.rscrates/ui/src/progress/progress.rscrates/ui/src/progress/progress_circle.rscrates/ui/src/root.rscrates/ui/src/select.rscrates/ui/src/sheet.rscrates/ui/src/sidebar/group.rscrates/ui/src/sidebar/menu.rscrates/ui/src/sidebar/mod.rscrates/ui/src/sidebar_shell/mod.rscrates/ui/src/styled.rscrates/ui/src/surface/flyout.rscrates/ui/src/surface/mod.rscrates/ui/src/switch.rscrates/ui/src/theme/contrast.rscrates/ui/src/theme/default-theme.jsoncrates/ui/src/theme/fluent_tokens.rscrates/ui/src/theme/mod.rscrates/ui/src/theme/schema.rscrates/ui/src/theme/tests.rscrates/ui/src/time/date_picker.rsdocs/docs/components/dropdown_button.mddocs/docs/components/menu.mddocs/docs/components/popover.mddocs/docs/components/sidebar.mddocs/docs/theme.mdvendor/gpui
The transactional startup smoke deliberately runs a binary that exits 2, so cargo writes 'process didn't exit successfully' to stderr. Actions runs pwsh with $ErrorActionPreference='Stop', where a native command's stderr under 2>&1 raises a terminating NativeCommandError — so the script died before reaching its own exit-code and marker checks, and the step failed without ever printing why. The app under test was behaving correctly the whole time: it emitted APP_SHELL_FAIL_START_REACHED and exited 2, exactly as the macOS leg asserts.
The previous attempt set $ErrorActionPreference='Continue' and still failed, so the terminating-error theory was only half the story: the runner appends `exit $LASTEXITCODE` after the step script, which propagates cargo's exit 2 as a step failure even when every assertion in the step passes. Redirect all streams to a file instead of piping through 2>&1, disable native-error promotion, and exit explicitly so the step's status reflects the assertions rather than the exit code of a command that is supposed to fail.
BumpyClock
added a commit
that referenced
this pull request
Jul 28, 2026
Several surfaces built their closing animation from the enter token while their presence unmounted on the exit token, so the visual animation was still running when its content disappeared. The accordion did this for both its layout and chevron animations, and its presence window also skipped the unmount grace; the sidebar submenu sprang back on collapse where its own chevron correctly used the exit token. The select dropdown slid the opposite way from every other flyout: Anchor::Top* means the menu sits below the trigger, which popover and hover card give -1.0, while select matched on Anchor::Bottom*. Command palette rows gained a 1px border only when selected inside a fixed-height row, shrinking the content box and shifting the label as selection moved. The border is now reserved on every row and only its color changes. exit_duration added two u16 values before widening. exit_duration_ms is deserialized from theme JSON, so a large value would wrap to a near-zero exit window and unmount on the first frame. Both progress controls rebuilt the standard curve inline from enter_duration_ms and standard_easing rather than calling standard_animation, so retuning that curve would have skipped them. Also fixes the floating sidebar story, where max() against the enter duration made the collapse branch a no-op, and a doc comment that counted five easing curves where there are four. Found by CodeRabbit review on #16.
BumpyClock
added a commit
that referenced
this pull request
Jul 28, 2026
defer_close changes should_defer_close, which decides whether a dialog stays mounted for its exit animation, but nothing pinned the behavior. Covers the builder defaults and all three branches: an animating dialog defers, a non-animating one that opted into defer_close also defers, and one that opted into neither keeps the immediate-unmount path. Reduced motion overrides both opt-ins, so a deferred window can never hold a dialog past its dismissal. Also aligns the hover popover with the 12px flyout rail. Its comment said it padded to match the completion menu labels beside it, but item_padding_x alone sits 4px inside the rail those labels use (inset + item_padding_x); content_padding is the token that lands on it. Renames a used _window parameter in the floating sidebar story. Found by CodeRabbit review on #16.
BumpyClock
added a commit
that referenced
this pull request
Jul 28, 2026
Search match highlights were colored with list_active_border — a border token used as text. Borders carry no legibility guarantee against the flyout material, so the emphasis that explains why a row matched could land at arbitrary contrast. Adds flyout_accent_foreground alongside the existing primary, secondary and disabled roles: theme.primary, run through the same minimum correction, so the accent stays readable on materials the theme never tuned it against. Found by CodeRabbit review on #16.
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.
A pass over the shared flyout surfaces — popover, popup/context menu, submenu, select, command palette, hover card, editor popovers, sheet, dialog — covering layout tokens, text contrast, surface tint, and motion. Each area landed as its own commit with measured before/after.
Layout tokens
Flyouts each carried their own geometry: menus hardcoded a 6px radius, popover and editor popovers used
theme.radius, select clamped to 8px, the palette usedradius_lg, and rows sat at a flat 4px unrelated to any container.FlyoutTokensderives all of it from theme tokens — container radius fromradius_lg, row radius asradius − insetso corners stay concentric, one 12px left rail, two type steps fromtheme.typography, two intra-row gaps.Also drops
StyledExt::popover_style, which hand-rolled the flyout material in parallel withSurfacePresetand had drifted — it missed the noise overlay and the opaque fallback used when backdrop blur is off.Text contrast
muted_foregroundis tuned against the window background, but flyouts sit above it — noticeably lighter in dark mode — so secondary text lost roughly a stop. All 22 bundled dark themes measured sub-AA on flyout material, Default Dark at 3.74:1 against a 4.5:1 floor.theme::contrastadds WCAG math that composites through translucent surfaces.flyout_secondary_foregroundkeeps the theme's token wherever it already passes and otherwise bisects lightness away from the material for the smallest correction that clears AA, preserving hue, saturation and alpha. Verified to keep theme character: Catppuccin stays lavender, Solarized sea-grey, Gruvbox warm.Also fixes a three-role collapse where section headers, disabled labels and enabled subtitles all rendered at luma 128, while a disabled row's own shortcut and icon sat at 142 — brighter than the label they belonged to. Now a three-step ladder (250/142/92 in Default Dark) holding hue across themes.
Surface tint
Every elevated surface resolved its color from fixed Fluent constants — flyout
#2C2C2C/#FCFCFC, panel#202020/#F3F3F3, card pure white — so under a tinted theme the chrome carried the theme while menus, panels and cards stayed neutral grey.All now blend the theme anchor toward the mode's elevation pole, calibrated so neutral Default reproduces the old Fluent values pixel-identically while tinted themes keep their hue. Solarized Dark menu material goes (43,43,43) → (42,68,77); panel (31,33,34) → (34,61,71). Removes 8 Fluent constants, 8 material fields and 48 lines of schema wiring.
Collapsing onto the plain
Popover/Sidebarsources was measured and rejected:popoverresolves equal tobackgroundin every bundled theme, so Default Dark menus would have dropped to#0A0A0Aand vanished into the window.Motion
Four surfaces had no animation at all despite full wiring. The submenu instrumented 20
Enteringframes while its pixels showed a one-frame pop — opacity and transform wrappers have no effect on ananchored()subtree unless it renders throughdeferred(). Hover card, select and palette simply had no exit path.Every exit was also truncated: presence close windows ran on an 83ms fade token while close animations ran 187ms, so surfaces unmounted at 44% of their curve.
ThemeMotionconsolidates 18 fields to 10 — four durations, one spring, four easings. Two durations had zero consumers, one easing was dead and byte-identical to another, and the mild/medium springs measured within three frames of each other at 60Hz.The dialog layer gains a deferred-unmount contract so any hosted surface can animate out with static chrome, which the palette and sheet needed. Five duplicated ~90-line open/close blocks collapse into one shared presence helper.
Measured at 60fps CFR: submenu 1-frame pop → 182/232ms, hover card → 166/216ms, select → 249ms both ways, palette close → 232ms, sheet close → 183ms, popover close from a truncated 99ms → complete 215ms.
gpui bump
Bumps to a fork build where backdrop blur scales with element opacity (BumpyClock/gpui#9), which removes the last artifact — a residual single-frame step where a faded-out flyout still painted its blur — and lets the palette drop its blur-off closing workaround.
Verification
cargo fmt,cargo clippy --workspace --all-targets,cargo test --all→ 358 passed, 0 failed,typosandmacheteclean. Visual and motion claims measured from 60fps recordings and retina crops rather than eyeballed; several early findings were discarded when pixel-level crops disproved them.Verified on macOS. The gpui change carries unverified DirectX/HLSL and wgpu paths — see the linked PR.
Note
Unify flyout visuals, motion tokens, and surface tinting across menus, popovers, and overlays
FlyoutTokensin crates/ui/src/surface/flyout.rs to provide shared geometry, typography, and spacing constants;PopupMenu,Select,Popover,HoverCard,ContextMenu, and editor popovers all derive sizing and color from these tokens.flyout_presenceandflyout_motion(spring slide + fade in, fade + slide-back out) across menus, popovers, select dropdowns, and hover cards.ThemeMotionto exit/enter/emphasis durations, a single spring config, and three easing curves (decelerate,standard,emphasis), removing the previous fast/slow/soft-dismiss/point-to-point fields; all animation helpers (enter_animation,exit_animation,spring_animation, etc.) are renamed and wired to the new tokens.ClosingScopeand aclosingflag onActiveSheet/Sheet/Dialogso surfaces remain mounted during their exit window and inner content can drive its own exit animation.contrastmodule with WCAG compositing math and acontrast_adjustedhelper; tests assert bundled themes meet a minimum text-contrast floor over flyout material.FloatingSidebarcollapse/expand width with a proportionally scaled duration, hiding the resizer during transitions and respecting reduced motion.ThemeMotionis a breaking schema change — any theme JSON or code using the old duration/easing/spring field names will fail to compile or be silently ignored.Macroscope summarized e5ea80a.