Repository navigation
sidebar scrollbar always visible — should only appear when content overflows - #3280
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 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)
📝 WalkthroughWalkthroughContentView.swift updated with sidebar scroll view configuration for overlay indicators and auto-hiding scrollbars before drag auto-scroll attachment; SidebarEmptyArea layout modified to constrain by maximum width instead of infinite height. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 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. Review rate limit: 3/8 reviews remaining, refill in 30 minutes and 3 seconds.Comment |
|
Closing this loader dogfood PR because the repro video is clean but the fixed-after videos still include the macOS FileVault confirmation sheet. I am patching the runner setup suppression again and will rerun from a fresh branch. |
Greptile SummaryThis PR fixes the sidebar scrollbar always-visible issue by configuring the resolved
Confidence Score: 3/5The scrollbar visual fix is correct, but the stated content-sizing fix is a no-op; the artificial height inflation remains. The primary symptom (scrollbar always visible) is addressed by the overlay/autohide changes. However, the SidebarEmptyArea body change is ineffective because maxHeight: .infinity is still applied at the call site in workspaceScrollContent (line 9882), leaving the underlying content-sizing issue described in the PR unresolved. This is a P1 logic defect on the changed path. Sources/ContentView.swift — specifically line 9882 where .frame(maxWidth: .infinity, maxHeight: .infinity) is still applied to SidebarEmptyArea. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[workspaceScrollArea] --> B[ScrollView]
B --> C[SidebarScrollViewResolver]
C --> D["scrollerStyle = .overlay\nautohidesScrollers = true\n✅ NEW"]
C --> E[dragAutoScrollController.attach]
B --> F[workspaceScrollContent]
F --> G[VStack]
G --> H[workspaceRows]
G --> I["SidebarEmptyArea\n.frame(maxWidth: .infinity, maxHeight: .infinity)\n⚠️ Call-site still has maxHeight: .infinity"]
I --> J["SidebarEmptyArea.body\n.frame(maxWidth: .infinity)\nchanged in PR — no-op"]
Reviews (1): Last reviewed commit: "Address https://github.com/manaflow-ai/c..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| Color.clear | ||
| .contentShape(Rectangle()) | ||
| .frame(maxWidth: .infinity, maxHeight: .infinity) | ||
| .frame(maxWidth: .infinity) |
There was a problem hiding this comment.
SidebarEmptyArea change is a no-op — maxHeight: .infinity still applied at call site
Removing maxHeight: .infinity from inside SidebarEmptyArea.body has no effect because the call site in workspaceScrollContent (line 9882) immediately re-applies .frame(maxWidth: .infinity, maxHeight: .infinity) to the same view:
SidebarEmptyArea(...)
.frame(maxWidth: .infinity, maxHeight: .infinity) // line 9882 — still presentThe outer frame modifier wraps the view, so the SidebarEmptyArea still requests unlimited height inside the ScrollView. If the goal is to remove artificial height inflation that causes unneeded scroll overflow, the maxHeight: .infinity at the call site also needs to be removed. The actual scrollbar visual fix (.overlay + autohidesScrollers) addresses the symptom, but the underlying content sizing issue described in the PR remains.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e511008d83
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Color.clear | ||
| .contentShape(Rectangle()) | ||
| .frame(maxWidth: .infinity, maxHeight: .infinity) | ||
| .frame(maxWidth: .infinity) | ||
| .onTapGesture(count: 2) { |
There was a problem hiding this comment.
Removing maxHeight: .infinity from SidebarEmptyArea shrinks the view that owns .onTapGesture/.onDrop, so the interactive empty-sidebar region can collapse to minimal height when there are few workspaces. In that state, double-click-to-create-workspace and drop-to-end only work on a thin strip instead of the whole empty area, because the outer frame applied at the call site does not restore this view’s internal hit-test region.
Useful? React with 👍 / 👎.
This branch was successfully deployed
1 participant
Addresses #3241
Generated by cmux-loader.
Recorded videos:
Run artifacts: https://github.com/manaflow-ai/cmux-loader/actions/runs/25090436911
Codex final message:
Implemented for #3241 in:
What changed:
ScrollViewresolver, the resolvedNSScrollViewis now configured for overflow-aware behavior:scrollerStyle = .overlayautohidesScrollers = truemaxHeight: .infinityfromSidebarEmptyArea, preventing it from forcing artificial overflow content sizing in the sidebar.Verification:
./scripts/reload.sh --tag loader(completed successfully)repro-before.movfixed-after-4.movworkspace 1 of 1) and rerunning the app after edits.No PR was opened.
Summary by cubic
Fixes the sidebar so the scrollbar only shows when content overflows and hides otherwise. Addresses #3241 with overlay scrollers and removal of artificial overflow.
NSScrollViewwithscrollerStyle = .overlayandautohidesScrollers = true.maxHeight: .infinityfromSidebarEmptyAreato stop forcing overflow.Written for commit e511008. Summary will update on new commits. Review in cubic
Summary by CodeRabbit