Repository navigation
Add extension browser pane - #5053
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughAdds a SwiftUI/AppKit sidebar panel surface for the extension browser, featuring a card-styled container with responsive layout constraints, dynamic compact/full mode toggling, and integration into the panel rendering system with English and Japanese localization for the compact layout label. ChangesExtension Browser Panel UI Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (15 passed)
✨ Finishing Touches🧪 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 |
…r-tab # Conflicts: # Resources/Localizable.xcstrings # Sources/CMUXSidebarExtensionBrowserPanel.swift # Sources/Panels/PanelContentView.swift
Greptile SummaryThis PR moves the ExtensionKit browser from a floating panel into a sidebar pane tab, adding
Confidence Score: 4/5Safe to merge after addressing the compact-label flash; the remaining gap is the incomplete locale coverage for sidebar.extensions.browser.compact already called out in the prior review thread. The compact-mode visibility check uses raw Sources/CMUXSidebarExtensionBrowserPanel.swift (isCompact guard) and Resources/Localizable.xcstrings (compact string locale coverage) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["PanelContentView\n(SwiftUI)"] -->|"case .extensionBrowser"| B["CMUXSidebarExtensionBrowserPanelView\n(NSViewControllerRepresentable)"]
B -->|makeNSViewController| C["CMUXSidebarExtensionBrowserContainerViewController"]
B -->|updateNSViewController| C
B -->|dismantleNSViewController| D["detachBrowserForTransientReparent()"]
C --> E["RootView (NSView)\nonLayout / onMouseDown / onMoveToWindow"]
C --> F["FocusCardView\nonMouseDown → onRequestPanelFocus"]
F --> G["contentView (NSView)\n4-edge pinned to cardView"]
F --> H["compactLabel (NSTextField)\ncentered in cardView"]
G --> I["browserViewController.view\n4-edge pinned to contentView"]
E -->|"layout()"| J{"updateLayoutForCurrentBounds()"}
J -->|"layoutWidth/Height > minimumUsable"| K["contentView visible\ncompactLabel hidden"]
J -->|"below minimum OR zero bounds"| L["contentView hidden\ncompactLabel shown\n'Open larger'"]
C --> M["CMUXSidebarExtensionBrowserPanel\n(ObservableObject, Panel)"]
M -->|"browserViewController"| C
Reviews (2): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| await withCheckedContinuation { continuation in | ||
| continuationBox.set(continuation) | ||
| Task { | ||
| await continuationBox.set(continuation) | ||
| } | ||
| withObservationTracking { | ||
| applyModernExtensionState(monitor.state) | ||
| } onChange: { | ||
| continuationBox.resume() | ||
| Task { | ||
| await continuationBox.resume() | ||
| } | ||
| } | ||
| } | ||
| } onCancel: { | ||
| continuationBox.cancel() | ||
| Task { | ||
| await continuationBox.cancel() | ||
| } | ||
| } |
There was a problem hiding this comment.
Task { await continuationBox.set(continuation) }, Task { await continuationBox.resume() }, and Task { await continuationBox.cancel() } are all unstructured tasks with no parent in the Swift concurrency task tree. They are not cancelled when the enclosing Task is cancelled, and any errors they surface are silently dropped. Per the cmux-swift-concurrency-modernization rule, actor hops from non-isolated callbacks should use Task only when there is no structured alternative — but the lifecycle of these Tasks should be tied to the owning context. Consider capturing the Task references (or using withCheckedContinuation's structured cancellation directly) so the actor operations remain bound to the observation loop's lifetime.
Rule Used: Flag new legacy async patterns in cmux-owned Swift... (source)
| let isCompact = visibleBounds.width < Self.minimumUsableWidth || | ||
| visibleBounds.height < Self.minimumUsableHeight |
There was a problem hiding this comment.
visibleBounds without the same zero-fallback that the constraint update code applies. When viewDidMoveToWindow fires before the first Auto Layout pass, rootView.visibleRect returns CGRect.zero, so both comparisons evaluate as true (0 < 600, 0 < 420). This sets contentView.isHidden = true and compactLabel.isHidden = false on every initial window attachment, producing a visible flash of the "Open larger" label before the real layout arrives.
| let isCompact = visibleBounds.width < Self.minimumUsableWidth || | |
| visibleBounds.height < Self.minimumUsableHeight | |
| let isCompact = layoutWidth > 0 && layoutHeight > 0 && ( | |
| layoutWidth < Self.minimumUsableWidth || | |
| layoutHeight < Self.minimumUsableHeight | |
| ) |
- Add Send Ctrl-F to Terminal passthrough (manaflow-ai#5011, force-stop CC agents) - Fix sidebar worktree spawn worktree-setup-as-input bug (manaflow-ai#5032) - Add boundary-aware ranking layer for command-palette fuzzy search - Open extension browser as pane tab + polish (manaflow-ai#5053) - Move sidebar kind selection to titlebar menu, fix clipped tooltip and floor sidebar width (manaflow-ai#5045) - Extract CmuxFoundation package — modular refactor wave 1 (manaflow-ai#5055) - Center empty sidebar-extension state, fade host bottom edge (manaflow-ai#5057) - Align titlebar accessory hints (manaflow-ai#5059) - Restore sidebar minimum width (manaflow-ai#5062) Conflicts resolved: - cmux.xcodeproj/project.pbxproj: merge fork's CMUXSettingsCore + CMUXSessionDaemon package refs with upstream's new CmuxFoundation package reference and product dependency.
This branch was successfully deployed
1 participant
Summary
Validation
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by cubic
Adds a Sidebar Extensions browser as a pane tab, replacing the floating utility window. Polishes layout with adaptive insets, a compact “Open larger” message, and click-to-focus for a better experience in narrow panes.
New Features
PanelType.extensionBrowserandCMUXSidebarExtensionBrowserPanelrendered inside the pane.openSidebarExtensionBrowser(from:title:)to open from menu or host view anchor.sidebar.extensions.browser.compactforenandja.Refactors
makeViewController(title:);present(from:title:)is unavailable to enforce the pane-tab flow.PanelContentViewnow fills available space; file path header spacing and padding adjusted.Written for commit 4eb4d29. Summary will update on new commits.
Summary by CodeRabbit