Fix #5099: titlebar band swallows right-sidebar button clicks#5102
Conversation
…5099) Right-sidebar top buttons (Files/Search/Feed/Vault, plus close/open-as-pane) rendered normally but dead-clicked — no hover, no press, no panel switch — while the keyboard toggle kept working. Root cause (regression from #5017, ca73d65): the workspace titlebar was changed from an overlay scoped "only over terminal content, not the sidebar" into a full-width `workspaceTitlebarBand` layered at `zIndex(100)` over the content/sidebar layout. Its `customTitlebar` is `.frame(maxWidth: .infinity).contentShape(Rectangle())` — a full-width, hit-capturing strip. The right-sidebar mode bar lives inside the titlebar-height strip but below that band, so the band won the SwiftUI hit-test across the whole top row and absorbed every click/hover on those buttons. The left sidebar's titlebar controls live in the AppKit titlebar accessory (above the band), so only the right sidebar regressed; the keyboard path never goes through hit-testing, so it was unaffected. Confirmed empirically on a tagged dev build: an `NSView.hitTest` probe on the content host resolved right-sidebar button clicks to the hosting view (no NSView subview claimed them), and a probe in the mode-button action never fired — the SwiftUI band in front was consuming the hits. Fix: confine the band's interactive surface to the area left of the right sidebar via `.padding(.trailing, rightSidebarWidth)`, restoring the original "not over the sidebar" intent so the right-sidebar mode bar receives its own clicks again. The title text is left-aligned and unaffected. 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 ChangesTitlebar Hit-Test Adjustment
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning, 1 inconclusive)
✅ Passed checks (15 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Greptile SummaryThis PR fixes the right-sidebar button dead-click regression introduced in #5017 by confining the
Confidence Score: 4/5Safe to merge; the targeted trailing-padding inset correctly restores right-sidebar hit-testing without touching any other interaction surface. The fix is minimal and correctly scoped — it pads only the customTitlebar overlay, leaving the outer Color.clear band (which has no hit surface) and all other overlays untouched. The two remaining issues are a redundant ternary that can be simplified and a speculative animation-desync edge case when the right sidebar is toggled inside an animated context. Sources/ContentView.swift — specifically the animation suppression gap between the sidebar panel's .transaction { $0.animation = nil } and the new trailing padding modifier. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant WorkspaceTitlebarBand as workspaceTitlebarBand (zIndex 100)
participant CustomTitlebar as customTitlebar (.contentShape(Rectangle()))
participant RightSidebarModeBar as RightSidebarModeBar (Files/Search/Feed/Vault)
Note over WorkspaceTitlebarBand,RightSidebarModeBar: Before fix (#5017 regression)
User->>WorkspaceTitlebarBand: Mouse click over right sidebar area
WorkspaceTitlebarBand->>CustomTitlebar: Hit-test, full-width .contentShape wins
CustomTitlebar-->>RightSidebarModeBar: Event swallowed, buttons dead
Note over WorkspaceTitlebarBand,RightSidebarModeBar: After fix (this PR)
User->>WorkspaceTitlebarBand: Mouse click over right sidebar area
WorkspaceTitlebarBand->>CustomTitlebar: .padding(.trailing, rightSidebarWidth) confines interactive surface
CustomTitlebar-->>RightSidebarModeBar: Event passes through to mode bar buttons
Reviews (1): Last reviewed commit: "fix: stop titlebar band from swallowing ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // not the sidebar" intent. The left sidebar's titlebar controls live in | ||
| // the AppKit titlebar accessory (above this band), so only the trailing | ||
| // (right-sidebar) edge needs to be ceded here. | ||
| .padding(.trailing, rightSidebarVisible ? rightSidebarWidth : 0) |
There was a problem hiding this comment.
rightSidebarWidth is already computed as rightSidebarVisible ? fileExplorerWidth : 0, so rightSidebarVisible ? rightSidebarWidth : 0 always equals rightSidebarWidth. Using rightSidebarWidth directly also keeps this line consistent with how the same property is used elsewhere (e.g. .frame(width: rightSidebarWidth) on the panel).
| .padding(.trailing, rightSidebarVisible ? rightSidebarWidth : 0) | |
| .padding(.trailing, rightSidebarWidth) |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Good catch — simplified to .padding(.trailing, rightSidebarWidth) in e386c2f, since rightSidebarWidth already evaluates to 0 when the sidebar is hidden.
— Claude Code
| // not the sidebar" intent. The left sidebar's titlebar controls live in | ||
| // the AppKit titlebar accessory (above this band), so only the trailing | ||
| // (right-sidebar) edge needs to be ceded here. | ||
| .padding(.trailing, rightSidebarVisible ? rightSidebarWidth : 0) |
There was a problem hiding this comment.
rightSidebarPanel carries .transaction { $0.animation = nil } (line 2240), so it snaps immediately when fileExplorerState.isVisible changes. The new .padding(.trailing, …) on the titlebar band has no matching animation suppression. If any ancestor view wraps the visibility toggle in a withAnimation, the sidebar panel disappears/appears instantaneously while the titlebar band slowly closes/opens its trailing gap — briefly leaving an uncovered strip (sidebar gone, band still padded) or a covered strip (sidebar present, band not yet padded). Adding .transaction { $0.animation = nil } on the same modifier or on the workspaceTitlebarBand call site would keep both surfaces in sync.
There was a problem hiding this comment.
Fixed in e386c2f: added .animation(nil, value: rightSidebarWidth) on the band's trailing inset so it snaps in lockstep with the sidebar panel (which uses .transaction { $0.animation = nil }). The hit-region edge can no longer animate out of step and momentarily re-cover the mode bar during toggle.
— Claude Code
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>
…ollowup Follow-up to #5102: simplify titlebar inset + fix sidebar-toggle animation desync
Address review feedback on manaflow-ai#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>
1 participant
Fixes #5099.
Symptom
Right-sidebar top buttons (Files / Search / Feed / Vault, plus close + open-as-pane) render normally but dead-click — no hover, no press, no panel switch. The keyboard toggle still works, which isolates it to buttons not receiving mouse events, not the
RightSidebarModeaction path.Root cause (regression from #5017,
ca73d65cf)Before #5017 the workspace titlebar was an overlay placed "only over terminal content, not the sidebar." #5017 ("Polish sidebar extensions and titlebar chrome") changed it into a full-width
workspaceTitlebarBandlayered atzIndex(100)over the content/sidebar layout. ItscustomTitlebaris:The right-sidebar mode bar lives inside the titlebar-height strip but below that band, so the band wins the SwiftUI hit-test across the entire top row and absorbs every click/hover on those buttons.
The left sidebar's titlebar controls live in the AppKit titlebar accessory (above the band), so only the right sidebar regressed. The keyboard path never goes through hit-testing, so it kept working — exactly the reported behavior.
How it was confirmed (empirical, on a tagged dev build)
NSView.hitTestprobe on the content host resolved right-sidebar button clicks (winPt x≈799–993, top strip) to the hosting view itself — i.e. no NSView subview (drag handle, portal, etc.) claimed them.rightSidebar.modeButton.tap) and the button'sonHoverboth fired zero times on clicks/hover — the full-width SwiftUI band in front was consuming the events before SwiftUI routed them to the buttons.Fix
Confine the band's interactive surface to the area left of the right sidebar, restoring the pre-#5017 "not over the sidebar" intent:
The title text is left-aligned, so the trailing inset hides nothing; window-drag/double-click over the terminal-area titlebar is preserved; and the right-sidebar mode bar now receives its own clicks. Only the trailing (right-sidebar) edge needs ceding — the left sidebar's controls are in the titlebar accessory above the band.
Tests
This is a SwiftUI hit-test / z-order layering bug in the full-size-content titlebar band; it depends on the real
NSWindowtitlebar compositing and is not cleanly reproducible in a headless unit test, so I'm not adding a synthetic one rather than faking a passing test. Verified manually on a tagged dev build (the mode-button action fires on click after the change).🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by CodeRabbit