Add zoom scale support to tab bar metrics and views#21
Add zoom scale support to tab bar metrics and views#21moyashin63 wants to merge 1 commit intomanaflow-ai:mainfrom
Conversation
Scale all tab bar dimensions (height, width, font size, icon size, padding, indicators) via bonsplitZoomScale environment value. Co-Authored-By: Claude Opus 4.6 <[email protected]>
📝 WalkthroughWalkthroughThe changes introduce scale-aware UI metrics for the tab bar by converting fixed sizing constants to scale-dependent functions and adding a new SwiftUI environment value that propagates zoom scaling across tab bar components. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
Comment |
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
Greptile SummaryThis PR introduces a SwiftUI environment key ( Key changes:
Confidence Score: 4/5
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Host App] -->|".environment bonsplitZoomScale scale"| B[SwiftUI Environment]
B -->|"@Environment bonsplitZoomScale"| C[TabBarView]
B -->|"@Environment bonsplitZoomScale"| D[TabItemView]
B -->|"@Environment bonsplitZoomScale"| E[TabDragPreview]
C -->|zoomScale| F[TabBarMetrics scale funcs]
D -->|zoomScale| F
E -->|zoomScale| F
F --> G["barHeight · tabHeight · iconSize<br/>titleFontSize · closeButtonSize<br/>contentSpacing · dropIndicatorWidth<br/>dropIndicatorHeight · etc"]
style A fill:#4A90D9,color:#fff
style B fill:#7B68EE,color:#fff
style F fill:#2ECC71,color:#fff
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Sources/Bonsplit/Internal/Styling/TabBarMetrics.swift (1)
7-33: Consider finishing the zoom-metric centralization here.This file now owns most scale-aware sizing, but there are still raw
* zoomScaleliterals inSources/Bonsplit/Internal/Views/TabDragPreview.swiftat Lines 23-24,Sources/Bonsplit/Internal/Views/TabItemView.swiftat Lines 137, 230, 247-248, andSources/Bonsplit/Internal/Views/TabBarView.swiftat Lines 414, 447-488, and 494-495. Pulling those intoTabBarMetricswould keep future zoom tuning in one place and reduce visual drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Bonsplit/Internal/Styling/TabBarMetrics.swift` around lines 7 - 33, TabBarMetrics centralizes scale-aware sizing, but several raw "* zoomScale" literals remain in TabDragPreview, TabItemView, and TabBarView; add new static metric helpers in TabBarMetrics (following existing pattern like iconSize(_:), titleFontSize(_:), tabHorizontalPadding(_:), activeIndicatorHeight(_:)) for each literal used in those files (e.g., drag preview offsets/sizes, extra padding/margins, hover/active indicator sizes, and any font or spacing values referenced at the listed lines) and then replace the raw "* zoomScale" expressions in TabDragPreview, TabItemView, and TabBarView with calls to the new TabBarMetrics methods passing the current scale. Ensure names clearly map to their use (e.g., dragPreviewWidth(_:), tabItemExtraPadding(_:), barDropSpacing(_:)) and keep the API consistent (static funcs taking scale: CGFloat returning CGFloat).
🤖 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/Public/ZoomScale.swift`:
- Around line 8-10: The public setter for bonsplitZoomScale must validate and
normalize incoming values before storing them in BonsplitZoomScaleKey.self:
ensure the value is finite and positive, then clamp it to a safe range (e.g.,
min 0.5, max 3.0) and store that clamped value; if the input is non-finite or <=
0, replace it with the minimum sane value. Update the setter for
bonsplitZoomScale to perform this check/clamp so TabBarView, TabItemView, and
TabDragPreview never receive 0, negative, or infinite scales.
---
Nitpick comments:
In `@Sources/Bonsplit/Internal/Styling/TabBarMetrics.swift`:
- Around line 7-33: TabBarMetrics centralizes scale-aware sizing, but several
raw "* zoomScale" literals remain in TabDragPreview, TabItemView, and
TabBarView; add new static metric helpers in TabBarMetrics (following existing
pattern like iconSize(_:), titleFontSize(_:), tabHorizontalPadding(_:),
activeIndicatorHeight(_:)) for each literal used in those files (e.g., drag
preview offsets/sizes, extra padding/margins, hover/active indicator sizes, and
any font or spacing values referenced at the listed lines) and then replace the
raw "* zoomScale" expressions in TabDragPreview, TabItemView, and TabBarView
with calls to the new TabBarMetrics methods passing the current scale. Ensure
names clearly map to their use (e.g., dragPreviewWidth(_:),
tabItemExtraPadding(_:), barDropSpacing(_:)) and keep the API consistent (static
funcs taking scale: CGFloat returning CGFloat).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a826bd88-91c1-4c5d-90c8-4a02b346b215
📒 Files selected for processing (5)
Sources/Bonsplit/Internal/Styling/TabBarMetrics.swiftSources/Bonsplit/Internal/Views/TabBarView.swiftSources/Bonsplit/Internal/Views/TabDragPreview.swiftSources/Bonsplit/Internal/Views/TabItemView.swiftSources/Bonsplit/Public/ZoomScale.swift
| public var bonsplitZoomScale: CGFloat { | ||
| get { self[BonsplitZoomScaleKey.self] } | ||
| set { self[BonsplitZoomScaleKey.self] = newValue } |
There was a problem hiding this comment.
Clamp the public zoom scale before storing it.
Every tab metric now depends on this value, so 0, negative, or non-finite input will flow straight into widths, heights, and font sizes in TabBarView, TabItemView, and TabDragPreview. Normalizing it here prevents one bad host-provided value from breaking the entire tab strip.
Proposed fix
extension EnvironmentValues {
public var bonsplitZoomScale: CGFloat {
get { self[BonsplitZoomScaleKey.self] }
- set { self[BonsplitZoomScaleKey.self] = newValue }
+ set {
+ let normalized = newValue.isFinite ? max(0.5, newValue) : 1.0
+ self[BonsplitZoomScaleKey.self] = normalized
+ }
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public var bonsplitZoomScale: CGFloat { | |
| get { self[BonsplitZoomScaleKey.self] } | |
| set { self[BonsplitZoomScaleKey.self] = newValue } | |
| public var bonsplitZoomScale: CGFloat { | |
| get { self[BonsplitZoomScaleKey.self] } | |
| set { | |
| let normalized = newValue.isFinite ? max(0.5, newValue) : 1.0 | |
| self[BonsplitZoomScaleKey.self] = normalized | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/Bonsplit/Public/ZoomScale.swift` around lines 8 - 10, The public
setter for bonsplitZoomScale must validate and normalize incoming values before
storing them in BonsplitZoomScaleKey.self: ensure the value is finite and
positive, then clamp it to a safe range (e.g., min 0.5, max 3.0) and store that
clamped value; if the input is non-finite or <= 0, replace it with the minimum
sane value. Update the setter for bonsplitZoomScale to perform this check/clamp
so TabBarView, TabItemView, and TabDragPreview never receive 0, negative, or
infinite scales.
Summary
bonsplitZoomScaleenvironment key for host apps to inject a zoom scale factorTabBarMetricsconstants tostatic func(scale:)so tab bar dimensions scale dynamicallyTabBarView,TabItemView, andTabDragPreviewWhy
Host apps (e.g. cmux) implement UI-wide zoom scaling. Bonsplit tab bars need to participate in that scaling by reading a host-provided scale factor via SwiftUI environment.
Testing
swift build(package compiles)./scripts/reload.sh --tag ui-zoomRelated
ui-zoomSummary by cubic
Add zoom scaling to tab bars via a new SwiftUI environment value so all tab dimensions, fonts, and icons scale consistently with host app UI zoom.
New Features
bonsplitZoomScaleenvironment key (default 1.0).TabBarMetricsfrom constants tostatic func(scale:)to compute scaled values.TabBarView,TabItemView, andTabDragPreview(heights, widths, fonts, icons, padding, indicators).Migration
.environment(\.bonsplitZoomScale, <scale>)at your app’s root view.Written for commit 3c45519. Summary will update on new commits.
Summary by CodeRabbit