fix(desktop): make v2 new-workspace project dropdown scrollable#3628
Conversation
|
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)
📝 WalkthroughWalkthroughAdded a wheel-event handler to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 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 two related UX issues in the
Confidence Score: 5/5Safe to merge — minimal, focused fix with no logic changes and a direct precedent in the codebase. The two-line change mirrors an identical existing pattern (CompareBaseBranchPicker), uses a well-supported Radix UI CSS variable, and introduces no new logic or state. No regressions are plausible. No files require special attention. Note that CompareBaseBranchPicker uses a fixed max-h-[400px] without the min() viewport constraint — a follow-up to align it with this newer approach could be worthwhile but is not a blocker.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/components/ProjectPickerPill/ProjectPickerPill.tsx | Adds onWheel stopPropagation to PopoverContent and a viewport-aware max-height to CommandList — a clean, minimal fix that mirrors the established CompareBaseBranchPicker pattern. |
Sequence Diagram
sequenceDiagram
participant User
participant CommandList
participant PopoverContent
participant Modal
Note over User,Modal: Before fix
User->>CommandList: wheel event
CommandList-->>PopoverContent: bubble
PopoverContent-->>Modal: bubble (modal scrolls)
Note over User,Modal: After fix
User->>CommandList: wheel event
CommandList->>CommandList: scroll internally (max-h constrained)
CommandList-->>PopoverContent: bubble
PopoverContent->>PopoverContent: stopPropagation()
Note over Modal: Modal does NOT scroll
Reviews (1): Last reviewed commit: "fix(desktop): make v2 new-workspace proj..." | Re-trigger Greptile
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Summary
CommandListtomin(320px, --radix-popover-content-available-height)so the list always fits inside the popover's available room and scrolls internally.wheelpropagation onPopoverContentso scrolling inside the dropdown doesn't scroll the modal behind. Mirrors the existing pattern inCompareBaseBranchPicker.Test plan
Summary by cubic
Fixes the v2 new-workspace project dropdown so it scrolls inside the popover and stays within the viewport.
Constrains the
CommandListtomin(320px, var(--radix-popover-content-available-height))and stopswheelpropagation onPopoverContentto prevent the modal behind from scrolling.Written for commit efaed13. Summary will update on new commits.
Summary by CodeRabbit