Repository navigation
Add right sidebar titlebar toggle - #7999
Closed
austinywang wants to merge 27 commits into
Closed
austinywang wants to merge 27 commits into
austinywang wants to merge 27 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds a right-sidebar titlebar toggle, replaces mobile-connect accessory wiring, updates titlebar accessory visibility and attachment handling, removes the in-sidebar close control, and revises AppKit, unit, and UI tests. ChangesTitlebar control integration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (22 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 |
Contributor
Greptile SummaryThis PR adds a right-sidebar control to the trailing edge of the title bar. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (23): Last reviewed commit: "test: keep minimal split controls beside..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmuxUITests/BonsplitTabDragUITests.swift`:
- Around line 200-204: The UI test’s titlebarToggle lookup uses the undefined
identifier titlebarControl.toggleRightSidebar. Update BonsplitTabDragUITests to
use the existing titlebarControl.toggleSidebar identifier, or add the matching
accessibility identifier to the right-sidebar control in WindowDragHandleView or
UpdateTitlebarAccessory, ensuring the test targets the actual control.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: cd2ae2c5-4861-4b6e-a8f7-96669b686b60
📒 Files selected for processing (1)
cmuxUITests/BonsplitTabDragUITests.swift
austinywang
force-pushed
the
issue-7998-right-sidebar-toggle-icon
branch
from
July 13, 2026 22:59
6bda984 to
dcffa2e
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmuxUITests/BonsplitTabDragUITests.swift (1)
182-252: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding fullscreen coverage for the titlebar toggle test.
This test only runs with the default
.minimalpresentation mode, so it verifies the toggle stays available in minimal mode but not in fullscreen, even though the linked issue explicitly requires the control to remain accessible in fullscreen.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmuxUITests/BonsplitTabDragUITests.swift` around lines 182 - 252, Extend testRightSidebarTitlebarToggleStaysAvailableAcrossVisibilityChanges to launch and exercise the app in fullscreen presentation mode in addition to the existing .minimal coverage. Reuse the same visibility, shortcut, selection, and hit-testing assertions for the fullscreen instance so the titlebarControl.toggleRightSidebar remains accessible and functional when the sidebar is hidden and shown.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/macOS/CmuxAppKitSupportUI/Sources/CmuxAppKitSupportUI/WindowChrome/Titlebar/NativeTitlebarBackdropCoordinator.swift`:
- Around line 81-91: Centralize the titlebar accessory identifiers by exposing
public constants from NativeTitlebarBackdropCoordinator (or a shared constants
type), and build its controlsIds set from those constants. In
Sources/Update/UpdateTitlebarAccessory.swift at line 2434, initialize
trailingControlsIdentifier and controlsIdentifier from the package constants
instead of literals; lines 2665 and 2680 require no direct changes once that
identifier is shared.
In `@Sources/MobileConnectTitlebarButton.swift`:
- Around line 13-28: Update the Button in MobileConnectTitlebarButton to apply
the existing titlebarInteractiveControl() modifier alongside its other button
modifiers, ensuring the mobile-connect control registers as an interactive
titlebar region.
In `@Sources/Update/UpdateTitlebarAccessory.swift`:
- Around line 2620-2654: Update attachTrailingControlsIfNeeded(to:) to use the
passed window’s registered context via contextForMainWindow(window), rather than
preferredRegisteredMainWindowContext(preferredWindow:), and keep removing any
existing accessory when no context is registered. Ensure the trailing
accessory’s fileExplorerState and toggle callback remain scoped to that same
window.
---
Outside diff comments:
In `@cmuxUITests/BonsplitTabDragUITests.swift`:
- Around line 182-252: Extend
testRightSidebarTitlebarToggleStaysAvailableAcrossVisibilityChanges to launch
and exercise the app in fullscreen presentation mode in addition to the existing
.minimal coverage. Reuse the same visibility, shortcut, selection, and
hit-testing assertions for the fullscreen instance so the
titlebarControl.toggleRightSidebar remains accessible and functional when the
sidebar is hidden and shown.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 11a4c480-8725-482f-a70f-aff0a152d392
📒 Files selected for processing (13)
Packages/macOS/CmuxAppKitSupportUI/Sources/CmuxAppKitSupportUI/WindowChrome/Titlebar/NativeTitlebarBackdropCoordinator.swiftPackages/macOS/CmuxAppKitSupportUI/Tests/CmuxAppKitSupportUITests/WindowChrome/Titlebar/NativeTitlebarBackdropCoordinatorTests.swiftSources/ContentView.swiftSources/MobileConnectTitlebarAccessory.swiftSources/MobileConnectTitlebarButton.swiftSources/RightSidebarPanelView.swiftSources/RightSidebarTitlebarToggleButton.swiftSources/RightSidebarTitlebarToggleOverlay.swiftSources/TitlebarTrailingAccessoryViewController.swiftSources/Update/TitlebarControlButtonStyleBody.swiftSources/Update/UpdateTitlebarAccessory.swiftcmux.xcodeproj/project.pbxprojcmuxUITests/BonsplitTabDragUITests.swift
💤 Files with no reviewable changes (1)
- Sources/MobileConnectTitlebarAccessory.swift
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/RightSidebarPanelView.swift (1)
288-295: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake the titlebar spacer non-hit-testable.
The transparent reservation sits aboveWindowDragHandleView, so this slot can swallow titlebar drags. Add.allowsHitTesting(false)to keep it layout-only.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/RightSidebarPanelView.swift` around lines 288 - 295, Add .allowsHitTesting(false) to trailingTitlebarControlReservation so the transparent titlebar reservation remains layout-only and does not intercept WindowDragHandleView drag interactions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Sources/RightSidebarPanelView.swift`:
- Around line 288-295: Add .allowsHitTesting(false) to
trailingTitlebarControlReservation so the transparent titlebar reservation
remains layout-only and does not intercept WindowDragHandleView drag
interactions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 44cc83fe-a23f-4189-811c-728a8d5053cd
📒 Files selected for processing (12)
Packages/macOS/CmuxAppKitSupportUI/Sources/CmuxAppKitSupportUI/WindowChrome/Titlebar/NativeTitlebarBackdropCoordinator.swiftPackages/macOS/CmuxAppKitSupportUI/Tests/CmuxAppKitSupportUITests/WindowChrome/Titlebar/NativeTitlebarBackdropCoordinatorTests.swiftSources/ContentView.swiftSources/RightSidebarPanelView.swiftSources/RightSidebarTitlebarToggleButton.swiftSources/RightSidebarTitlebarToggleOverlay.swiftSources/TitlebarTrailingAccessoryViewController.swiftSources/Update/UpdateTitlebarAccessory.swiftcmuxTests/AppDelegateShortcutRoutingTests.swiftcmuxTests/HiddenRightSidebarContentMountingTests.swiftcmuxTests/WindowAndDragTests.swiftcmuxUITests/BonsplitTabDragUITests.swift
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/RightSidebarTitlebarToggleButton.swift (1)
37-40: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFill in the missing tooltip translations
Resources/Localizable.xcstringshasshortcut.toggleRightSidebar.labelin all supported locales, butrightSidebar.toggle.tooltiponly hasenandja. Add the remaining locale entries or reuse a shared key if this text is meant to be identical.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/RightSidebarTitlebarToggleButton.swift` around lines 37 - 40, Update the localization resources for rightSidebar.toggle.tooltip to include entries for every supported locale, or reuse the existing shared localization key if the tooltip text is identical. Keep the RightSidebarTitlebarToggleButton accessibility label unchanged.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/TitlebarTrailingAccessoryViewController.swift`:
- Line 2: Replace the new Combine-based notification subscription associated
with TitlebarTrailingAccessoryViewController with an async/await loop using
NotificationCenter.notifications(named:). Remove the now-unused Combine import
and preserve the existing refresh behavior for each notification.
---
Outside diff comments:
In `@Sources/RightSidebarTitlebarToggleButton.swift`:
- Around line 37-40: Update the localization resources for
rightSidebar.toggle.tooltip to include entries for every supported locale, or
reuse the existing shared localization key if the tooltip text is identical.
Keep the RightSidebarTitlebarToggleButton accessibility label unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9953eef8-6a9b-4d67-b58b-8e1817757756
📒 Files selected for processing (3)
Sources/RightSidebarTitlebarToggleButton.swiftSources/TitlebarTrailingAccessoryViewController.swiftcmuxUITests/BonsplitTabDragUITests.swift
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
1 similar comment
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
austinywang
force-pushed
the
issue-7998-right-sidebar-toggle-icon
branch
from
July 14, 2026 04:45
6ba7d8c to
55cb19b
Compare
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
CMUX_DISABLE_AUTOMATIC_PACKAGE_RESOLUTION=1 ./scripts/reload.sh --tag issue-7998-right-sidebar-toggle-icon --launchtitlebarControl.toggleRightSidebarinside the collapsed edge celltestRightSidebarUsesCloseButtonWhenVisibleAndTitlebarToggleWhenHiddenacross standard/minimal presentation modes with native trailing-control collision assertions; XCUITests run in CI per repository policyWorkspaceRuntimeSettings.swiftoverage onorigin/main)Closes #7998