Repository navigation
UI Consistency: 2 issues found
Two findings on the panel-animation opt-in:
apps/web/src/components/preview/PreviewPanelShell.tsx(lines 94-96): the newoverflow-hiddenon the animated panel host clipsRightPanelResizeHandle, which intentionally overhangs the left edge by 4px (absolute inset-y-0 -left-1 w-2). With panel animations enabled the inline resize grab target halves (8px to 4px) and the 1px hover/active indicator centered on the border is half-clipped. Suggested fix: clip only while the panel is closed/animating shut (the ancestorSidebarInsetis alreadyoverflow-hidden), or move the clip to a wrapper around the children so the handle stays outside it.apps/web/src/index.css(lines 137-145): theWorkspacePageHeaderpadding-lefttransition was moved out of the owning module into an unlayered global rule plus a newdata-workspace-page-headerhook. This relocates one-owner presentation away from its owner and, since Tailwind utilities are layered, the shorthand outranks anytransition-*a call site passes viaclassName. The owner already uses the ancestor-attribute variant idiom (COLLAPSED_SIDEBAR_TITLEBAR_INSET_CLASS), so[[data-panel-animations=true]_&]:motion-safe:transition-[padding-left](plus duration/easing) at the owner lets both CSS blocks go.
Both comments were posted inline.
Details
Note
Your check run agent prompt is: .macroscope/check-run-agents/ui-consistency.md
More information about how Check Run Agents work can be found in our Help Center.
Checked and found acceptable:
usePanelPresenceretention logic: closing panels stay mounted for 200ms only whenanimatedis true; when animations are off, the returnedpresentcollapses immediately, so mount/unmount timing matches the previous behavior. Sheet/inline breakpoint flips also cannot render both panels at once becauseanimatedis scoped per branch.renderedRightPanelSurface/renderedPullRequestSurfacesubstitutions preserve the previousactiveRightPanelSurfaceandactivePullRequestSurfacesemantics while the panel is open, and the pull-requests panel still resolvespanelEnvironmentIdfrom the explicit surface scope, so environment routing stays explicit.Sidebaris instantiated once (AppSidebarLayout), so the newanimatedprop has no ungated sibling instances;data-panel-animationsis forwarded bySidebarProvidervia{...props}and everyWorkspacePageHeader/inline panel consumer sits inside that wrapper.SheetPopup'sanimated={false}overrides disable both backdrop and popup transitions withtransition-none!, so Base UI unmounts immediately; the retained sheet presence only applies when animations are on.PanelAnimationsPreviewis a purpose-built preview/replay target rather than a reconstruction ofButton, matching existing settings swatch/preview idioms (ThemeColorPicker), and keeps afocus-visiblering andaria-label.RightPanelTabsconditional prop spreads match the file's existing exact-optional forwarding style.- The
@starting-stylerule needs!importantto beat the inline width and is browser-behavior-specific, so keeping it inindex.cssis consistent with the stated ownership rules.
🔍 Agent activity — 23 tool calls
💭 I'm looking at the changed files to check consistency—particularly the new panel animations hook and how the global CSS now handles the panel header's transition logic that was previously owned by the WorkspacePageHeader component itself. That conditional transition could actually be expressed with Tailwind's ancestor-attribute variants and the motion-reduce variant, so moving it to global CSS looks like a violation of the ownership convention worth flagging. I'm also examining the
@starting-styleblock for the panel open animation, which forces width to zero so the element animates in from nothing when it first mounts in the open state, even though inline styles set the target width separately. I'm also checking whetherPreviewPanelShellactually sits inside the ancestor with thedata-panel-animationsattribute in the pull-requests route, since the global selector depends on that hierarchy. And I'm reviewingusePanelPresence's logic around the deferredsetTimeoutwhen presence goes false and the visibility formula that combines animation state, presence, and retained values, plus how the scoped retained value keys offactiveThreadKeyin ChatView. Then I move to a concern about mutating a ref inside an effect and reading it during render, though it likely still works since the accompanying state change forces a re-render. Next I checkRightPanelTabs.tsx, where the animated prop is spread conditionally ontoPreviewPanelShellrather than passed directly, probably to satisfy strict optional property typing rather than a genuine issue. Finally I look atPanelAnimationsPreview.tsx, where a raw button element with a large class string appears to reimplement a Button component, though the rule allows raw elements that intentionally serve a semantic purpose. I decide this button is more like a custom preview/replay target similar to a swatch or image preview, so it's probably fine even though it duplicates focus-ring styling, and I want to check whether other settings previews follow a similar pattern for consistency. I also trace through the effect that togglespanelsOpenon animation frame whenenabledchanges, confirming that clicking still replays the motion even when animations are disabled, which seems like intended behavior given the reduced-motion handling in the transition classes. I'm also reviewing several other transition tweaks across the sheet, sidebar, and sidebar rail components — changing easing curves from linear to ease-out and adjusting duration values to match the new global CSS convention, which all seem like minor, consistent refinements rather than issues. Now I want to trace how theanimatedprop actually flows through the sidebar and layout components — checking whetheruseMediaQuerysupports SSR properly, whether the settings selector hook works as expected, whether the mobile sheet inherits the animated flag, and whetherAppSidebarLayoutcorrectly passesanimated={panelAnimationsActive}down into the Sidebar component itself. That's fine since it's off by default anyway, but if WorkspacePageHeader appears outside AppSidebarLayout with animations enabled, the transition just won't apply, which seems acceptable. I want to check the actual files now, and I'm also noticing that ChatView's inline right panel presence logic dropped the!shouldUseRightPanelSheetcheck in the render condition, though sinceinlineRightPanelPresentalready requires that flag to be false viaopen, it should still be consistent. Though there's a subtle timing edge case during a closing animation—if the window resizes narrower andshouldUseRightPanelSheetflips true mid-animation,inlineRightPanelPresentcould briefly stay true while the sheet also becomes present, causing both to render simultaneously. It's minor sinceusePanelPresence's animated flag also flips off on resize, forcing present to false via a zero-timeout, but worth keeping in mind. I should pull up the actual source files now to verify this.
1. browse_code — apps/web/src/panelAnimations.ts
2. browse_code — apps/web/src/components/WorkspacePageHeader.tsx
💭 I notice the data attribute placement before spreading props could allow consumers to override it, which is a minor concern. For the global CSS rule on the header, I'm figuring out how to express the padding-left transition as a Tailwind variant keyed off an ancestor's data attribute rather than a scoped one-off rule. This is a real ownership violation — they moved presentation from the owning component into global CSS with a new data attribute hook, when it should stay as static Tailwind classes on the owner. It's expressible with arbitrary ancestor variants, so I'll flag it as a moderate finding. Next I want to check the sidebar file and AppSidebarLayout for other Sidebar instances that aren't receiving the
animatedprop.
3. browse_code — apps/web/src/components/ui/sidebar.tsx
4. browse_code — apps/web/src/components/AppSidebarLayout.tsx
💭 I want to check if there are other Sidebar usages in apps/web, and also look at SidebarInset for transition classes—since if the sidebar itself stops animating but coupled elements like the inset margin still use ease-linear transitions, that would create an inconsistency worth flagging.
5. git_tools — transition-[|transition-all|ease-linear
6. git_tools — <Sidebar$|<Sidebar |<SidebarInset
💭 I notice there's only one Sidebar usage, which is good. Looking at the group label transition on margin and opacity, that's pre-existing behavior tied to the sidebar collapse animation rather than something gated by the new setting—though it's worth considering whether the new "slide sidebar on open/close" setting should also account for it. Now I want to check the CSS rules in index.css around the new additions, and see whether COLLAPSED_SIDEBAR_TITLEBAR_INSET_CLASS involves padding-left transitions.
7. browse_code — apps/web/src/index.css
8. git_tools — COLLAPSED_SIDEBAR_TITLEBAR_INSET_CLASS
💭 I notice
workspaceTitlebar.tsalready uses this ancestor-attribute Tailwind variant pattern, confirming the padding transition could have stayed at the owner using the same idiom rather than being duplicated elsewhere. Checking RightPanelTabs too, since it also toggles padding-left with the collapsed sidebar class, but that check isn't essential here. More importantly, I'm realizing the global CSS rule targetingdata-workspace-page-headeris unlayered, meaning in Tailwind v4 it would override layered utility classes regardless of specificity — so anytransition-colorsor similar utility a consumer applies via className would get silently reset by this global shorthand transition rule. I want to verify this by searching for consumers of WorkspacePageHeader that pass in transition classes.
9. git_tools — <WorkspacePageHeader
💭 Since no consumer actually passes transition classes to WorkspacePageHeader, the shorthand-clobbering issue is more theoretical than practical. But the CSS-in-global styling still seems worth flagging as a finding, since per the ownership guidelines this presentation logic belongs in the owning module as static Tailwind classes rather than a global override — I'll note it against index.css lines 137-145 and the WorkspacePageHeader component. Turning to the second candidate finding, I'm tracing through PreviewPanelShell's
@starting-styleglobal rule with a forced zero width. Checking the usePanelPresence logic, when the panel opens the element renders with open state and target width immediately while@starting-stylesupplies the initial zero width, so the transition should animate correctly from closed to open — that part looks sound. For closing, the width resets to zero with pointer-events disabled while presence persists for the exit duration before unmounting, which also looks correct. Now I want to check whether thedata-panel-motionanddata-panel-stateattributes get properly placed relative to spread props in PreviewPanelShell, since attribute ordering there could cause them to be overridden.
10. browse_code — apps/web/src/components/preview/PreviewPanelShell.tsx
💭 I'm wondering if overflow-hidden only kicks in when animated, and whether that clips something meant to overflow — like the resize handle positioned with a negative left offset, which would get cut off unintentionally. Dropdown menus likely render in portals so they'd be unaffected. Let me check RightPanelResizeHandle directly.
11. browse_code — apps/web/src/components/preview/RightPanelResizeHandle.tsx
💭 I'm realizing the resize handle deliberately overlaps 4px outside the panel edge so users can grab it without precise aiming, but adding overflow-hidden to the panel when animated would clip that outer overlap, effectively halving the grab target and cutting off its hover indicator line which sits right at the panel's edge. The best fix seems to be avoiding overflow-hidden on the host entirely — either clipping an inner wrapper instead, or only enabling the clip while the panel is closed or closing rather than whenever animation is enabled. During opening, that still lets children briefly overflow while the width tween resolves, but it preserves the interactive resize handle otherwise, which feels like the right tradeoff to flag as a suggestion. The cleaner fix would be moving the overflow-hidden clip to an inner wrapper around the children specifically, keeping the resize handle outside that clipped region — but that touches two separate parts of the JSX, so it's not a single clean diff hunk. I'll describe this as a suggestion rather than trying to force it into one patch. For the second finding, the header's transition rule was moved into global CSS as an unlayered rule, which means it will override any Tailwind transition utility a consumer passes in, since Tailwind utilities sit in a layer that loses to unlayered CSS. Nothing breaks today since ChatView's className is simple, but this creates a fragile ownership boundary worth flagging. I'll report this as a comment alongside the main overflow-hidden regression as the primary failure driver. I also want to verify that
data-panel-animationscorrectly propagates through SidebarProvider to all the nested headers (settings, usage, etc.) — checking that they're all wrapped inside SidebarInset within SidebarProvider, which they are, and that the attribute is forwarded properly via spread props on the div, which it is. Next I want to look intousePanelPresencefor a potential bug around how the "present" state is tracked. I'm tracing through the animated-off and animated-on closing paths — with animations disabled, the visible state resolves to false immediately so there's no lingering render tick despite the internal setTimeout(0), and with animations enabled, the retained value stays correctly populated via the ref during close since the scope key hasn't changed. I'm now checking the opening path to see if there's a similar edge case with the visibility flag timing relative to the rAF callback. Looking further, I'm considering whether the effect's dependency on the surface value object causes unnecessary re-runs due to identity changes each render — likely harmless. More concerning is a change in the ChatView render condition that dropped a check against using the right-panel sheet mode, which could let both the inline panel and the sheet render simultaneously if the viewport crosses the sheet breakpoint during an animated close. Then I trace through the animated flag logic and confirm that once shouldUseRightPanelSheet becomes true, animated turns false, so visibility correctly resolves to false — no simultaneous rendering issue after all. Next I want to check whether the RightPanelTabs component correctly forwards its animated/open props to PreviewPanelShell, and whether open should also control the tab bar visibility.
12. browse_code — apps/web/src/components/RightPanelTabs.tsx
13. browse_code — apps/web/src/components/RightPanelTabs.tsx
💭 I'm now tracing how RightPanelSheet passes the animated prop down to SheetPopup, and checking whether the open state controlled by ChatView actually allows the exit animation to play correctly when closing.
14. browse_code — apps/web/src/components/ui/sheet.tsx
15. browse_code — apps/web/src/components/RightPanelSheet.tsx
💭 I'm noticing that previously the Sheet unmounted instantly on close since the parent condition controlled mounting directly, meaning there was no exit animation. Now that open is tied to rightPanelOpen directly, I need to check whether this actually enables a proper exit transition instead. With animated=false, transitions are disabled so it still unmounts immediately when animations are off, matching the old behavior — that seems consistent. I should also verify that usePanelAnimationsActive and useMediaQuery exist with the expected signatures in the codebase.
16. git_tools — export function useMediaQuery
17. git_tools — export function useClientSettings
💭 The selector recomputing each render is fine since it just returns a boolean. Now I want to check how other settings previews, like theme swatches, are implemented elsewhere in the settings panels to model the PanelAnimationsPreview button consistently.
18. browse_code — apps/web/src/components/settings/*.tsx
19. git_tools — focus-visible:ring-offset-background
💭 I want to check ThemeWireframe for a similar preview-as-button pattern to confirm this is consistent with existing idioms, though it's likely fine. I'm also verifying the PanelAnimationsPreview keeps animating even when the setting is disabled—on mount with it off, panels start collapsed before the animation frame toggles state. The aria-label says "Replay panel animation preview" but clicking actually toggles rather than replays, which is a minor accessibility inconsistency worth noting. I'm also confirming that the ChatView changes to
revealLine/revealRequestIdnow derived fromrenderedRightPanelSurfaceinstead ofactiveRightPanelSurfacepreserve the same semantics, and that file-panel-related props remain correct while the panel is retained during close. Now I'm looking at the pull-requests route's panel presence logic to make sure it correctly reflects when the panel should be open based on the selected surface. I noticed the value passed isselectedPullRequestSurface, but the old code usedactivePullRequestSurface— and ifrenderedPullRequestSurface(derived fromselectedPullRequestSurface) isn't equivalent to the originalactivePullRequestSurface, that could be a real behavior change, so I want to check the surrounding code around lines 350-420 to confirm.
20. git_tools — selectedPullRequestSurface|activePullRequestSurface|rightPanelRef|PULL_REQUESTS_PANEL_ID
💭 I'm confirming that the derived surface value stays consistent between open and closing states, retaining its last value during the animation. I also notice that the row highlight logic depends on the same active surface variable, so it clears immediately when closing starts even though the panel content persists during the animation. Checking further, the sheet layout isn't affected since only inline mode gets animated, and the maximized state correctly disables transitions entirely. There's a minor visual detail where the border might show a thin sliver at zero width during closing, but since the element unmounts shortly after, it leaves no lasting artifact. I confirm the
@starting-stylerule correctly targets the sidebar wrapper ancestor that ChatView's inline panel sits within, and verify that its!importantwidth declaration properly overrides the non-important inline style — which is exactly why!importantwas necessary there. I also note the reduced-motion handling wraps this behavior appropriately. Checking further,usePanelAnimationsActivealready disables under reduced motion, so thedata-panel-animationsattribute andmotion-reduce:transition-noneclass are redundant but harmless safety nets, not real issues. The header's padding-left transition being conditional on panel animations is consistent — with animations off, both sidebar and header padding jump instantly together, which is the intended behavior, not a regression. I want to make sure there's no other sidebar-synced transition elsewhere that stays unconditional, which would create an inconsistent mix when animations are off, so I'm searching the web source for other transitions keyed on the sidebar state attribute.
21. git_tools — data-sidebar-state=collapsed
💭 I notice the transition CSS applied via
[data-workspace-page-header]doesn't cover RightPanelTabs' tab bar, even though it shares the same collapsed sidebar inset class — so that element won't get the animated padding change, unlike the main header. The more concrete issue is thatoverflow-hiddenon PreviewPanelShell clips the RightPanelResizeHandle's intentional 4px overhang when animations are enabled, shrinking the resize hit target and cutting the hover indicator line roughly in half — a real regression against a documented exception. I'm weighing fix options: wrapping children in a separate overflow-clipped div would risk breaking flex layout since children currently rely on being direct flex children, and moving the handle isn't viable either. A safer approach might be only applying the clip while the panel is animating closed, so the open/resizable state keeps its full overhang. Actually overflow-clip-margin might solve this elegantly — it extends the clip region so the handle stays paintable and hittable, but browser support is a concern since Safari's support is uncertain, and while Electron/Chromium is fine, web Safari users could break. I'm weighing whether to just suggest clipping only during closing so the handle's slight overlap survives while opening, but that risks the tab bar's fixed-width children overflowing visibly in a zero-width box during the open animation, so I need to think through whether that bleed is actually visible or contained. Since the panel's box is anchored on the right edge, oversized content overflows rightward offscreen rather than into the visible chat area, so clipping it during the closed-animation state is safe. The chat column shrinks alongside it as flex-1 with min-w-0, so applying overflow-hidden and pointer-events-none only when animated and closed seems like a reasonable fix. Now moving to the second finding: the WorkspacePageHeader transition was pulled out of its owner's Tailwind classes into a global unlayered rule in index.css, tied to a new data-workspace-page-header hook, even though this same header already scopes similar ancestor-conditional styling right at the owner via COLLAPSED_SIDEBAR_TITLEBAR_INSET_CLASS. This breaks the pattern that one-owner presentation belongs at the owner, and worse, the unlayered transition shorthand will override any Tailwind transition utility a consumer tries to pass in via className, since Tailwind utilities are layered. I'm weighing how to present the fix since it spans both the header component and the CSS file -- a proper diff would need multiple hunks across files, which isn't supported here, so I'm considering whether to give an inline suggestion on the component line and just describe the CSS deletion separately in the comment text. I'll comment on the actual CSS rule at index.css lines 137-145, since that's within scope. I'm also reconsidering whether the ownership violation is significant enough to flag as a real failure versus a secondary note, and thinking about whether there's a third finding worth including around the sidebar's transition timing changes. I checkSidebarGroupLabel's margin/opacity transition but conclude it likely doesn't apply since this app uses offcanvas collapsible mode rather than icon mode, so I'll skip flagging that. I also verify thedata-panel-animationsattribute placement on the shared SidebarProvider wrapper looks fine as a call-site customization. Looking atPanelAnimationsPreview, the rAF-based effect ties panelsOpen to the enabled flag, which creates a slightly confusing visual metaphor where disabling animations shows panels as collapsed even though the actual sidebar isn't collapsed—this feels aesthetic rather than a real bug, so I'll skip it. The settings row description text and searchable id look fine too. I'm now tracing how ChatView'sPreviewPanelShellreceives itsopenprop from RightPanelTabs, confirming it correctly derives fromrightPanelOpenwhileinlineRightPanelPresentis preserved separately—that checks out. I want to verify there's no other consumer ofPreviewPanelShellin inline mode that might be missing this same wiring.
22. git_tools — <RightPanelTabs|<PreviewPanelShell
💭 I'm preparing two inline review comments: one flagging that
overflow-hiddenon the panel host in PreviewPanelShell.tsx clips content, and another on the added CSS lines in index.css around the panel styling. Working out the suggestion diff... Now for the second comment on index.css, the padding-left transition for WorkspacePageHeader is being moved into a global unlayered rule scoped by a data attribute, which would sit outside the owning module and could outrank Tailwind utility classes passed through className. I'm noting the ancestor already scopes similar padding using a variant class, so the same pattern could apply here and let the block plus its reduced-motion override be removed—though I need to check whether motion-reduce:transition-none alone can actually override the new variant since both are utilities of equal specificity. Since Tailwind v4 supports stacked variants, I realize combining motion-safe: with the ancestor data-attribute variant into a single stacked class would cleanly handle both the transition and the reduced-motion preference in one utility. I'll leave the @starting-style rule alone since it's unrelated browser behavior, and finalize this as two findings under a "UI Consistency" review before posting.
23. modify_pr — Reviewed the panel-animation opt-in for shared-primitive and CSS-ownership consistency. Two findings: a clipped resize handle in PreviewPanelShell when animations are enabled, and header transition ownership moving from the owning module into a global rule. The @starting-style block is appropriate to keep in index.css.
Agent Credits: 172 credits
Token usage: 32 uncached input · 960,474 cache-read input · 97,681 cache-write input · 21,605 output
Agent Credits may also include non-token charges from external tools such as web research.