Add configurable tab title typography#35
Add configurable tab title typography#35alexgorbatchev wants to merge 2 commits intomanaflow-ai:mainfrom
Conversation
📝 WalkthroughWalkthroughIntroduces TabBarTypography to derive SwiftUI fonts for tab titles from BonsplitConfiguration.Appearance (tabTitleFontFamily, tabTitleFontScale). Replaces fixed title fonts in tab views with the new API and adds tests for font resolution, scaling, and non-finite scale handling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Greptile SummaryThis PR adds two new
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["TabItemView / TabDragPreview\n.font(TabBarTypography.titleFont(for: appearance))"] --> B["TabBarTypography.titleFont"]
B --> C["resolvedTitleNSFont\nscaledSize = max(0.5, scale) × titleFontSize"]
C --> D{"tabTitleFontFamily\nset?"}
D -- "nil / empty" --> E["NSFont.systemFont(ofSize: scaledSize)"]
D -- "non-empty" --> F["resolvedFont(family:size:weight:)"]
F --> G["resolvedPostScriptFontName"]
G --> H["NSFontManager.shared.font(withFamily:traits:weight:size:)"]
H --> I{"fontMatchesRequestedFamily?"}
I -- "✓ match" --> J["return fontName"]
I -- "✗ no match" --> K["NSFont(descriptor: systemDescriptor.withFamily(family), size:)"]
K --> L{"fontMatchesRequestedFamily?"}
L -- "✓ match" --> J
L -- "✗ no match" --> M["NSFont(name: family, size:)"]
M --> N{"familyName match\nOR postScript name match?"}
N -- "✓" --> J
N -- "✗" --> O["return nil"]
O --> E
J --> P["NSFont(name: postScriptName, size:)"]
P --> Q["Font(nsFont)\n→ SwiftUI Font"]
E --> Q
Reviews (1): Last reviewed commit: "Add configurable tab title typography" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| /// `1.0` preserves the current size. | ||
| public var tabTitleFontScale: CGFloat |
There was a problem hiding this comment.
tabTitleFontScale
The implementation in TabBarTypography.resolvedTitleNSFont silently clamps the scale to a minimum of 0.5 via max(0.5, appearance.tabTitleFontScale). The public doc-comment for this property doesn't mention this floor, so a host app passing 0.1 will be surprised that the effective minimum is 0.5.
| /// `1.0` preserves the current size. | |
| public var tabTitleFontScale: CGFloat | |
| /// Multiplier applied to pane tab title size. | |
| /// `1.0` preserves the current size. Values below `0.5` are clamped to `0.5`. | |
| public var tabTitleFontScale: CGFloat |
|
|
||
| let systemDescriptor = NSFont.systemFont(ofSize: size, weight: weight).fontDescriptor | ||
| let familyDescriptor = systemDescriptor.withFamily(family) | ||
| if let font = NSFont(descriptor: familyDescriptor, size: size), | ||
| fontMatchesRequestedFamily(font, family: family) { | ||
| return font.fontName | ||
| } |
There was a problem hiding this comment.
The intermediate lookup builds a new NSFontDescriptor by calling systemDescriptor.withFamily(family). withFamily only overrides the NSFontFamilyAttribute key, but the system font descriptor carries private San Francisco–specific attributes (e.g., NSCTFontUIUsageAttribute) that rank above a plain family match during font matching. In practice, NSFont(descriptor: familyDescriptor, size:) will almost always return another San Francisco variant rather than the requested family, which is then caught and rejected by fontMatchesRequestedFamily. As a result this branch is effectively dead code and adds confusion without contributing coverage.
Consider removing it so the fallback path is a cleaner: first NSFontManager, then NSFont(name:size:).
![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 the current code and only fix it if needed.
Inline comments:
In `@Sources/Bonsplit/Internal/Styling/TabBarTypography.swift`:
- Around line 10-15: Sanitize appearance.tabTitleFontScale before computing
scaledSize in TabBarTypography: read the raw scale into a local (e.g., let
rawScale = appearance.tabTitleFontScale), if rawScale.isFinite is false then
replace it with a sane default (e.g., 1.0), then compute let scaledSize =
max(0.5, sanitizedScale) * TabBarMetrics.titleFontSize and proceed to call
resolvedFont(...) ?? NSFont.systemFont(ofSize: scaledSize); update any relevant
tests for resolvedFont/tabTitleFontScale to include NaN and +∞ inputs to ensure
the guard works.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: af1c5463-83e8-4a57-9cd4-a3bcfea90585
📒 Files selected for processing (5)
Sources/Bonsplit/Internal/Styling/TabBarTypography.swiftSources/Bonsplit/Internal/Views/TabDragPreview.swiftSources/Bonsplit/Internal/Views/TabItemView.swiftSources/Bonsplit/Public/BonsplitConfiguration.swiftTests/BonsplitTests/BonsplitTests.swift
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 5 files
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Sources/Bonsplit/Internal/Styling/TabBarTypography.swift (1)
66-88: Consider replacing raw weight integers with named constants/comments.The mapping is fine, but documenting the NSFontManager weight scale (or using local constants) would make this safer to maintain.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Bonsplit/Internal/Styling/TabBarTypography.swift` around lines 66 - 88, The switch in private static func fontManagerWeight(for weight: NSFont.Weight) currently returns raw integers (e.g., 2,3,4...) which are unclear and fragile; replace those magic numbers with named constants or an internal enum (e.g., FONT_WEIGHT_ULTRA_LIGHT, FONT_WEIGHT_THIN, etc.) or add an inline comment referencing the NSFontManager weight scale to document each mapping, then update the return values to use those constants (references: fontManagerWeight and the NSFont.Weight cases .ultraLight, .thin, .light, .regular, .medium, .semibold, .bold, .heavy, .black).Sources/Bonsplit/Public/BonsplitConfiguration.swift (1)
192-194: Document non-finite scale behavior in the public API docs.Line 193 documents clamping for values below
0.5, but runtime logic also normalizes non-finite values. Adding that note here would reduce host-app surprises.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Bonsplit/Public/BonsplitConfiguration.swift` around lines 192 - 194, Update the public doc comment for the BonsplitConfiguration.tabTitleFontScale property to mention that values below 0.5 are clamped to 0.5 and that non-finite inputs (NaN, +inf, -inf) are normalized/handled at runtime (e.g., converted to a safe finite value or defaulted) to avoid surprises for host apps; locate the comment above the tabTitleFontScale declaration and add a concise sentence specifying the exact non‑finite behavior and resulting value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sources/Bonsplit/Internal/Styling/TabBarTypography.swift`:
- Around line 66-88: The switch in private static func fontManagerWeight(for
weight: NSFont.Weight) currently returns raw integers (e.g., 2,3,4...) which are
unclear and fragile; replace those magic numbers with named constants or an
internal enum (e.g., FONT_WEIGHT_ULTRA_LIGHT, FONT_WEIGHT_THIN, etc.) or add an
inline comment referencing the NSFontManager weight scale to document each
mapping, then update the return values to use those constants (references:
fontManagerWeight and the NSFont.Weight cases .ultraLight, .thin, .light,
.regular, .medium, .semibold, .bold, .heavy, .black).
In `@Sources/Bonsplit/Public/BonsplitConfiguration.swift`:
- Around line 192-194: Update the public doc comment for the
BonsplitConfiguration.tabTitleFontScale property to mention that values below
0.5 are clamped to 0.5 and that non-finite inputs (NaN, +inf, -inf) are
normalized/handled at runtime (e.g., converted to a safe finite value or
defaulted) to avoid surprises for host apps; locate the comment above the
tabTitleFontScale declaration and add a concise sentence specifying the exact
non‑finite behavior and resulting value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3acabe68-9a5f-4a56-9bf4-24da3038e343
📒 Files selected for processing (3)
Sources/Bonsplit/Internal/Styling/TabBarTypography.swiftSources/Bonsplit/Public/BonsplitConfiguration.swiftTests/BonsplitTests/BonsplitTests.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- Tests/BonsplitTests/BonsplitTests.swift
1 participant
Add configurable tab title typography
Summary
Add configurable pane tab title typography to Bonsplit so host apps can override tab title family and scale without changing the default system font behavior.
Why
Host apps need a supported way to style pane tab titles to match their UI typography. Bonsplit previously hardcoded the system font for tab titles and drag previews, which made host-level font customization impossible without patching internal views.
Changes
tabTitleFontFamilyandtabTitleFontScaletoBonsplitConfiguration.AppearanceTabBarTypographyhelper instead of hardcoded system font callsValidation
swift testSummary by cubic
Adds configurable typography for pane tab titles so host apps can set a font family and scale while keeping the system font as the default. Updates tab titles and drag previews to use a shared resolver and hardens scale handling.
BonsplitConfiguration.AppearanceaddstabTitleFontFamilyandtabTitleFontScale.TabBarTypographyresolves fonts viaAppKit, enforces the requested family, clamps scale to 0.5+, and falls back to the system font when missing or non-finite.TabBarTypography.Written for commit 0d0ce6e. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests