Repository navigation
sidebar scrollbar always visible — should only appear when content overflows - #3248
lawrencecchen wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThe sidebar workspace list's Changes
Possibly related PRs
Poem
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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. Review rate limit: 0/8 reviews remaining, refill in 56 minutes and 44 seconds.Comment |
Greptile SummaryThis PR fixes the sidebar scrollbar always reserving visible track space by setting Confidence Score: 4/5Safe to merge; only one P2 style concern about overriding the system scroll bar preference. The change is a small, targeted AppKit property assignment with no logic errors. The only concern is a P2: forcing .overlay style may not respect the macOS "Always show scroll bars" accessibility setting. No P0 or P1 issues found. Sources/ContentView.swift — specifically the scrollerStyle line if system preference compatibility matters. Important Files Changed
Sequence DiagramsequenceDiagram
participant SwiftUI as SwiftUI (ScrollView)
participant Resolver as SidebarScrollViewResolverView
participant NSScrollView as NSScrollView (AppKit)
SwiftUI->>Resolver: viewDidMoveToWindow / viewDidMoveToSuperview
Resolver->>Resolver: resolveScrollView() (async)
Resolver->>NSScrollView: enclosingScrollView (lookup)
NSScrollView-->>Resolver: NSScrollView? instance
Resolver->>SwiftUI: onResolve(scrollView)
Note over SwiftUI,NSScrollView: dragAutoScrollController.attach(scrollView)
Note over NSScrollView: scrollView.autohidesScrollers = true (NEW)
Note over NSScrollView: scrollView.scrollerStyle = .overlay (NEW)
Reviews (1): Last reviewed commit: "Address https://github.com/manaflow-ai/c..." | Re-trigger Greptile |
|
Closing this first dogfood attempt because the loader runner was missing Zig, so it could not reload cmux or record the required before/after videos. Rerunning from a fresh branch with https://github.com/manaflow-ai/cmux-loader/pull/2. |
| dragAutoScrollController.attach(scrollView: scrollView) | ||
| if let scrollView { | ||
| scrollView.autohidesScrollers = true | ||
| scrollView.scrollerStyle = .overlay |
There was a problem hiding this comment.
scrollerStyle overrides system "Always show scroll bars" preference
Setting scrollerStyle = .overlay unconditionally will override the user's macOS System Preferences setting (System Settings → Appearance → Show scroll bars → Always). NSScrollView normally observes NSScrollerStyleDidChangeNotification and resets its scrollerStyle to match the system pref, but an explicit programmatic assignment wins until the next system-preference change — at which point AppKit will override the custom value since the view never registers to re-apply it.
For users who have "Always" selected (common accessibility need), the sidebar scrollbar will appear as a thin overlay rather than a permanently-visible track. Consider guarding the scrollerStyle assignment behind a check, or omitting it entirely: autohidesScrollers = true alone already removes the always-visible track when the system preference is not "Always".
Addresses #3241
Generated by cmux-loader.
Codex final message:
Implemented a narrow fix for the sidebar scrollbar behavior.
What I changed
Change is in
workspaceScrollArea’sSidebarScrollViewResolvercallback:scrollView.autohidesScrollers = truescrollView.scrollerStyle = .overlayThis makes the sidebar scrollbar follow overflow/interaction behavior instead of reserving a permanent track.
Verification performed
./scripts/reload.sh --tag loaderCMUX_SKIP_ZIG_BUILD=1 ./scripts/reload.sh --tag loaderError: zig is not installed. Install via: brew install zigResult: I could not build a tagged app in this runner, so I could not run
computer-useor capture before/after videos here.Environment/build constraints
.gitis not writable in this session:git commitfailed withUnable to create .../.git/index.locktouch .git/...also fails withOperation not permittedRecorded videos
repro-before: not recordedfixed-after: not recordedPR
.git).If you want, I can finish this by creating the commit/PR locally once run in an environment with
ziginstalled and writable.git, then I can also capture and include both required videos.Summary by cubic
Fixes #3241 by making the sidebar scrollbar appear only when content overflows or during interaction, instead of always being visible. Sets
scrollView.autohidesScrollers = trueandscrollView.scrollerStyle = .overlayin the sidebar scroll resolver to remove the permanent track.Written for commit 11a95bb. Summary will update on new commits. Review in cubic