Follow-up to #5102: simplify titlebar inset + fix sidebar-toggle animation desync#5104
Conversation
Address review feedback on #5102: - `rightSidebarVisible ? rightSidebarWidth : 0` is redundant — `rightSidebarWidth` already evaluates to 0 when the sidebar is hidden. Use it directly. - The right sidebar panel snaps without animation; match that on the titlebar band's trailing inset (`.animation(nil, value: rightSidebarWidth)`) so the hit-region edge can't animate out of step with the panel during toggle and briefly re-cover the mode bar. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change ensures the workspace titlebar’s right-edge interactive region now always uses the actual right sidebar width for its inset, disabling the inset animation to keep the interaction zone perfectly synchronized with sidebar show/hide transitions. ChangesRight Sidebar Titlebar Inset Update
Sequence Diagram(s)sequenceDiagram
participant User
participant TitlebarBand
participant Sidebar
User->>TitlebarBand: Drag/double-click near right edge
TitlebarBand->>Sidebar: Reads rightSidebarWidth
TitlebarBand-->>User: Sets interactive region right inset to Sidebar's width (sync/no animation)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 16 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (16 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Greptile SummaryTightens the #5099/5102 titlebar-band hit-region fix: drops the redundant
Confidence Score: 5/5Safe to merge — both changes are strictly additive cleanup with no functional risk. The ternary removal is a provably equivalent simplification: No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant RightSidebarPanel
participant TitlebarBand
User->>RightSidebarPanel: toggle sidebar visibility
Note over RightSidebarPanel: .transaction { $0.animation = nil }
Note over TitlebarBand: .animation(nil, value: rightSidebarWidth)
RightSidebarPanel-->>TitlebarBand: rightSidebarWidth changes
TitlebarBand-->>User: hit-region edge and panel edge move together
Reviews (1): Last reviewed commit: "fix: simplify titlebar inset and keep it..." | Re-trigger Greptile |
Follow-up to #5102 (the #5099 titlebar-band hit-region fix). #5102 merged at its first commit, so these two review-feedback improvements from Greptile didn't make it into main:
Redundant ternary —
rightSidebarVisible ? rightSidebarWidth : 0is redundant becauserightSidebarWidthalready evaluates to0when the sidebar is hidden. Simplified to.padding(.trailing, rightSidebarWidth).Animation desync (P2) — the right sidebar panel snaps without animation (
.transaction { $0.animation = nil }), but the titlebar band's trailing inset could animate out of step during toggle, momentarily re-covering the right-sidebar mode bar (a brief dead-click window right after the sidebar appears). Added.animation(nil, value: rightSidebarWidth)so the band's hit-region edge snaps in lockstep with the panel.No functional change to the merged fix; this only tightens the inset and removes the transient desync. Addresses the two open Greptile threads on #5102.
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Note
Low Risk
Small SwiftUI layout/hit-testing tweak in the titlebar overlay with no auth, data, or API impact.
Overview
Tightens the fullscreen titlebar drag band trailing inset in
ContentView: it now usesrightSidebarWidthdirectly (no redundantrightSidebarVisiblecheck) and disables animation on that inset so it snaps in sync with the right sidebar panel.That removes a brief window where the band could animate out of step with the sidebar and re-cover the right-sidebar mode bar after toggle—dead clicks on Files/Search/Feed/Vault controls—without changing the underlying #5099/#5102 hit-region fix.
Reviewed by Cursor Bugbot for commit e3a3411. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Simplified the titlebar band's trailing inset and synced it with the right sidebar snap to eliminate a brief dead-click window during sidebar toggle.
Bug Fixes
rightSidebarWidth, matching the sidebar panel’s non-animated toggle.Refactors
rightSidebarWidthdirectly for trailing padding; remove redundant ternary.Written for commit e3a3411. Summary will update on new commits.
Summary by CodeRabbit