Add outline style for workspace selection - #9559
Closed
austinywang wants to merge 2 commits into
Closed
austinywang wants to merge 2 commits into
austinywang wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughChangesWorkspace outline indicator
Estimated code review effort: 3 (Moderate) | ~20 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 (2 errors, 1 warning)
✅ Passed checks (22 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 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
Author
|
@codex review |
@austinywang I can't start this review because your workspace has reached its free monthly review limit. cubic has reviewed 245,275 of the 240,000 allowed lines of code this month. Reviews resume on 1 September 2026 (in 28 days). Paid plans include much higher monthly review limits. Upgrade now to resume reviews. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
|
To use Codex here, create a Codex account and connect to github. |
|
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@Resources/Localizable.xcstrings`:
- Around line 214050-214078: Update the localization entry
sidebar.activeTabIndicator.outline to include translated stringUnit values for
ar, bs, da, de, es, fr, it, km, nb, pl, pt-BR, ru, th, tr, zh-Hans, and zh-Hant,
while preserving the existing translations.
In `@web/data/cmux.schema.json`:
- Around line 1150-1153: Update the selectionColor schema definition to
reference a descriptionKey for its localized description, add the corresponding
maintained entries in the English and Japanese message catalogs, and regenerate
the schema reference document so the localized description is reflected
consistently.
🪄 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 Plus
Run ID: d313bd96-cb78-44ec-9d5a-9817edf4abb3
📒 Files selected for processing (11)
Packages/macOS/CmuxSettings/Sources/CmuxSettings/Values/WorkspaceIndicatorStyle.swiftPackages/macOS/CmuxSettingsUI/Sources/CmuxSettingsUI/Sections/WorkspaceColorsSection.swiftResources/Localizable.xcstringsSources/ContentView.swiftSources/Sidebar/AppKitList/Cells/SidebarWorkspaceRowCellView.swiftSources/Sidebar/AppKitList/Cells/SidebarWorkspaceRowSupportViews.swiftSources/Sidebar/SidebarAppearanceSupport.swiftSources/WorkspaceIndicatorStyle+Display.swiftcmuxTests/SidebarAppKitRowCellTests.swiftskills/cmux-settings/references/all-keys.mdweb/data/cmux.schema.json
Comment on lines
+214050
to
+214078
| "sidebar.activeTabIndicator.outline": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Outline" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "アウトライン" | ||
| } | ||
| }, | ||
| "ko": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "윤곽선" | ||
| } | ||
| }, | ||
| "uk": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Контур" | ||
| } | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Add translations for every supported locale.
sidebar.activeTabIndicator.outline currently defines only en, ja, ko, and uk. Add entries for ar, bs, da, de, es, fr, it, km, nb, pl, pt-BR, ru, th, tr, zh-Hans, and zh-Hant before merge.
As per path instructions, new localization keys must include translated values for every locale supported by the touched catalog.
🤖 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 `@Resources/Localizable.xcstrings` around lines 214050 - 214078, Update the
localization entry sidebar.activeTabIndicator.outline to include translated
stringUnit values for ar, bs, da, de, es, fr, it, km, nb, pl, pt-BR, ru, th, tr,
zh-Hans, and zh-Hant, while preserving the existing translations.
Sources: Path instructions, Learnings
Comment on lines
1150
to
+1153
| "selectionColor": { | ||
| "$ref": "#/$defs/colorHexOrNull", | ||
| "default": null, | ||
| "description": "Override the selected workspace background color." | ||
| "description": "Override the selected workspace background or outline color." |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Localize the changed schema description.
Line 1153 adds user-facing outline behavior as raw English text. Add a descriptionKey, add the matching maintained web catalog entries, and regenerate the reference document.
As per path instructions, user-facing schema descriptions must have matching localized message coverage. Based on learnings, schema-description keys are maintained in web/messages/en.json and web/messages/ja.json.
🤖 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 `@web/data/cmux.schema.json` around lines 1150 - 1153, Update the
selectionColor schema definition to reference a descriptionKey for its localized
description, add the corresponding maintained entries in the English and
Japanese message catalogs, and regenerate the schema reference document so the
localized description is reflected consistently.
Sources: Coding guidelines, Path instructions, Learnings
austinywang
force-pushed
the
issue-7460-sidebar-active-workspace-indicator-that
branch
from
August 4, 2026 08:03
8df5934 to
2ab0cc7
Compare
lawrencecchen
added
the
stale-revisit
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
Fixes #7460
Testing
./scripts/reload-cloud.sh xctest --tag sym7460-red3 --scheme cmux-unit --only-testing cmuxTests/SidebarAppKitRowCellTests(fleet runsym7460-red3-461055e1ea59) failed the new test on all four intended assertions: decoded style, preserved fill, 1.5-point border, and selection-color stroke../scripts/reload-cloud.sh --tag sym7460 --launchvia the cloud Blacksmith path, run 30884758925, including deep-signature verification./tmp/cmux-debug-sym7460.sock: with workspace color#C0392B,workspaceColors.indicatorStyle = outline, and selection color#123456, the active row retained the red fill and showed a distinct dark outline. Pixel sampling measured the border assrgba(27,51,83,1)and the interior assrgba(93,47,43,1)./tmp/cmux-debug-sym7460.sock../scripts/lint-pbxproj-test-wiring.shResources/Localizable.xcstringsandweb/data/cmux.schema.jsonwithjq; audited the changed Swift/UI strings and verified the Outline label matches the existing indicator locales.Checklist
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Adds an Outline active-workspace indicator that keeps the workspace fill and draws a 1.5pt border using the selection color. Unifies sidebar selection visuals across AppKit and SwiftUI, exposes the option in Settings and the config schema. Fixes #7460.
New Features
outlineindicator style in Settings with a localized label; selection color subtitle now says “Background or outline.”outlinewhile preserving the workspace fill; inverted foreground is off foroutline.outline; tests verify style decoding, preserved fill, 1.5pt stroke, and selection-color usage.Bug Fixes
Written for commit 2ab0cc7. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes