feat(web): add opt-in panel collapse animations - #8026
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Two behavioral issues in the opt-in panel animation wiring. Everything else (sidebar transition gating, PanelPresence composition, settings row/search entry) looks consistent with the existing primitives.
Posted via Macroscope — UI Consistency
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR introduces a new persisted opt-in animation feature across several panels, including measured CSS transitions and deferred store/URL close behavior. Its cross-cutting layout and interaction changes are substantial enough to require human review. You can add or adjust custom eligibility rules. Learn more. |
There was a problem hiding this comment.
UI consistency: one new finding on the sidebar animation gate. Two findings from the previous run (conditional hook in usePanelAnimations, hidden gating in PersistentThreadTerminalDrawer) are still present on this head and are not re-posted here.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
Reviewed the panel-animation opt-in for UI consistency. The earlier hook-order and background-drawer hidden issues look resolved. One new layout regression stands out: inserting a PanelPresence wrapper between the flex row and PreviewPanelShell breaks the shell's resize clamp, which measures its immediate parent. Details inline.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One issue in the new PanelPresence primitive (the exit-animation freeze never captures live children, so the pull-requests panel snaps instead of animating), plus a minor settings-row accessibility naming inconsistency. The earlier findings (clamp measurement, reduced motion, conditional hook, background-drawer hiding, ungated sidebar/header transitions) all look addressed.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One issue in the new PanelPresence primitive (the exit-animation freeze never captures live children, so the pull-requests panel snaps instead of animating), plus a minor settings-row accessibility naming inconsistency. The earlier findings (clamp measurement, reduced motion, conditional hook, background-drawer hiding, ungated sidebar/header transitions) all look addressed.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One finding: the collapse phase is committed in a passive effect, so the closing frame paints before the exit animation starts.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One finding: an in-flight collapse/expand can be aborted by an unrelated re-render, because the phase machine re-runs whenever animate changes, and ChatView's terminal drawer computes animate from a ref that is updated in an effect (so it is only true for the toggle render).
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One finding: the exit path in PanelPresence still unmounts and remounts the panel it is animating away. Details inline.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
UI consistency review: one regression found — the new right-panel exit animation desynchronizes the titlebar control cluster from the panel edge at both inline call sites.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One finding on the new panel-collapse animation: closing a maximized right panel can't hit its own animation guard, so the panel snaps from full width to its stored inline width before collapsing.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One new finding on the presence hook: in right-panel sheet mode the phase can never settle, which strands the titlebar controls. Previously reported items that are still open (the !rightPanelMaximized animate guard on close) are not repeated here.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One finding: the exit collapse for a maximized right panel still measures after its sibling column has already reclaimed half the row, so the close jumps before it animates. Details inline.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One finding: closing the right panel while it is maximized still starts the collapse from roughly half the row, because only the panel wrapper's layout class is held through the exit — the sibling chat column's is not.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One finding: the always-present sizer div inside PanelPresence breaks the flex chain the maximized right panel relies on, so maximizing no longer fills the row. Details inline.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One finding on the animated close of a maximized right panel: the exit collapses toward the wrong edge and reveals empty background before the chat column reappears. Details inline.
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
One interaction regression and one ownership note on the new panel-presence work. Details inline.
Posted via Macroscope — UI Consistency
github-actions
Bot
added
the
size:L
There was a problem hiding this comment.
Two findings on the new PanelCollapse layer. Details inline.
Posted via Macroscope — UI Consistency
- Retiring a flight now clears its fallback timer so retargeting an entrance into an exit cannot fire the old settle early (Cursor+Macroscope). - Identity switches cancel entrance flights, not just exits. - Disabling animations mid-flight snaps to the current endpoint instead of finishing animated. - The collapse state object is memoized so ChatView toggle callbacks and the global keydown effect keep stable identities across renders. - PanelCollapseFrame pins the content box at flight size so a maximized flex-1 shell clips during close instead of reflowing every frame.
UtkarshUsername
changed the title
UtkarshUsername
changed the title
There was a problem hiding this comment.
One finding on the new PanelCollapseFrame structure added in f7519dc: the flight-only sizer div remounts the panel subtree, which tears down live panel content (Ghostty terminal surfaces, panel-local state) on every animated open/close.
Posted via Macroscope — UI Consistency
The pinned sizer introduced for maximized closes was conditional on the flight, which flips element types at that tree slot and remounts the whole panel subtree on every animated open/close (Ghostty surfaces were disposed and recreated mid-toggle). The box is now always mounted with display:contents while idle, so child fiber identity never changes and idle layout stays identical to the bare panel.
There was a problem hiding this comment.
One finding: the pinned content box in PanelCollapseFrame only sizes the animated axis, so the panel loses its h-full for the whole width collapse at both dimension="width" call sites.
Posted via Macroscope — UI Consistency
The sizer box pinned only the animated axis, so at the flex-none width call sites its height stayed auto and h-full content shrank to intrinsic height for the duration of a close/open. Pin the cross axis to 100% so the panel is clipped at full size, matching the height path.
A pending exit always ran its captured onClose, so reopening intent that landed during the 200ms collapse got wiped: picking another PR row cleared the fresh selection, and a surface opened in chat was closed again by the stale commit. requestClose now snapshots a supersedeKey and skips the commit when it changed by settle time. Call sites key on surface/session identity. Direct store closes during an exit still commit immediately since their onClose is idempotent.
github-actions
Bot
added
size:XL
There was a problem hiding this comment.
One finding: the pull-requests route keys its deferred-close supersede check differently from the ChatView call site, which leaves the case the comment says it covers reachable. Details inline.
Posted via Macroscope — UI Consistency
The supersede check ran on every settle, so a mid-collapse thread switch compared the old key against the new thread key and almost always dropped the commit it existed to force, stranding the previous panel as open. Explicit finishes (toggle settle, identity switch, bypassing store close) now always commit; only natural completion honors supersession. Also keys supersession on the active surface/session id: activating a PR row that already has a tab leaves the surfaces array unchanged (rightPanelStore upsert), and focusing an existing terminal session did the same for drawers.
There was a problem hiding this comment.
One finding on the new PanelCollapse frame: the ref-detach path drops the node reference before it retires the flight, so the inline animation styles are never cleared. The terminal drawer swaps this ref between threads, so a mid-collapse thread switch strands the previous thread's drawer wrapper at height: 0px and drops its deferred close.
Posted via Macroscope — UI Consistency
The ref callback nulled nodeRef before retiring, so clearWrapperStyles saw no node and the detaching wrapper kept its inline height:0px; a thread switch inside a drawer collapse then repainted the terminal outside a zero-height box with nothing to recover it. The detach also pre-empted the identity-switch commit via supersede-checked retirement, stranding the old thread terminalOpen. Retire with an explicit commit while the ref still points at the element.
A changed supersedeKey was only honored at natural completion, so the collapse kept running over the newer intent (freshly selected surface) and snapped open 200ms later. Cancel the outbound flight as soon as the key diverges.
There was a problem hiding this comment.
One finding: the pull-requests route's deferred close can run after the route unmounts, and its URL write then navigates back to /pull-requests. Details inline.
Posted via Macroscope — UI Consistency
The shared drawer ref can swap across two different wrappers on a thread switch, and React may attach the new ref before detaching the old one; retiring through nodeRef could then clear the new drawer DOM while the old wrapper kept its inline height. Flights now capture their wrapper at arm time and all style application and cleanup target it, whatever order refs interleave in.
Toggling the panel closed and navigating away within the collapse ran the deferred onClose after the route unmounted, and updateSearch bound to the old route path with replace yanked the user back to /pull-requests. The store close still lands; only the URL write is guarded on the route being alive.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c6279b5. Configure here.
The mount-flag guard raced the ref detach: unmount cleanup ordering versus mutation-phase detach is not something to bet on. Check the router live location at commit time instead, which encodes the actual intent (rewrite this route URL only while this route is on screen) with no lifecycle phase dependence.
1 participant
What Changed
panelAnimations(default off), surfaced as Settings -> Appearance -> "Panel animations" and registered in settings search.PanelCollapseprimitive (apps/web/src/components/PanelCollapse.tsx): when enabled, closing a panel starts a 200ms CSS transition and defers the store close until the transition lands. Opening animates the entrance of the freshly mounted wrapper.Why
The previous iteration kept panels mounted through an exit animation while flipping the store closed instantly. That split one truth into two ("open" vs "still visible"), so every consumer needed
open || mountedworkarounds, frozen child snapshots, and phase bookkeeping - and each fix seeded the next bug (stuck phases, flicker on close, deadlocked toggles).This version keeps a single source of truth: the panel is semantically open for as long as it is visually open. Titlebar placement, badge suppression, terminal reconcile, and focus logic all read the store with zero animation awareness. Reopen mid-collapse cancels the pending commit and CSS retargets natively; a second toggle mid-collapse finishes the close; switching threads commits the old panel's close against its own identity so nothing leaks across threads.
With the setting off, the wrapper renders byte-for-byte like the unwrapped panel - the dormant path is "no class", not an idle engine.
Behavior notes
usePanelAnimations) and flight level.Testing
PanelCollapse.logic.test.tscovers the close contract: deferral, single commit, snap-when-disabled, no-op when closed, callback capture across re-renders.Happy to attach a short clip of the opted-in behavior if useful.
Worked by ox-alpha via opencode.
Note
Add opt-in
panelAnimationssetting with deferred-close panel transitionspanelAnimationssetting (defaultfalse) toClientSettingsSchemawith a toggle in the Appearance settings page.usePanelCollapseandPanelCollapseFrameinPanelCollapse.tsxto coordinate animated panel close/expand, keeping the DOM mounted and deferringonCloseuntil the transition completes.ChatView, and right panels inChatViewandPullRequestsRouteView.PreviewPanelShell.useClampedMaxWidthwas updated to bypassdata-panel-collapsewrappers to avoid miscalculated widths during the transition.Macroscope summarized 65e0d05.
Note
Medium Risk
Layout and close timing for the right panel and terminal drawer now defer store/URL updates until a CSS transition settles, which can race with thread switches, maximized layout, and PR route navigation if a flight is superseded incorrectly.
Overview
Adds an opt-in Panel animations client setting (default off) so the left sidebar, chat right panel, terminal drawer, and PR detail panel can collapse and expand with a 200ms CSS transition instead of snapping.
A new
usePanelCollapse/PanelCollapseFrameprimitive keeps the store open until the exit animation lands. Toggling mid-collapse finishes the close; a newer surface or thread identity supersedes a pending hide so it cannot wipe later intent. With the setting off (or OS reduced motion), wrappers behave like the unwrapped panels.Sidebar and titlebar padding transitions are gated on the same setting. Preview width clamping walks past collapse wrappers so row measurement stays correct.
Reviewed by Cursor Bugbot for commit 65e0d05. Bugbot is set up for automated code reviews on this repo. Configure here.