feat(client): shared SurfaceMotion and open/close motion for eleven overlay surfaces (UI/UX v3 P3b1) - #79
Merged
Merged
Conversation
Hover cross-fade for the two hover models that exist, a pressed widget state that does not exist yet, and open/close motion for ten stored surfaces plus the tooltip, via a shared SurfaceMotion that P3a's hand-written settings-panel fields move onto. PasswordModal gets a redacted ghost rather than a clone: its input is a private Zeroizing<String> whose purpose is to minimise how long the secret exists, and the renderer already reads only input_len() under the H-6 comment. Ships with a test so a future 'just clone the modal' refactor fails rather than passes. 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>
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 tautological `sp.is_open` assertion always passed because `open()` sets it unconditionally, regardless of whether the exit animation was actually cancelled. Replace it with a check that samples progress after reopening: only a resumed entrance rises toward 1.0, while an un-cancelled exit would keep falling toward 0. Verified by temporarily neutering `SurfaceMotion::open`'s `self.closing = None` and confirming the new assertion fails (then reverting the injection). Sanctioned deviation from the P3b1 task-3 brief's literal wording, per review finding and controller ruling.
…ender-only ghosts
…d_modal Fix wave from the P3b1 whole-branch review, three findings: - Important: the AccessKit click handler for a password-auth host assigned `host_manager.password_modal` directly instead of calling `HostManager::show_password_modal`, so a screen-reader user got the modal with no entrance animation and a possibly stale `password_modal_opening` timer. Routed it through the helper, mirroring the existing keyboard path. A sweep for the same bypass shape across the other three Option-shaped surfaces (context menu, consent dialog, close-window dialog) found no further instances. - Minor: nexterm-client-gpu/CLAUDE.md's "Adding an overlay surface" note did not account for surfaces drawn from a nested helper (the tooltip's apply_surface_fade call lives in theme_tab.rs, not render_frame.rs). Added a clause covering that case. - Documented, not fixed: the four Option-shaped surfaces' show_* helpers restart their entrance from Timed::new(now, ...) on reopen instead of resuming mid-fade like the bool-shaped surfaces' SurfaceMotion::open, causing a visible alpha jump when reopened inside their ~100-150ms exit window. Recorded as an accepted decision in the P3b1 plan's closing checklist rather than fixed now.
Coverage reportGenerated by |
This was referenced Aug 28, 2026
Merged
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
P3a built the time-based motion foundation (
Timed,Curve,duration) and proved it on one surface. This PR spends it on the other ten, plus the tooltip, on shared machinery — the P3b1 row of the P3b design's delivery table. Hover cross-fade (P3b2) and thepressedwidget state (P3b3) are deliberately out of scope.Eleven surfaces now arrive and leave rather than appearing and vanishing:
SurfaceMotion(animations/surface.rs) — an open/closeTimedpair, lifted from P3a's hand-written settings-panel logic including its two ordering rules. Used by the sixbool-shaped surfaces: settings panel (retrofitted), command palette, macro picker, host manager, block-name modal, file-transfer dialog.Option-shaped surfaces, where theOptionis the openness so dismissing destroys the content the exit still needs to draw: context menu, close-window dialog, consent dialog, password modal. The live field goesNoneimmediately — nothing is clickable, hoverable or answerable during a fade — while a clone moves into a(content, Timed)slot only the renderer reads.PasswordModalGhostcarries no password, onlyinput_len: usize, and a newPasswordModalViewmakes the existing advisoryHIGH H-6boundary structural — the builder now has no field the secret could reach, live or fading. TheZeroizing<String>is dropped at dismiss time, not when the animation ends.tick_tooltipturns the hover-dwell predicate into motion once per frame, with a snapshot so the exit can still draw afterhover_widgethas cleared.apply_surface_fade(renderer/overlay/fade.rs) — the visual: scalecolor[3]over the vertex range a surface's builder appended. Both overlay shaders take straight alpha and premultiply in the fragment stage, so this is correct for the background pipeline, the glyph pipeline and acrylic panel fills (acrylic_mixblendsrgbonly). Applying it after the builder is what let ten builders stay untouched.is_open(or the liveOption) remains the single truth for input routing and the AccessKit tree and still flips the instant the user acts; only the renderer consults the motion. Durations and curves come from the spec's Fluent 2 table and were not re-tuned.Two decisions the design left open
Known and accepted
Reopening an
Option-shaped surface inside its exit window restarts the entrance from 0, where the sixbool-shaped surfaces resume from the value already on screen (SurfaceMotion::opencallsTimed::resuming_at; the fourshow_*helpers callTimed::new). Resuming would mean inverting each ghost's progress across four helpers — new behaviour beyond this phase, wanting its own tests — and the affected windows are 100–150 ms. Recorded in the plan's closing section so it reads as decided rather than missed.The gate+fade block in
render_frame.rsis now nine near-identical copies. Extraction was deliberately deferred: the sites are mechanically diffable,render_frame.rshas no unit-test net (GPU-bound), and a closure-based extraction over the&mut Vec→&mut [..]borrow pattern is easy to get subtly wrong. Worth revisiting once P3b2/P3b3 fix the final call-site count.Test plan
cargo test --workspace— 0 failures (nexterm-client-gpu: 994 passed)cargo clippy --workspace --all-targets -- -D warnings— cleancargo fmt --check— cleana_fully_idle_state_wants_no_animation_frames— the phase's acceptance criterion: a state with nothing animating must not request frames, asserted for both a real duration and 0. Eleven clauses inhas_active_animationare eleven ways for that to regress.SurfaceMotionpinned directly: open runs 0 → 1; close keeps the surface visible while fading; reopening mid-fade is continuous;retiredrops only a finished exit; a zero duration is finished on creation (the whole reduced-motion path).retire.a_fading_consent_dialog_cannot_be_answered—pending_consentisNonefrom the instant the user answers, before the fade begins.the_ghost_carries_no_string_the_secret_could_live_in— asize_oftripwire so a future "just clone the modal" refactor fails CI rather than passing review.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 the durations feel right — whether a 300 ms entrance reads as arrival or as lag, whether a consent prompt that is still on screen after it stopped accepting input is acceptable, whether the context menu's 150/100 ms pair feels immediate. None of that 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 entry already notes has no established capture format for transitions.
Notes for review
renderer/event_handler/accessibility.rs— the AccessKit path that opens the password modal was bypassingshow_password_modaland so skipped its entrance. Found by the whole-branch review, fixed in the last commit; a sweep confirmed no other bypass of anyshow_*/dismiss_*helper exists.nexterm-client-gpu/CLAUDE.mdgains the rule this phase learned: a new surface must be added in three places — thehas_active_animationclause, thelifecycle.rsretire call, and the recorded-rangeapply_surface_fadecall — or it will silently never animate. There is no registry that catches an omission.Spec:
docs/superpowers/specs/2026-08-28-p3b-motion-application-design.mdPlan:
docs/superpowers/plans/2026-08-28-p3b1-surface-motion.md🤖 Generated with Claude Code