Auto-hide terminal scroll bar fixes 2674#2678
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This review could not be run because your cubic account has exceeded the monthly review limit. If you need help restoring access, please contact contact@cubic.dev. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe changes implement overlay scrollbar functionality with dynamic visibility control. AppDelegate reorders geometry reconciliation timing, GhosttyTerminalView adds scrollbar configuration reading and mouse-tracking behavior, and tests update assertions to reflect the new scroller style handling. Changes
Sequence DiagramsequenceDiagram
participant GhosttyApp as GhosttyApp
participant Config as Ghostty Config
participant ScrollView as GhosttySurfaceScrollView
participant Scroller as NSScroller
participant Mouse as Mouse Events
GhosttyApp->>Config: Read 'scrollbar' setting
Config-->>GhosttyApp: Return config value
GhosttyApp->>ScrollView: scrollbarVisibility()
ScrollView->>ScrollView: synchronizeScrollbarAppearance()
ScrollView->>Scroller: Apply visibility setting<br/>(hasVerticalScroller)
ScrollView->>ScrollView: updateTrackingAreas()
ScrollView->>Scroller: Install tracking area<br/>over scroller bounds
Mouse->>ScrollView: mouseMoved(with:)
ScrollView->>Scroller: flashScrollers() if legacy style
Scroller-->>Mouse: Display scrollbar
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 SummaryThis PR fixes Confidence Score: 5/5Safe to merge; all changes are well-scoped scrollbar layout fixes with no data-path side effects. The implementation correctly forces overlay scrollerStyle before reading contentSize in both the system-pref-change and config-reload paths. The new test verifies the key invariant. No P0 or P1 issues found — remaining observations are all P2 style notes. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
autonumber
box System/AppKit
participant System as macOS System
participant NSSV as NSScrollView (internal)
end
box GhosttySurfaceScrollView
participant GSSV as GhosttySurfaceScrollView
participant SA as synchronizeScrollbarAppearance()
participant CS as synchronizeCoreSurface()
end
box Config Reload
participant AD as AppDelegate
end
rect rgb(230,240,255)
Note over System,CS: System scroller style change
System->>NSSV: preferredScrollerStyleDidChangeNotification
NSSV-->>NSSV: may set scrollerStyle=.legacy
NSSV->>GSSV: notification (queue:nil, main thread)
GSSV->>GSSV: handlePreferredScrollerStyleChange()
GSSV->>SA: synchronizeScrollbarAppearance()
SA-->>NSSV: scrollerStyle=.overlay + updateTrackingAreas()
GSSV->>NSSV: scrollView.tile()
GSSV->>CS: synchronizeCoreSurface()
CS-->>CS: reads full-width contentSize
CS->>GSSV: pushTargetSurfaceSize(fullWidth, height)
end
rect rgb(230,255,235)
Note over AD,CS: Config reload path
AD->>GSSV: refreshHostBackgroundAfterGhosttyConfigReload()
GSSV->>SA: synchronizeScrollbarAppearance()
SA-->>NSSV: reads new scrollbarVisibility, forces overlay
AD->>GSSV: reconcileGeometryNow()
GSSV->>CS: synchronizeCoreSurface()
CS->>GSSV: pushTargetSurfaceSize(correctWidth, height)
end
Reviews (1): Last reviewed commit: "Fix terminal scrollbar occlusion" | Re-trigger Greptile |
1 participant
Closes #2674.
What changed
scrollbarconfig in the cmux host wrapper soscrollbar = neveris respected instead of always showing a vertical scrollerWhy
cmux embeds Ghostty but its forked
GhosttySurfaceScrollViewhad drifted from Ghostty's macOS scrollbar behavior. That let a persistent right-side scrollbar gutter overlap the terminal's last column. This brings cmux back to Ghostty-style overlay behavior so the scrollbar only surfaces as an overlay instead of permanently occluding content.Notes
Note
Medium Risk
Touches terminal scroll view sizing and event handling, which can regress layout (grid width) or scrolling behavior across macOS scroller-style modes. Changes are localized to UI but affect a core interaction path.
Overview
Restores Ghostty-like macOS scrollbar behavior in the embedded terminal: reads Ghostty’s
scrollbarconfig (honoringscrollbar = never), forces overlay scrollers (no reserved gutter), and re-syncs scroller appearance when system preferred scroller style changes.Adds hover flashing for overlay scrollers when the system preference is legacy via a scroller tracking area, and refreshes scrollbar appearance before geometry reconciliation on config reload. Updates the existing regression test to assert overlay-width restoration after preferred scroller style changes.
Reviewed by Cursor Bugbot for commit c4a08fb. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes terminal scrollbar occlusion by restoring macOS overlay scroller behavior in the embedded Ghostty view and honoring the
scrollbarconfig (e.g., hides whenscrollbar = never). This removes the permanent right gutter and keeps the last column visible.scrollbarfrom Ghostty config and hide the vertical scroller when set tonever.Written for commit c4a08fb. Summary will update on new commits.
Summary by CodeRabbit