feat(client): hover cross-fade for the settings rows and the context menu (UI/UX v3 P3b2a) - #80
Merged
Merged
Conversation
…rvey The P3b design document claimed hover existed in exactly two places. It missed the tab bar and the custom title bar's window buttons, both of which are chrome that responds to the pointer. Scope is now all four models. Adds a forward-pointing correction note to the parent document rather than rewriting its reasoning. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
Self-review of the plan found a real defect in the design's single-Timed HoverTransition: the sum-to-1 invariant only holds when the outgoing item was already at weight 1, so interrupting a fade mid-flight makes the incoming item jump. Sweeping a list crosses row boundaries faster than the 100 ms fade, making that the common case. Splits it into two timers and adds a test that pins the defect. Also records the remaining single-slot limitation rather than hiding it. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
The hover cross-fade (UI/UX v3 P3b2a) replaced its only reader with a weight from HoverTransition, leaving the field written by nine builders and read by nothing. SettingsPanel.hover_widget is already the truth for whether the pointer is over a control, so the boolean was redundant rather than merely unused.
Deleting the hovered field also deleted the only unit test exercising the focused() and tooltip() builder methods directly, rather than through an unrelated field. Re-add a trimmed version scoped to what still exists.
…mments Fix wave from the P3b2 whole-branch review (zero Critical/Important, three of four minors picked up in this pass): 1. Panel-closed pointer-motion branch (mouse.rs) cleared hover_widget and theme_hover_preview on Esc dismiss but left hover_transition pointed at the last hovered row. The fade-out that should start immediately instead waited for the next pointer move (up to 100ms later), and has_active_animation stayed true for a closed panel. Retarget to None alongside the existing clears, mirroring the open-panel branch's AnimationsConfig::clone() borrow workaround. Added a state-level test pinning that the retarget settles rather than running forever. 2. hover.rs: document why Default's throwaway Instant::now() is safe (zero duration short-circuits Timed's start-dependent paths). 3. color_util.rs: note that lerp_rgba_clamps_t's endpoints are exactly representable, so unlike lerp_rgba_hits_both_endpoints_exactly it can't distinguish the early-return implementation from a clamp-then-lerp one. HoverTransition::target()'s #[allow(dead_code)] is left as-is per the review's own recommendation to revisit at P3b2b.
Coverage reportGenerated by |
7 tasks
mizu-jun
added a commit
that referenced
this pull request
Aug 28, 2026
Three accuracy fixes to the phase-tracking steering document: - Add the missing P3b1 entry (shipped via #79, squash 5d6e167, merged to master): the shared SurfaceMotion timer pair and open/close motion for eleven overlay surfaces, including the redacted password-modal ghost. - Fix P3b2, which claimed "shipped via #80" while #80 is still open. A checklist that overstates what is on master is worse than one that lags behind -- it's exactly the failure mode the CONFIGURATION.md reconciliation (#73) exists to prevent. P3b2a is pending review in #80, P3b2b in #81; the item is unchecked since neither half has merged. - Replace the dangling "this branch" reference with #81, matching every other entry's PR-number provenance convention. No code changes.
7 tasks
mizu-jun
added a commit
that referenced
this pull request
Aug 29, 2026
…/UX v3 P3b2b) (#82) * docs(plan): P3b2b implementation plan for the tab bar and window buttons Records two traps found by measuring, neither of which is in the design: the tab bar's is_hovered also gates whether the tear-out and close buttons are drawn (a behavioural use that must keep the boolean, since a button drawn at weight 0.05 is still clickable), and hovered_window_button has a second write site in the Windows snap-layout event handler. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering> * feat(client): cross-fade the tab bar's hover tint * test(client): remove decorative tab-hover gate test a_tab_transition_that_is_never_retargeted_stays_quiet could not fail: it only exercised HoverTransition::default(), already covered by a_fully_idle_state_wants_no_animation_frames, and never called retarget, so it did not exercise the config gate in mouse.rs even by proxy. A test that cannot fail but carries the name of a requirement is worse than no test - it creates the appearance of coverage. The tab_bar.hover_highlight gate is verified by code inspection only. Covering it properly needs an EventHandler test harness this crate does not have, which is out of scope for P3b2b. * feat(client): cross-fade the window buttons' hover fill and glyph * docs(client): record the hover-model rule, and resolve HoverTransition::target target() has no production caller across all four hover models, so gate it behind #[cfg(test)] instead of #[allow(dead_code)] -- following AnimationManager::tick_by_dt's precedent -- rather than deleting it and losing the retarget-moved-the-target assertion the weight checks alone cannot express. Also closes out P3b2 in the plan doc and adds the hover-model rule to nexterm-client-gpu/CLAUDE.md. * docs(plan): correct P3b provenance in ui-ux-modernization-v3 Three accuracy fixes to the phase-tracking steering document: - Add the missing P3b1 entry (shipped via #79, squash 5d6e167, merged to master): the shared SurfaceMotion timer pair and open/close motion for eleven overlay surfaces, including the redacted password-modal ghost. - Fix P3b2, which claimed "shipped via #80" while #80 is still open. A checklist that overstates what is on master is worse than one that lags behind -- it's exactly the failure mode the CONFIGURATION.md reconciliation (#73) exists to prevent. P3b2a is pending review in #80, P3b2b in #81; the item is unchecked since neither half has merged. - Replace the dangling "this branch" reference with #81, matching every other entry's PR-number provenance convention. No code changes. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Happy <yesreply@happy.engineering>
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.
Summary
P3b1 (#79) gave eleven overlay surfaces an entrance and an exit. P3b2 does the same for the state a pointer creates. This is P3b2a — the two overlay hover models; the tab bar and the window buttons are P3b2b.
Two surfaces stop snapping:
draw_row_backgroundused to paintsurface_3atalpha * HOVER_ALPHAwhen a boolean was true and nothing when it was false. It now scales that alpha by a hover weight.text_secondary→text_primary).HoverTransition<Id>uses two timers, and that is the pointThe obvious design is a single
Timedwith the outgoing item at1 - progressand the incoming atprogress, always summing to 1. It is wrong: that invariant only holds when the outgoing item was already at weight 1. Enter row A and, 50 ms later while A is still at 0.5, move to row B — a single timer makes B jump to 0.5 on the frame the pointer crosses the boundary. Sweeping a list crosses row boundaries faster than the 100 ms fade, so that jump is the common case, not a corner.So the outgoing item decays from the weight it actually held and the incoming rises from the weight it actually held (0 normally, or its partly-decayed value when the pointer comes back). The pair does not sum to 1 mid-handoff, which is correct — at that instant neither row is fully hovered.
interrupting_mid_fade_jumps_neither_itemexists specifically to fail against the single-timer form.This was caught while writing the plan, not while writing the code; the parent design carried the naive form.
The mechanism is a scalar, not a post-pass
P3b1's visual was
apply_surface_fade— scalecolor[3]over the vertex range a builder appended. That does not transfer to hover. Three of the four hover models interpolate more than one property, and two compute the hovered colour by brightening the resting one rather than adding a layer. Fading a tab's quad would fade the tab out of the bar, not into its hover colour.So each draw site asks for a weight and does its own interpolation, and the two kinds stay distinct:
color_util::lerp_rgba, which is exact at both endpoints via early returns.Lerping an additive fill would have been wrong twice over: it assumes what sits behind the item, and it keeps painting a quad at weight 0.
Also in here
WidgetSpec.hoveredis removed. The cross-fade left it written by nine builders and read by nothing but one test;SettingsPanel.hover_widgetwas already the truth for "is the pointer over this". Gone with it: thehovered()builder, twelve call sites, and nine locals that existed only to feed them. Its accidental test-coverage loss (the deleted test also coveredfocused()andtooltip()) was caught and restored in the next commit.2026-08-28-p3b-motion-application-design.md) gets a correction note. It claimed hover existed in exactly two places; it missed the tab bar and the window buttons, which is why P3b2's scope is four models rather than two. The note is inserted at the heading it corrects and points forward, rather than rewriting an accepted document's reasoning.The logical hover state stays the truth throughout:
hover_widgetstill gates the tooltip dwell timer,ContextMenu.hoveredstill drives click dispatch and the AccessKit focus node, and neither is consulted for rendering weight or vice versa.Known and accepted
One slot means one item fading out at a time. Sweeping across five rows in 200 ms drops the three intermediate rows to 0 as each is replaced, leaving a trail that cuts off rather than one that fades. A fixed-capacity
id → Timedmap would fix it behind an unchangedweight(), so no consumer would move. Documented in the design rather than discovered later.HoverTransition::target()has no production caller and carries an#[allow(dead_code)]. Revisit when P3b2b lands the other two models — if neither needs it, delete it then.Test plan
cargo test --workspace— 0 failures (nexterm-client-gpu: 1009 passed), re-run after rebasing onto the squashed feat(client): shared SurfaceMotion and open/close motion for eleven overlay surfaces (UI/UX v3 P3b1) #79cargo clippy --workspace --all-targets -- -D warnings— cleancargo fmt --check— cleanHoverTransitionpinned directly, including the cases that matter: interrupting mid-fade jumps neither item; returning to a fading item resumes rather than restarts it; leaving entirely still fades the last item out; retargeting the same id is a no-op that does not restart the fade; a zero duration snaps (the whole reduced-motion path).lerp_rgba— endpoints exact, midpoint linear,tclamped.surface_2and no hover fill, whatever the weight says.a_fully_idle_state_wants_no_animation_framesstill green with two more clauses in the aggregate.Cargo.lock,nexterm-i18n/locales/,docs/CONFIGURATION.mdorpkg/flatpak/change, so no flatpak sources regeneration and no key-parity risk.Not verified here, stated plainly: whether 100 ms reads as responsive or as lag, and whether the context menu's three simultaneous interpolations read as one coherent motion or as three things moving. Neither is judgeable from CI or from a container, and motion cannot be captured by the repo's screenshot convention. This joins the on-device verification backlog, which the P3a and P3b1 entries already note has no established capture format for transitions.
Notes for review
renderer/event_handler/mouse.rs— the closed-panel branch now retargets the transition toNonealongside its existing clears. Without it, dismissing the panel with Esc while hovering kept asking for frames for up to 100 ms after the panel was logically closed.WidgetThemeliterals inrenderer/overlay/settings/*_tab.rseach pass the panel's own transition. A literal "fixed" with a freshHoverTransition::default()would compile and silently never animate, so that is worth a glance.Design:
docs/superpowers/specs/2026-08-29-p3b2-hover-crossfade-design.mdPlan:
docs/superpowers/plans/2026-08-29-p3b2a-hover-crossfade.md🤖 Generated with Claude Code