Repository navigation
Scale sidebar workspace group header with sidebar font size (#5398) - #5401
Conversation
Extract the workspace group/folder header's font sizes, frames, and badge padding into a pure SidebarWorkspaceGroupHeaderMetrics helper and add a behavior-level test asserting the metrics scale with the sidebar font scale. This commit captures only the current (unscaled) behavior plus the failing test: the header still ignores the sidebar font scale, so the enlarged-scale assertions go red. The fix follows in the next commit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The collapsible workspace group/folder header (chevron, folder icon, group name, unread badge, + button) hardcoded every font size and frame, so it stayed small while the workspace rows grew with the configurable sidebar-font-size. Thread the existing sidebar font scale (settings.sidebarFontScale, the same SidebarTabItemFontScale value that already feeds TabItemView) into SidebarWorkspaceGroupHeaderView and drive every size through SidebarWorkspaceGroupHeaderMetrics, which multiplies each base size by the scale. fontScale joins the Equatable == so a font-size change invalidates the header under the LazyVStack. No second scaling path is introduced; the header and rows now grow at the same rate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds ChangesSidebar workspace group header font scaling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 17 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (17 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Greptile SummaryThis PR fixes the sidebar workspace group/folder header not scaling with the configurable
Confidence Score: 5/5Safe to merge — a purely presentational change confined to the sidebar header with no data, auth, or business-logic impact. The change is narrow and well-contained: a new pure value type extracts the scaling math, the view adds one immutable property and wires it through the existing single scaling path, and fontScale is correctly included in the manual Equatable implementation so the .equatable() optimisation under LazyVStack still triggers on font-size changes. No new state, no blocking primitives, no actor isolation issues, no new user-facing strings, and unit tests cover the key scaling invariants. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[settings.sidebarFontScale] -->|"fontScale"| B[SidebarWorkspaceGroupHeaderView]
A -->|"sidebarFontScale"| C[TabItemView]
B --> D["SidebarWorkspaceGroupHeaderMetrics(fontScale:)"]
D --> E[chevronFontSize / chevronFrame]
D --> F[iconFontSize / iconFrame]
D --> G[nameFontSize]
D --> H[unreadFontSize / unreadHPad / unreadVPad]
D --> I[plusFontSize / plusFrame]
J[SidebarTabItemFontScale.scale] -->|"produces fontScale"| A
B -->|".equatable() — fontScale in =="| K[LazyVStack invalidation]
Reviews (2): Last reviewed commit: "Add scale-down coverage for sidebar grou..." | Re-trigger Greptile |
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 `@cmuxTests/SidebarWorkspaceGroupHeaderMetricsTests.swift`:
- Around line 10-52: Add a new test in SidebarWorkspaceGroupHeaderMetricsTests
that verifies scaling down (fontScale < 1) by creating
SidebarWorkspaceGroupHeaderMetrics(fontScale: 0.5) and asserting the key metrics
(chevronFontSize, chevronFrame, iconFontSize, iconFrame, nameFontSize,
unreadFontSize, unreadHorizontalPadding, unreadVerticalPadding, plusFontSize,
plusFrame) equal their corresponding base values multiplied by 0.5; name the
test metricsScaleProportionallyWhenSidebarFontReduced to mirror
metricsScaleProportionallyWhenSidebarFontEnlarged and keep the same assertion
style (`#expect`) used in the suite.
🪄 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: 85922c56-ff82-4e1e-9878-1e4dbe5bc2bc
📒 Files selected for processing (5)
Sources/SidebarWorkspaceGroupHeaderMetrics.swiftSources/SidebarWorkspaceGroupHeaderView.swiftSources/VerticalTabsSidebar+WorkspaceGroups.swiftcmux.xcodeproj/project.pbxprojcmuxTests/SidebarWorkspaceGroupHeaderMetricsTests.swift
There was a problem hiding this comment.
No issues found across 5 files
You’re at about 98% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Address CodeRabbit review: assert the metrics shrink proportionally at fontScale < 1 (0.5) in addition to the enlarged (2.0) and default (1.0) cases, covering the full proportional-scaling contract. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Fixes #5398. cmux's sidebar font size is configurable via
sidebar-font-size, and the workspace rows already scale with it — but the workspace group/folder header (the collapsible row with the chevron, folder icon, group name, unread badge, and+button) hardcoded every font size and frame, so enlarging the sidebar font left the header small and mismatched against the rows beneath it.Root cause
SidebarWorkspaceGroupHeaderView(Sources/SidebarWorkspaceGroupHeaderView.swift) hardcoded every font (.system(size: 9/10/11)) and frame (14x14,18x18) and never received the sidebar font scale. The workspace-row path threadssettings.sidebarFontScale(=SidebarTabItemFontScale.scale(for:),1.0at the default size) intoTabItemViewand multiplies every size by it. The header is built atVerticalTabsSidebar+WorkspaceGroups.swift:77, wheresettingsis already in scope, but the scale was never passed in.Fix
SidebarWorkspaceGroupHeaderMetricsvalue type that multiplies each base size byfontScale.let fontScale: CGFloattoSidebarWorkspaceGroupHeaderViewand included it in theEquatable ==so a font-size change invalidates the header under theLazyVStack.settings.sidebarFontScalein at the call site — reusing the single scaling path that already feedsTabItemView, not adding a second one.The outer/inner
HStackspacings stay fixed, matchingTabItemView(which also keeps fixed spacing), so the row layout stays stable while glyphs/frames scale.Tests
Two-commit red/green structure (
SidebarWorkspaceGroupHeaderMetricsTests, Swift Testing):1.0) and enlarged (2.0) scales, and that the header reusesSidebarTabItemFontScale— CI goes red.fontScaleand wires the scale into the view + call site — CI goes green.Test file is wired into the
cmuxunit test target inproject.pbxproj(lint-pbxproj-test-wiringpasses, 152 files).Localization
No new user-facing strings.
🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Low Risk
Localized SwiftUI sizing and equatable snapshot changes for the group header, with unit tests and no auth or data-path changes.
Overview
Fixes workspace group/folder headers staying at fixed sizes when
sidebar-font-sizeis increased, so they no longer look smaller than the workspace rows below them.Introduces
SidebarWorkspaceGroupHeaderMetricswith base design sizes multiplied byfontScale, and wiresSidebarWorkspaceGroupHeaderViewto use those metrics for chevron, icon, title, unread badge, and plus control instead of hardcoded.system(size:)and frames. The sidebar passessettings.sidebarFontScale(same path asTabItemViewviaSidebarTabItemFontScale) and includesfontScaleinEquatableso font-size changes refresh the equatable header in theLazyVStack. Unit tests cover default, enlarged, and shrunk scaling plus shared scaling with row font scale.Reviewed by Cursor Bugbot for commit 933425b. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Make the sidebar workspace group header scale with the configurable sidebar font size so it matches the workspace rows. Removes hardcoded sizes and uses the same scaling path as rows for consistent sizing.
settings.sidebarFontScaleintoSidebarWorkspaceGroupHeaderViewand includefontScaleinEquatableto re-render on font-size change.SidebarWorkspaceGroupHeaderMetrics, multiplying base values by the scale for icons, text, badge, and plus button.VerticalTabsSidebar+WorkspaceGroups.swift; keep existing HStack spacings unchanged for stable layout.SidebarWorkspaceGroupHeaderMetricsTeststo verify scaling at default, enlarged, and shrunk sizes, and reuse ofSidebarTabItemFontScale.Written for commit 933425b. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests