Repository navigation
Draw resting sidebar rows lighter and give workspace titles their room back - #17726
azooz2003-bit wants to merge 27 commits into
Conversation
Every workspace title in the sidebar was semibold, so the list had no spare emphasis left for the rows that matter: a selected row and a row with unread notifications looked exactly as heavy as the twenty quiet rows around them. Resting titles now draw at regular weight. The selected row (including a row that joined a multi-selection) and any row with unread notifications keep semibold, so both states still read at a glance. Group header names step down from semibold to medium, which keeps them distinct from the member rows they sit above without shouting. The sidebar has two renderers, the AppKit table cells and the SwiftUI rows, and both used to hardcode the weight separately. They now resolve it through one shared SidebarRowTextWeight, so a row cannot look different depending on which one drew it. The inline rename field takes the same weight as the title it replaces, and the group header's height measurement uses the same font it draws with. Badges, links, buttons and the small secondary lines keep their current weights: those are 8 to 10 point and semibold is legibility there, not emphasis. No new setting and no new user-facing string. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The weight change is only judgeable side by side, and the four row states that matter (resting, selected, unread, group header) never appear together by accident. This tour builds all four in one window, including a title long enough to truncate, so before and after frames show truncation, row height and baseline alignment rather than weight alone. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Row heights are measured from the stored model, and weight changes text metrics: a wrapped title can need one more line at semibold than at regular. The selection preview applies a selection-flipped copy of the model for its colors, so with the title weight keyed off selection it could draw a row at one weight inside a frame measured at the other, and the pump height override can record that mis-measured height and keep it after the preview reverts. The title font now resolves from the stored model even while a painted copy supplies the colors. Colors still flip on press; the weight follows the authoritative apply. While renaming, the field tracks the same font, so a notification arriving mid-rename cannot leave it a weight behind. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
At the default 240pt sidebar width a title was cut off inside about a dozen characters, which is not enough to tell "Fix cmux pane focus indicator flicker" from "Fix cmux pane resize jitter". Three defaults were spending that width on something other than the title: - The title drew at 12.5pt. Row titles are labels in a narrow column, so they now draw at 12pt, shared by both renderers through SidebarRowTitleMetrics. - Every row kept 10pt of content padding on each side, on top of the 6pt outer padding. That is now 8pt. - Every row that can be closed held a 24pt trailing column open for a close button that only appears on hover. The column is now held open while the button (or a badge or spinner) is actually there. The reveal insets the title instead, which is safe for one line because a single line's height does not depend on its width. Wrapped titles keep the reservation, since hovering must not change their height. A one-line title now truncates in the middle rather than at the end, so it keeps its start and its distinctive tail: "cmux-remote-status @host" keeps the host, and a PR-shaped title keeps its last word. sidebar.twoLineWorkspaceTitles (off by default) is the middle ground between one line and sidebar.wrapWorkspaceTitles, which shows a title in full however many lines that takes. SidebarWorkspaceTitleRoomTests measures the before and after geometry with the same text layout the rows use and prints the visible-character counts for five real titles, so a later change that takes the room back fails there. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The tour is the before/after evidence for the title truncation defaults, so it now creates the same titles the character-count test measures, plus enough rows to fill the sidebar, and two names with a host suffix so the middle truncation is visible. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The five measured titles include one short enough to fit whole at the old geometry, so requiring strictly more visible characters for every title asked for room that does not exist. Require no title to lose room, a strict gain only where the title was being cut off, and a whole title to stay whole. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Four fixes from the review pass and from CI, none of them changes to the weight or the title-room behavior this PR is about. The trailing edge of a row can hold three things: the status badge or spinner, the hover-revealed close button, and the shortcut hint pill. Dropping the close button's standing reservation gave the title that width back, but the pill is drawn as an overlay rather than as a slot occupant, and it replaces the close button while it shows, so nothing was holding the column open for it: a held modifier, or `alwaysShowShortcutHints`, painted the pill over the end of the title. Middle truncation makes that end a part of the name worth reading, so the title now yields to the pill the same way it yields to the button, in both renderers. A title on more than one line keeps the reservation at all times, whatever is in it, because its height depends on its width and neither hover nor a held modifier may restate a row's height. `settings.app.twoLineWorkspaceTitles` and its subtitle were in the CmuxSettingsUI package catalog while all three of their call sites resolve against the main bundle, so they are moved to `Resources/Localizable.xcstrings` with the same nine locales. `SettingsRowAnchorResolutionTests.rowConfigPaths` is the hand-maintained list that makes a settings search hit scrollable, and the new row was missing from it, which is what `everyCuratedSettingEntryIsReachable` was reporting. `SidebarRowTitleMetrics` becomes a value built from the two title settings instead of a namespace of static functions: the line limit is what the truncation mode is derived from, so it belongs inside the type rather than being passed back in. That also clears the package-conventions lint, which rejects an all-static public type. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Catch-up merge by scripts/ci/catch_up_pr.py (RFC #14631). Merged by scripts/merge-main.sh: origin/main at 56ec600. Resolved conflicts: - Resources/Localizable.xcstrings: xcstrings key-level union - cmux.xcodeproj/project.pbxproj: union of added entries, then normalize-pbxproj.py - Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ConfigValidation/CmuxConfigSchema.generated.swift: generate-cmux-config-schema.py, regenerated from the merged schema (both sides changed the schema) Catch-up-previous-head: 896422c Catch-up-base: 56ec600
Catch-up merge by scripts/ci/catch_up_pr.py (RFC #14631). Merged by scripts/merge-main.sh: origin/main at b681e7e. Resolved conflicts: - Resources/Localizable.xcstrings: xcstrings key-level union - Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ConfigValidation/CmuxConfigSchema.generated.swift: generate-cmux-config-schema.py, regenerated from the merged schema (both sides changed the schema) Catch-up-previous-head: 984885d Catch-up-base: b681e7e
The badge test named `.byTruncatingTail`, which was the truncation mode before single line titles started shortening in their middle, so this branch turned it into an assertion about the old default and it failed on a file the branch does not otherwise touch. The claim the test needs is that the badge leaves the title a truncating single line rather than a wrapping one, so it now asserts the line limit and takes the mode from `SidebarRowTitleMetrics` for that limit. The modes themselves stay pinned by `SidebarRowTitleMetricsTests`. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Without `paths` the tour only ran when a pull request edited the tour file itself, so sidebar row changes got the broad sidebar tour and not this one. The globs are the row cells and the sidebar typography package, which is what the tour is there to show, and the note says which row states it captures. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Catch-up merge by scripts/ci/catch_up_pr.py (RFC #14631). Merged by scripts/merge-main.sh: origin/main at 56eacd4. Resolved conflicts: - Resources/Localizable.xcstrings: xcstrings key-level union - Packages/macOS/CmuxFoundation/Sources/CmuxFoundation/ConfigValidation/CmuxConfigSchema.generated.swift: generate-cmux-config-schema.py, regenerated from the merged schema (both sides changed the schema) Catch-up-previous-head: 05aedc3 Catch-up-base: 56eacd4
Catch-up merge by scripts/ci/catch_up_pr.py (RFC #14631). Merged by scripts/merge-main.sh: origin/main at 8b23dd7, the newest commit with green CI fast guards (1 newer skipped). Resolved conflicts: - Resources/Localizable.xcstrings: xcstrings key-level union Catch-up-previous-head: dda0455 Catch-up-base: 8b23dd7
Catch-up merge by scripts/ci/catch_up_pr.py (RFC #14631). Merged by scripts/merge-main.sh: origin/main at 0b2d3e0, the newest commit with green CI fast guards (1 newer skipped). Resolved conflicts: - cmux.xcodeproj/project.pbxproj: union of added entries, then normalize-pbxproj.py Catch-up-previous-head: 3c7a5df Catch-up-base: 0b2d3e0
…text-weight # Conflicts: # Sources/ContentView.swift # Sources/Sidebar/AppKitList/Cells/SidebarWorkspaceRowCellView.swift # Sources/SidebarTabItemSettingsSnapshot.swift
The tour read whatever the machine had stored for the sidebar row settings. cmux#15061 found that a fleet machine keeps settings from an earlier run, so "no launch argument" does not mean "the shipping default" in the frames. Pin the three settings that change the row layout, plus the presentation mode, so the frames show the default the PR is about. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…text-weight # Conflicts: # Sources/WindowChromeMetrics.swift # cmux.xcodeproj/project.pbxproj
Catch-up merge so the pull request can run CI again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 53 minutes. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Review configuration: ⚙️ Run configuration
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (28)
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 |
|
Note Pull Request opener @azooz2003-bit is not an author or co-author of any commit in this PR (commit identities: All contributors have signed the CLA ✍️ ✅ |
Summary
Two complaints about the same list, fixed together: the sidebar was all semibold, and workspace titles ran out of room long before they ran out of name.
Weight. Every workspace title was semibold, so the list had no spare emphasis left for the rows that matter. A selected row and a row with unread notifications looked exactly as heavy as the twenty quiet rows around them. Resting titles now draw at regular weight. The selected row (including a row that joined a multi-selection) and any row with unread notifications keep semibold, so both states still read at a glance. Group header names step down from semibold to medium, which keeps them distinct from the member rows below without shouting.
The sidebar has two renderers, the AppKit table cells and the SwiftUI rows, and each used to hardcode the weight on its own. Both now resolve it through one shared
SidebarRowTextWeightinCmuxSidebar, so a row cannot look different depending on which one drew it. Two details follow: the inline rename field takes the weight of the title it replaces, so text does not jump when you start renaming, and the group header's height measurement uses the same font it draws with.Room. Three defaults were spending horizontal space a title could have used: a 12.5pt title size, 10pt of content padding on each side, and a trailing column held open for a close button that only appears on hover. The title now draws at 12pt, content padding is 8pt, and a resting row spends the close button's column on its title until the button actually shows. A wrapped title keeps the reservation, because hovering must not change a row's height. One-line titles truncate in the middle instead of at the end, so a name like
cmux-remote-status @workstationkeeps both its start and the host that tells it apart from its siblings.sidebar.twoLineWorkspaceTitles(off by default) is a middle ground between one line and the existingsidebar.wrapWorkspaceTitles: long titles get a second line, short ones keep their height. It is the only user-facing string this PR adds: its Settings label, its subtitle and its search alias are in the catalog in all nine languages, and the schema andcmux.jsonaccept the key.Badges, links, buttons and the small secondary lines keep the weights they had. Those are 8 to 10 point, where semibold is legibility rather than emphasis.
How much this buys, and why the default width does not move
Title width at the 240pt default goes from 172pt to 212pt. Measured with the same text layout the rows use, these are the characters of each title that fit on the title line, before and after:
The obvious alternative was to widen the default sidebar. These numbers are the argument against it. Nothing here lands anywhere near the dozen characters the complaint described, and the one title that gains nothing is 24 characters long, so it already fitted end to end and has nothing left to gain. A wider default would cost every user horizontal space in the surface they actually work in, to fix a problem that the padding, the point size and the close column had already been causing on their own.
SidebarWorkspaceTitleRoomTestskeeps these numbers honest: it reads the current geometry from the shipping sources, requires that no title loses room, that a title which was being cut off gains strictly, that a title which fitted whole still fits whole, and that every title clears 20 characters.Those four clauses replaced a single one, "every title shows strictly more than before", which the first measurement run failed on the 24 character title. The failure was the assertion asking for room that cannot exist, not the change giving less; the replacement pins down more of the property than the original claim did, rather than relaxing it to fit the code.
Before and after
Both frames come from the same dogfood tour, at the same window size and the same default sidebar width: ten workspaces, one selected row, one unread row and a group.
The pair below was captured from commit
ad8b0f3696d, not from the current head. The branch has taken main merges since, including #14838's compact status glyph, and a grep of that delta for this change's own symbols (SidebarRowTitleMetrics,SidebarRowTextWeight,twoLineWorkspaceTitles,titleLineLimit) returns 16 lines inSources/ContentView.swiftandSources/Sidebar/AppKitList/Cells/SidebarWorkspaceRowCellView.swift, which are the paths these frames render. So they do not carry on that test, and the tour is being run again on the current head; the images above update in place when it finishes. The claim they support, that resting rows are lighter and long titles keep their tails, is unchanged, but treat them as the earlier commit's evidence until the new run lands.main)The sidebar alone, from the same tour run at 200 percent font magnification so the stroke weight and the truncation point are both legible. On the left every row is semibold and the long titles lose their tails. On the right only the selected row and the unread row are semibold, and the titles that have to truncate keep both their start and their end, so
cmux-remote-status @workstationstill shows the host. A CJK title and an emoji title are in the list as well, since a change to title measurement should be looked at with both.main)Testing
Added:
CmuxSidebarTests/SidebarRowTextWeightTestscovers the weight decision and pins the AppKit and SwiftUI mappings to each other, which is what stops the two renderers from drifting apart again.SidebarAppKitRowCellTests.workspaceTitleWeightTracksSelectionAndUnreadconfigures a row cell and reads the font off its title view for resting, selected, multi-selected and unread rows.SidebarWorkspaceTitleRoomTestsmeasures the table above and pins the close column's 24pt to the resting row.Executed: the focused runs linked in the comments, plus the two dogfood tours that produced the frames above (one on the branch at
ad8b0f3696d, one onmain, same scenario), with a fresh pair running on the current head.Changelog
Changed: Sidebar workspace titles are lighter and show more of their name, with semibold kept for the selected row and rows with unread notifications
Demo Video
Frames above.
Checklist
cmux sshString catalog merges
This branch adds catalog keys, so every merge from main runs through the repository's
.xcstringsmerge driver. After the latest merge the audit says the driver did its job: the merged key set is exactly the union of both sides, 7195 keys from parents of 7194 and 7192, with nothing missing and nothing invented. No key lost a locale it had on either side, and the four keys this PR adds are present in all nine languages.localization_catalog.py checkreports no parity errors, and the embedded config schema check passes.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Draws resting sidebar workspace titles at regular weight so selected and unread rows stand out, and gives titles more room at the default sidebar width.
SidebarRowTextWeightandSidebarRowTitleMetrics.sidebar.twoLineWorkspaceTitles(off by default) as a middle ground between one line andsidebar.wrapWorkspaceTitles; wrapping outranks it. The key is localized in all nine languages and wired into the config schema, settings search, andcmux.json.Written for commit a844a96. Summary will update on new commits.
Summary by CodeRabbit
Migrated from #15229 after correcting the PR author identity. The head branch and commit history are preserved.