Repository navigation
sidebar scrollbar always visible — should only appear when content overflows - #3279
lawrencecchen wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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 (1)
📝 WalkthroughWalkthroughThe sidebar workspace list scroll view composition in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 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: 2/8 reviews remaining, refill in 39 minutes and 11 seconds.Comment |
Greptile SummaryAdds Confidence Score: 4/5Safe to merge; the functional change is a single targeted modifier addition with no behavioural regressions. Only P2 finding (cosmetic indentation inconsistency). The core fix — No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[VerticalTabsSidebar] --> B[workspaceScrollArea]
B --> C[GeometryReader]
C --> D[ScrollViewReader]
D --> E[ScrollView]
E -->|".scrollIndicators(.automatic) — NEW"| F{Content overflows?}
F -- Yes --> G[Show scroll indicator]
F -- No --> H[Hide scroll indicator]
E --> I[".background — SidebarScrollViewResolver"]
E --> J[".safeAreaInset / .overlay / .onAppear / .onChange"]
Reviews (1): Last reviewed commit: "Address https://github.com/manaflow-ai/c..." | Re-trigger Greptile |
| ScrollView { | ||
| workspaceScrollContent( | ||
| renderContext: renderContext, | ||
| minHeight: geometryProxy.size.height | ||
| ) | ||
| } | ||
| .scrollIndicators(.automatic) |
There was a problem hiding this comment.
Indentation de-sync inside
ScrollViewReader closure
After the refactor, ScrollView and its immediately chained modifiers (.scrollIndicators, .background) are unindented to the same column as ScrollViewReader { scrollProxy in, while the modifiers that follow (.safeAreaInset, .overlay, .onAppear, .onChange, …) are indented one level deeper. Swift doesn't use whitespace for parsing, so this compiles fine, but the visual mismatch makes it look like two separate view expressions live inside the closure — which would be a compile error — rather than one long modifier chain.
|
Closing this loader dogfood PR because the downloaded before/after videos still show the macOS FileVault confirmation sheet over cmux. I am patching FileVault suppression in the loader and rerunning from a fresh branch. |
Addresses #3241
Generated by cmux-loader.
Recorded videos:
Run artifacts: https://github.com/manaflow-ai/cmux-loader/actions/runs/25090033696
Codex final message:
Implemented a focused fix for #3241 in the workspace sidebar.
Change made
VerticalTabsSidebar.workspaceScrollAreaso the sidebarScrollViewnow uses:scrollIndicators(.automatic)This keeps sidebar scrollers from being permanently visible and lets indicator visibility follow overflow/OS scroll behavior.
Verification performed
./scripts/reload.sh --tag loader=======================================================
loader: file:///Users/runner/Library/Developer/Xcode/DerivedData/cmux-loader/Build/Products/Debug/cmux%20DEV%20loader.app
repro-before(status=ok)fixed-after(status=ok)Summary by cubic
Fixes #3241: The workspace sidebar scrollbar now appears only when content overflows. Updated the
VerticalTabsSidebarScrollView to use.scrollIndicators(.automatic)so visibility follows the OS.Written for commit 114756c. Summary will update on new commits. Review in cubic
Summary by CodeRabbit