feat: configurable sidebar position - #4826
Closed
austinywang wants to merge 22 commits into
Closed
austinywang wants to merge 22 commits into
austinywang wants to merge 22 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a configurable ChangesSidebar Position Configuration and Layout
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (14 passed)
✨ 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
Greptile SummaryAdds
Confidence Score: 5/5Safe to merge; layout, resizer, and width-accounting changes are well-scoped to sidebar chrome and do not touch terminal rendering or persistence paths. All four position paths have corresponding layout branches, tests, and locale entries. Previous review findings are confirmed resolved. Swift string keys reused in HorizontalTabsSidebar were verified pre-existing in the full xcstrings catalog. No correctness issues found in drag-direction reversal, divider hit-testing, or file-explorer width accounting. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[sidebarPositionRaw] --> B[SidebarPositionSettings.resolved]
B --> C{SidebarPositionOption}
C -->|left| D[SidebarContentLayoutPolicy.mode]
C -->|right| D
C -->|top| D
C -->|bottom| D
D -->|left+withinWindow| E[leftOverlay]
D -->|left+stack| F[leftStack]
D -->|right| G[rightStack]
D -->|top| H[topStack]
D -->|bottom| I[bottomStack]
G --> J[drag reversed, dividerX=totalWidth-sidebarWidth]
G --> K[rightSidebarAvailableWidth=totalWidth-workspaceSidebarWidth]
H --> L[HorizontalTabsSidebar h=48pt]
I --> L
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[sidebarPositionRaw] --> B[SidebarPositionSettings.resolved]
B --> C{SidebarPositionOption}
C -->|left| D[SidebarContentLayoutPolicy.mode]
C -->|right| D
C -->|top| D
C -->|bottom| D
D -->|left+withinWindow| E[leftOverlay]
D -->|left+stack| F[leftStack]
D -->|right| G[rightStack]
D -->|top| H[topStack]
D -->|bottom| I[bottomStack]
G --> J[drag reversed, dividerX=totalWidth-sidebarWidth]
G --> K[rightSidebarAvailableWidth=totalWidth-workspaceSidebarWidth]
H --> L[HorizontalTabsSidebar h=48pt]
I --> L
Reviews (11): Last reviewed commit: "fix: instantiate sidebar selection polic..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ee1a73d. Configure here.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/KeyboardShortcutSettingsFileStore.swift`:
- Around line 652-658: The current block only logs invalid values when the
string exists but doesn't map to SidebarPositionOption; if section["position"]
is present but not a string it silently ignores it. Update the logic around
jsonString(section["position"]) in the code that sets
snapshot.managedUserDefaults[SidebarPositionSettings.key] so that if jsonString
returns nil but section["position"] is non-nil you call
logInvalid("sidebar.position", sourcePath: sourcePath); keep the existing branch
that converts a valid string to SidebarPositionOption and logs when the raw
string doesn't map, using the same identifiers (jsonString,
SidebarPositionOption, snapshot.managedUserDefaults,
SidebarPositionSettings.key, logInvalid, sourcePath).
In `@web/data/cmux.schema.json`:
- Around line 624-634: Add a descriptionKey for the user-facing schema
description on the "position" property under the "sidebar" schema so the
description is localizable; specifically add "descriptionKey":
"schemaDescriptions.sidebar.position" alongside the existing "description" in
the schema and then add matching entries for
"schemaDescriptions.sidebar.position" to each locale file in web/messages (one
translation per locale listed in web/i18n/routing.ts) so the message exists for
all supported locales.
🪄 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: f5b4987a-2859-48ed-bfda-bf6a73402124
📒 Files selected for processing (12)
Sources/CmuxSettingsJSONPathSupport.swiftSources/ContentView.swiftSources/KeyboardShortcutSettingsFileStore+Template.swiftSources/KeyboardShortcutSettingsFileStore.swiftSources/SettingsNavigation.swiftSources/Sidebar/SidebarState.swiftcmuxTests/KeyboardShortcutSettingsFileStoreStartupTests.swiftcmuxTests/SettingsSearchIndexTests.swiftcmuxTests/SidebarWidthPolicyTests.swiftdocs/configuration.mdweb/app/[locale]/docs/configuration/page.tsxweb/data/cmux.schema.json
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/HorizontalTabsSidebar.swift`:
- Around line 183-194: The Shift-range anchor is being computed against
tabManager.tabs (using firstIndex where id == workspaceId) but the UI is
rendered from SidebarWorkspaceRenderItem.renderItems(...), so update the
selection logic in the handlers that touch lastSidebarSelectionIndex and range
calculation to use the rendered order: derive a renderedIds array from
SidebarWorkspaceRenderItem.renderItems(...) (or the same source the view uses),
compute indices and ranges using renderedIds (findIndex of workspaceId), persist
lastSidebarSelectionIndex referencing the rendered index, and when a workspaceId
is not present in renderedIds clear lastSidebarSelectionIndex; apply this change
for the occurrences that set or use lastSidebarSelectionIndex, the Shift-range
computation (currently using tabManager.tabs[...] and map(\.id)), and the
single-click selection path so all anchors are against rendered order.
- Line 161: The UI is using a simple workspaceCount > 1 check instead of the
TabManager policy; replace those boolean checks with the
TabManager.canCloseWorkspace(_:) call so pinned workspaces follow the same close
rules. Find where canCloseWorkspace is set (e.g., in HorizontalTabsSidebar where
you currently use workspaceCount > 1) and change it to use the TabManager
instance method (for example TabManager.shared.canCloseWorkspace(workspace) or
the local tabManager.canCloseWorkspace(workspace) depending on available
context), and apply the same replacement for the other occurrences referenced
(around the blocks at the other noted locations).
🪄 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: fc3ce71c-20ba-4320-8362-918e5c3138c2
📒 Files selected for processing (3)
Sources/ContentView.swiftSources/HorizontalTabsSidebar.swiftcmux.xcodeproj/project.pbxproj
💤 Files with no reviewable changes (1)
- Sources/ContentView.swift
Contributor
Author
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 30 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
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
sidebar.positionconfig support forleft,top,right, andbottomTesting
git diff --checkscripts/check-pbxproj.shplutil -lint cmux.xcodeproj/project.pbxprojweb/data/cmux.schema.jsonand allweb/messages/*.jsonLocal builds/tests were not run per workspace instruction. Launch verification is intentionally deferred until the maintainer requests the required tagged command:
CMUX_SKIP_ZIG_BUILD=1 ./scripts/reload.sh --tag issue-2919-feat-configurable-sidebar-position-top --launchDemo Video
Not included yet. This PR changes macOS app layout, and the requested local dev launch must wait for explicit maintainer approval before recording or manual inspection.
Review Trigger
Pushed the latest fixes to
origin/issue-2919-feat-configurable-sidebar-position-top; GitHub checks and configured review bots are triggered from the branch update. If CodeRabbit is rate-limited, re-run with@coderabbitai reviewafter quota resets.Closes #2919
Note
Medium Risk
Touches main window layout, sidebar resizing, and portal geometry sync; broad surface area but localized to sidebar chrome with tests for import and layout policy.
Overview
Adds
sidebar.position(left|top|right|bottom) so the workspace sidebar can sit on any window edge; default remains left.Top/bottom use a new
HorizontalTabsSidebar(scrollable workspace tabs with groups, unread, multi-select, close/new/toggle) instead of the vertical sidebar.ContentViewpicks layout viaSidebarContentLayoutPolicy(left overlay/stack, right stack, top/bottomVStack) and reacts when the setting changes (geometry, traffic lights, width clamps).Right placement updates divider hit-testing, resizer edge/drag direction, and right file-explorer width so it does not overlap the workspace sidebar. Horizontal bars skip the vertical resize band. Setting is wired through JSON/JSONPath, settings template, schema, docs, locales, and tests.
Reviewed by Cursor Bugbot for commit 9597bbc. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds a configurable
sidebar.positionso the workspace sidebar can sit on any edge. Top/bottom render a horizontal tab strip; layout, resizers, and the right explorer adapt. Default stays left.New Features
sidebar.position(left|top|right|bottom) with schema, docs, locales, settings template, settings search anchor, JSON import, and tests.HorizontalTabsSidebarfortop/bottom: groups-aware tabs with multi/range select, unread badges, pin/close, keyboard hints, auto-scroll to selection, toggle-sidebar/new-workspace, copy workspace ID; driven by a centralized render model.SidebarContentLayoutPolicyand modes: left overlay/stack, right stack, top/bottom stacks; right sidebar available width accounts for a right-positioned workspace sidebar; horizontal bars skip the resizer band.Bug Fixes
right; updated resizer overlay/hit-testing; clamped widths on position/setting changes.Written for commit a12deb3. Summary will update on new commits.
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by CodeRabbit
New Features
Settings & Persistence
Documentation
Tests