Add configurable UI Scale Factor setting#1387
Open
DreaminDani wants to merge 3 commits intomanaflow-ai:mainfrom
Open
Add configurable UI Scale Factor setting#1387DreaminDani wants to merge 3 commits intomanaflow-ai:mainfrom
DreaminDani wants to merge 3 commits intomanaflow-ai:mainfrom
Conversation
|
Someone is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds an app-wide UI scaling system: new Changes
sequenceDiagram
participant User as User
participant SettingsView as Settings View
participant AppStorage as UserDefaults / AppStorage
participant Env as SwiftUI Environment (.uiScale)
participant Views as App Views (ContentView, Panels)
User->>SettingsView: adjust UI scale (stepper/text/reset)
SettingsView->>AppStorage: write uiScaleFactor (clamped)
AppStorage-->>Env: publish uiScale change
Env->>Views: environment .uiScale updated
Views->>Views: re-render using .scaledFont (size * uiScale)
Views->>AppStorage: (non‑SwiftUI measurement via UIScaleSettings.scaled)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 |
2 tasks
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 14 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/cmuxApp.swift">
<violation number="1" location="Sources/cmuxApp.swift:4766">
P3: Use `.scaledFont` for the reset icon so it scales with the user’s UI scale factor.</violation>
</file>
<file name="Sources/ContentView.swift">
<violation number="1" location="Sources/ContentView.swift:9510">
P2: Shortcut hint width cache ignores UI scale, so widths become stale after changing the scale factor.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| value = UIScaleSettings.defaultValue | ||
| } label: { | ||
| Image(systemName: "arrow.counterclockwise") | ||
| .font(.system(size: 10, weight: .medium)) |
There was a problem hiding this comment.
P3: Use .scaledFont for the reset icon so it scales with the user’s UI scale factor.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/cmuxApp.swift, line 4766:
<comment>Use `.scaledFont` for the reset icon so it scales with the user’s UI scale factor.</comment>
<file context>
@@ -4721,6 +4733,65 @@ extension SettingsPickerRow where ExtraTrailing == EmptyView {
+ value = UIScaleSettings.defaultValue
+ } label: {
+ Image(systemName: "arrow.counterclockwise")
+ .font(.system(size: 10, weight: .medium))
+ .foregroundColor(.secondary)
+ }
</file context>
Suggested change
| .font(.system(size: 10, weight: .medium)) | |
| .scaledFont(size: 10, weight: .medium) |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
Sources/Panels/BrowserPanelView.swift (2)
704-713:⚠️ Potential issue | 🟠 MajorSuggestion/theme rows can truncate at high UI scale
Scaled text is now used, but row heights are still hard-capped (
.frame(height: 24)andmaxHeight: rowHeight(...)). At larger UI scale this will clip text and badges.💡 Suggested fix (remove hard caps, keep minimums)
- .frame(height: 24) + .frame(minHeight: 24) - .frame( - maxWidth: .infinity, - minHeight: rowHeight(for: item), - maxHeight: rowHeight(for: item), - alignment: .leading - ) + .frame( + maxWidth: .infinity, + minHeight: rowHeight(for: item), + alignment: .leading + )Also applies to: 3581-3604
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Panels/BrowserPanelView.swift` around lines 704 - 713, The theme/ suggestion row is being clipped at larger UI scales because of hard-coded max heights (e.g., the explicit .frame(height: 24) and any usages of rowHeight(...) that cap maxHeight); change these to use flexible sizing with minimum constraints instead: remove fixed height caps and replace them with .frame(minHeight: <appropriate min>, alignment: .center) or rely on intrinsic sizing, and remove/maxHeight constraints in the rowHeight(...) consumer so the row can grow with scaled fonts while still keeping a reasonable minimum; update the same pattern at the other occurrence (lines referenced around 3581-3604) so both suggestion and theme rows behave the same.
598-639:⚠️ Potential issue | 🟠 MajorScaled icons can clip in fixed-size chrome controls
The icon fonts now scale, but several containers still use fixed sizes (22/26 button frames and 18 omnibar field height). At larger scale factors this can clip icons and crowd the address bar.
💡 Suggested fix (allow controls to grow with scaled content)
- Image(systemName: "chevron.left") - .scaledFont(size: 12, weight: .medium) - .frame(width: addressBarButtonHitSize, height: addressBarButtonHitSize, alignment: .center) + Image(systemName: "chevron.left") + .scaledFont(size: 12, weight: .medium) + .frame(minWidth: addressBarButtonHitSize, minHeight: addressBarButtonHitSize, alignment: .center) - .frame(width: addressBarButtonSize, height: addressBarButtonSize, alignment: .center) + .frame(minWidth: addressBarButtonSize, minHeight: addressBarButtonSize, alignment: .center) - ) - .frame(height: 18) + ) + .frame(minHeight: 18)Also applies to: 665-685, 737-795
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Panels/BrowserPanelView.swift` around lines 598 - 639, The scaled icon frames are fixed (using .frame(width: addressBarButtonHitSize, height: addressBarButtonHitSize) and a fixed omnibar field height) which causes clipping at larger font scales; update the button and field frames in BrowserPanelView so they can grow with scaled content by using .frame(minWidth: addressBarButtonHitSize, minHeight: addressBarButtonHitSize) (or .frame(minHeight: ...) for the omnibar field) instead of fixed width/height, and ensure any padding/contentShape/OmnibarAddressButtonStyle behavior still aligns; apply the same change to the other button blocks referenced (the forward and reload/stop buttons and the omnibar field definitions mentioned around the other ranges).Sources/ContentView.swift (1)
9510-9538:⚠️ Potential issue | 🟠 MajorShortcut width cache does not account for UI scale, causing stale measurements after scale changes.
SidebarWorkspaceShortcutHintMetrics.cachedHintWidthsis keyed only by label text, but measured width now depends onUIScaleSettings.scaled(10). After changing UI scale at runtime, cached widths can be wrong until cache reset/app restart.💡 Suggested fix (include effective font size in cache key)
enum SidebarWorkspaceShortcutHintMetrics { + private struct CacheKey: Hashable { + let label: String + let pointSize: CGFloat + } private static var measurementFont: NSFont { NSFont.systemFont(ofSize: UIScaleSettings.scaled(10), weight: .semibold) } private static let minimumSlotWidth: CGFloat = 28 private static let horizontalPadding: CGFloat = 12 private static let lock = NSLock() - private static var cachedHintWidths: [String: CGFloat] = [:] + private static var cachedHintWidths: [CacheKey: CGFloat] = [:] static func hintWidth(for label: String) -> CGFloat { + let pointSize = measurementFont.pointSize + let cacheKey = CacheKey(label: label, pointSize: pointSize) lock.lock() - if let cached = cachedHintWidths[label] { + if let cached = cachedHintWidths[cacheKey] { lock.unlock() return cached } lock.unlock() - let textWidth = (label as NSString).size(withAttributes: [.font: measurementFont]).width + let textWidth = (label as NSString).size( + withAttributes: [.font: NSFont.systemFont(ofSize: pointSize, weight: .semibold)] + ).width let measuredWidth = ceil(textWidth) + horizontalPadding lock.lock() - cachedHintWidths[label] = measuredWidth + cachedHintWidths[cacheKey] = measuredWidth `#if` DEBUG measurementCount += 1 `#endif` lock.unlock() return measuredWidth }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 9510 - 9538, The cachedHintWidths map currently keys only on the label string causing stale widths when UI scale changes; update the caching strategy in SidebarWorkspaceShortcutHintMetrics by including the effective font/scale in the cache key (for example concatenate label with UIScaleSettings.scaled(10) or use a tuple/struct key) and change both the lookup in hintWidth(for:) and the write to cachedHintWidths to use that combined key (or alternatively invalidate the cache on scale change), so measurements that depend on measurementFont (UIScaleSettings.scaled(10)) are recomputed correctly.
🤖 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/cmuxApp.swift`:
- Around line 4765-4767: The reset icon uses a fixed font size via
.font(.system(size: 10, weight: .medium)) which prevents it from following UI
scaling; replace that modifier on the Image(systemName:
"arrow.counterclockwise") with a dynamic text-style/font or imageScale (for
example use .font(.system(.caption, design: .default).weight(.medium)) or
.imageScale(.small)) so the glyph responds to system UI scale and Dynamic Type.
In `@Sources/ContentView.swift`:
- Around line 8962-8967: Hard-coded frame sizes (e.g., the Image(systemName:
"questionmark.circle") using .frame(width: buttonSize, height: buttonSize)) are
clipping scaled glyphs at high UI scale factors; update these views to compute
and apply scale-aware container sizes by multiplying the fixed dimensions
(buttonSize, iconSize, badgeSize, etc.) by the current
NSScreen/backingScaleFactor (or use GeometryReader to derive scale) so the
.frame(width: ..., height: ...) grows with the UI scale; apply the same change
to the similar occurrences around the Image(help) block and the other listed
spots (the unread badge and close button usages) so all scaledFont() glyphs get
matching container geometry.
---
Outside diff comments:
In `@Sources/ContentView.swift`:
- Around line 9510-9538: The cachedHintWidths map currently keys only on the
label string causing stale widths when UI scale changes; update the caching
strategy in SidebarWorkspaceShortcutHintMetrics by including the effective
font/scale in the cache key (for example concatenate label with
UIScaleSettings.scaled(10) or use a tuple/struct key) and change both the lookup
in hintWidth(for:) and the write to cachedHintWidths to use that combined key
(or alternatively invalidate the cache on scale change), so measurements that
depend on measurementFont (UIScaleSettings.scaled(10)) are recomputed correctly.
In `@Sources/Panels/BrowserPanelView.swift`:
- Around line 704-713: The theme/ suggestion row is being clipped at larger UI
scales because of hard-coded max heights (e.g., the explicit .frame(height: 24)
and any usages of rowHeight(...) that cap maxHeight); change these to use
flexible sizing with minimum constraints instead: remove fixed height caps and
replace them with .frame(minHeight: <appropriate min>, alignment: .center) or
rely on intrinsic sizing, and remove/maxHeight constraints in the rowHeight(...)
consumer so the row can grow with scaled fonts while still keeping a reasonable
minimum; update the same pattern at the other occurrence (lines referenced
around 3581-3604) so both suggestion and theme rows behave the same.
- Around line 598-639: The scaled icon frames are fixed (using .frame(width:
addressBarButtonHitSize, height: addressBarButtonHitSize) and a fixed omnibar
field height) which causes clipping at larger font scales; update the button and
field frames in BrowserPanelView so they can grow with scaled content by using
.frame(minWidth: addressBarButtonHitSize, minHeight: addressBarButtonHitSize)
(or .frame(minHeight: ...) for the omnibar field) instead of fixed width/height,
and ensure any padding/contentShape/OmnibarAddressButtonStyle behavior still
aligns; apply the same change to the other button blocks referenced (the forward
and reload/stop buttons and the omnibar field definitions mentioned around the
other ranges).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a61a0585-a01f-4ea7-9faa-48b7c37029ad
📒 Files selected for processing (14)
GhosttyTabs.xcodeproj/project.pbxprojResources/Localizable.xcstringsSources/ContentView.swiftSources/NotificationsPage.swiftSources/Panels/BrowserPanelView.swiftSources/Panels/MarkdownPanelView.swiftSources/UIScale.swiftSources/Update/UpdatePill.swiftSources/Update/UpdatePopoverView.swiftSources/Update/UpdateTitlebarAccessory.swiftSources/WindowToolbarController.swiftSources/WorkspaceContentView.swiftSources/cmuxApp.swiftvendor/bonsplit
Comment on lines
+4765
to
+4767
| Image(systemName: "arrow.counterclockwise") | ||
| .font(.system(size: 10, weight: .medium)) | ||
| .foregroundColor(.secondary) |
There was a problem hiding this comment.
Reset icon still uses a fixed font size (misses UI scale).
At Line 4766, the reset glyph uses .font(.system(size: 10, ...)), so it won’t follow the new UI scale behavior consistently.
Suggested fix
Button {
value = UIScaleSettings.defaultValue
} label: {
Image(systemName: "arrow.counterclockwise")
- .font(.system(size: 10, weight: .medium))
+ .scaledFont(size: 10, weight: .medium)
.foregroundColor(.secondary)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/cmuxApp.swift` around lines 4765 - 4767, The reset icon uses a fixed
font size via .font(.system(size: 10, weight: .medium)) which prevents it from
following UI scaling; replace that modifier on the Image(systemName:
"arrow.counterclockwise") with a dynamic text-style/font or imageScale (for
example use .font(.system(.caption, design: .default).weight(.medium)) or
.imageScale(.small)) so the glyph responds to system UI scale and Dynamic Type.
Comment on lines
8962
to
8967
| Image(systemName: "questionmark.circle") | ||
| .symbolRenderingMode(.monochrome) | ||
| .font(.system(size: iconSize, weight: .medium)) | ||
| .scaledFont(size: iconSize, weight: .medium) | ||
| .foregroundStyle(Color(nsColor: .secondaryLabelColor)) | ||
| .frame(width: buttonSize, height: buttonSize, alignment: .center) | ||
| } |
There was a problem hiding this comment.
Scaled glyphs are now clipped by fixed-size frames at higher UI scale factors.
Several newly scaled icons/text are still constrained by hard-coded frame sizes (e.g., 22pt help button, 16pt unread badge, 16pt close button). At larger scales (especially 2.0x–3.0x), this will truncate or clip content.
💡 Suggested fix (scale container geometry with the same UI scale factor)
- private let buttonSize: CGFloat = 22
- private let iconSize: CGFloat = 11
+ private let buttonBaseSize: CGFloat = 22
+ private let iconBaseSize: CGFloat = 11
+ private var buttonSize: CGFloat { UIScaleSettings.scaled(buttonBaseSize) }
+ private var iconSize: CGFloat { iconBaseSize }- if unreadCount > 0 {
+ if unreadCount > 0 {
+ let badgeSize = UIScaleSettings.scaled(16)
ZStack {
Circle()
.fill(activeUnreadBadgeFillColor)
Text("\(unreadCount)")
.scaledFont(size: 9, weight: .semibold)
.foregroundColor(.white)
}
- .frame(width: 16, height: 16)
+ .frame(width: badgeSize, height: badgeSize)
}Also applies to: 9779-9786, 9809-9816
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/ContentView.swift` around lines 8962 - 8967, Hard-coded frame sizes
(e.g., the Image(systemName: "questionmark.circle") using .frame(width:
buttonSize, height: buttonSize)) are clipping scaled glyphs at high UI scale
factors; update these views to compute and apply scale-aware container sizes by
multiplying the fixed dimensions (buttonSize, iconSize, badgeSize, etc.) by the
current NSScreen/backingScaleFactor (or use GeometryReader to derive scale) so
the .frame(width: ..., height: ...) grows with the UI scale; apply the same
change to the similar occurrences around the Image(help) block and the other
listed spots (the unread badge and close button usages) so all scaledFont()
glyphs get matching container geometry.
Author
|
The contributing guide does not specify. Is it a requirement to address these AI feedbacks before this is ready to merge? |
Adds a "UI Scale Factor" setting (Settings > App) that scales all UI fonts and icons by a user-defined multiplier. Users can step by 0.25 increments or type a custom value (range 0.5x–3.0x). Terminal text is unaffected (controlled by Ghostty config). - New UIScale.swift: environment key, scaledFont() view modifier, and AppKit helper for NSFont sites - Replace ~112 hardcoded .font(.system(size:)) calls with .scaledFont() across 8 source files - Replace ~5 NSFont.systemFont(ofSize:) calls with UIScaleSettings.scaled() - Bridge uiScale environment to bonsplit's bonsplitUIScale for tab bar scaling - Inject environment at titlebar controls for icon scaling - Add settings UI with stepper, text input, and reset button - Add localization keys (en + ja) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The validateDrop functions in SidebarTabDropDelegate, SidebarBonsplitTabDropDelegate, and SidebarExternalDropDelegate were using [String] type identifiers instead of [UTType] when calling info.hasItemsConforming(to:). This caused the drag-and-drop validation to fail on macOS. Additionally, NSItemProvider visibility was changed from .ownProcess to .all for macOS 26+ compatibility where stricter process-scoped drag session validation caused drag reorder to fail on fresh installs. Changes: - [SidebarTabDragPayload.typeIdentifier] -> SidebarTabDragPayload.dropContentTypes - [BonsplitTabDragPayload.typeIdentifier] -> BonsplitTabDragPayload.dropContentTypes - visibility: .ownProcess -> visibility: .all with explanatory comment Fixes manaflow-ai#1385
DreaminDani
force-pushed
the
feat/ui-scale-factor
branch
from
6121b59 to
164d231
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Sources/UIScale.swift (1)
25-27: Consider referencingUIScaleSettings.defaultValueto avoid duplication.The default value
1.0is defined both here and inUIScaleSettings.defaultValue. Using a single source of truth reduces the risk of divergence if the default ever changes.♻️ Suggested improvement
private struct UIScaleKey: EnvironmentKey { - static let defaultValue: CGFloat = 1.0 + static let defaultValue: CGFloat = UIScaleSettings.defaultValue }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/UIScale.swift` around lines 25 - 27, UIScaleKey currently hardcodes the default CGFloat 1.0 causing duplicated defaults; update UIScaleKey to use UIScaleSettings.defaultValue instead so there's a single source of truth—replace the literal in UIScaleKey.defaultValue with a reference to UIScaleSettings.defaultValue and ensure UIScaleSettings is accessible from this file (import or adjust visibility) so the environment key uses that shared constant.
🤖 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/UIScale.swift`:
- Around line 25-27: UIScaleKey currently hardcodes the default CGFloat 1.0
causing duplicated defaults; update UIScaleKey to use
UIScaleSettings.defaultValue instead so there's a single source of truth—replace
the literal in UIScaleKey.defaultValue with a reference to
UIScaleSettings.defaultValue and ensure UIScaleSettings is accessible from this
file (import or adjust visibility) so the environment key uses that shared
constant.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eb3560eb-afc3-4fd3-8394-7b7149bf8900
📒 Files selected for processing (14)
GhosttyTabs.xcodeproj/project.pbxprojResources/Localizable.xcstringsSources/ContentView.swiftSources/NotificationsPage.swiftSources/Panels/BrowserPanelView.swiftSources/Panels/MarkdownPanelView.swiftSources/UIScale.swiftSources/Update/UpdatePill.swiftSources/Update/UpdatePopoverView.swiftSources/Update/UpdateTitlebarAccessory.swiftSources/WindowToolbarController.swiftSources/WorkspaceContentView.swiftSources/cmuxApp.swiftvendor/bonsplit
✅ Files skipped from review due to trivial changes (8)
- vendor/bonsplit
- Sources/WindowToolbarController.swift
- Sources/Update/UpdatePopoverView.swift
- Sources/Panels/MarkdownPanelView.swift
- Sources/Update/UpdatePill.swift
- GhosttyTabs.xcodeproj/project.pbxproj
- Resources/Localizable.xcstrings
- Sources/Panels/BrowserPanelView.swift
🚧 Files skipped from review as they are similar to previous changes (3)
- Sources/NotificationsPage.swift
- Sources/Update/UpdateTitlebarAccessory.swift
- Sources/ContentView.swift
Remove onTabCloseRequest callback (replaced by shouldCloseTab delegate) and stub syncTrafficLightInset (tabBarLeadingInset removed from bonsplit). Update ghostty and homebrew-cmux submodule pointers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
This review could not be run because your cubic account has exceeded the monthly review limit. If you need help restoring access, please contact contact@cubic.dev. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Sources/ContentView.swift (2)
12294-12327:⚠️ Potential issue | 🟠 MajorInvalidate the shortcut-width cache when UI scale changes.
measurementFontnow depends onUIScaleSettings, butcachedHintWidthsis still keyed only bylabel. After a live scale change, this reuses widths measured at the old point size, so the shortcut pill can clip or leave stale trailing space until relaunch.Suggested fix
private enum SidebarWorkspaceShortcutHintMetrics { - private static var measurementFont: NSFont { NSFont.systemFont(ofSize: UIScaleSettings.scaled(10), weight: .semibold) } + private struct CacheKey: Hashable { + let label: String + let pointSize: CGFloat + } + + private static var measurementFont: NSFont { + NSFont.systemFont(ofSize: UIScaleSettings.scaled(10), weight: .semibold) + } private static let minimumSlotWidth: CGFloat = 28 private static let horizontalPadding: CGFloat = 12 private static let lock = NSLock() - private static var cachedHintWidths: [String: CGFloat] = [:] + private static var cachedHintWidths: [CacheKey: CGFloat] = [:] @@ static func hintWidth(for label: String) -> CGFloat { + let font = measurementFont + let cacheKey = CacheKey(label: label, pointSize: font.pointSize) lock.lock() - if let cached = cachedHintWidths[label] { + if let cached = cachedHintWidths[cacheKey] { lock.unlock() return cached } lock.unlock() - let textWidth = (label as NSString).size(withAttributes: [.font: measurementFont]).width + let textWidth = (label as NSString).size(withAttributes: [.font: font]).width let measuredWidth = ceil(textWidth) + horizontalPadding lock.lock() - cachedHintWidths[label] = measuredWidth + cachedHintWidths[cacheKey] = measuredWidth `#if` DEBUG measurementCount += 1 `#endif`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 12294 - 12327, The cachedHintWidths map is keyed only by label but measurementFont depends on UIScaleSettings.scaled(...), so on scale changes you must either include the scale in the cache key or clear the cache when scale changes; update hintWidth/slotWidth logic to incorporate the current scale (e.g., compute a scaleIdentifier from UIScaleSettings.scaled(10) and use "\(scaleIdentifier)|\(label)" as the cache key) or add a method that clears cachedHintWidths and call it whenever UIScaleSettings updates; ensure you continue to protect cache access with lock and update references to cachedHintWidths and hintWidth accordingly (symbols: measurementFont, cachedHintWidths, hintWidth, slotWidth, UIScaleSettings).
14602-14612:⚠️ Potential issue | 🟠 MajorGuard provider visibility behind macOS 26 to match dragConfiguration restrictions.
The
visibility: .allis unconditional, butdragConfiguration(which prevents external drag operations) is only applied on macOS 26+ withcompiler(>=6.2). On older builds, the drag type is exposed to Finder/external apps without the external-operation guardrails. Use.ownProcesson pre-macOS 26 builds wheredragConfigurationis unavailable.Suggested fix
static func provider(for tabId: UUID) -> NSItemProvider { let provider = NSItemProvider() let payload = "\(prefix)\(tabId.uuidString)" - // Use .all visibility for compatibility with macOS 26+ where .ownProcess - // can cause drag reorder to fail on fresh installs due to stricter - // process-scoped drag session validation. - provider.registerDataRepresentation(forTypeIdentifier: typeIdentifier, visibility: .all) { completion in + let visibility: NSItemProviderRepresentationVisibility = { + `#if` compiler(>=6.2) + if `#available`(macOS 26.0, *) { + return .all + } + `#endif` + return .ownProcess + }() + provider.registerDataRepresentation(forTypeIdentifier: typeIdentifier, visibility: visibility) { completion in completion(payload.data(using: .utf8), nil) return nil } return provider }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 14602 - 14612, The NSItemProvider in provider(for:) currently always uses visibility: .all; change this to use .all only when building/running on macOS 26+ with the newer compiler support and fall back to .ownProcess on older builds. Concretely, around the provider.registerDataRepresentation call determine the visibility value via a conditional compilation/runtime check (e.g., `#if` compiler(>=6.2) plus a macOS availability check for macOS 26 at runtime) and pass that visibility variable instead of hardcoding .all; refer to the provider(for:) function and the typeIdentifier constant to locate and update the registration call.
🧹 Nitpick comments (2)
Sources/ContentView.swift (2)
11289-11336: The feedback sheet’s main editor still ignores UI scale.The surrounding labels now scale, but
FeedbackComposerMessageEditorViewstill hard-codes a 12pt editor/placeholder font. That leaves the dominant input in this sheet visually out of sync with the rest of the dialog.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 11289 - 11336, FeedbackComposerMessageEditorView is still using a hard-coded 12pt font for the editor and placeholder, so update the view used by FeedbackComposerMessageEditor to use the app’s dynamic/scaled font API instead of fixed 12pt; locate FeedbackComposerMessageEditorView (and any placeholder text rendering inside it) and replace the fixed Font.system(size: 12) / hard-coded 12pt usages with the same scaledFont(...) modifier or a dynamic TextField/TextEditor font (e.g., scaledFont(size: …, weight: …) or .font(.body) with .dynamicTypeSize support) so the editor text and placeholder respond to UI scale just like the surrounding labels.
3961-3962: Scale the whole command-palette surface, not just the SwiftUI text.These changes move the empty state and single-line field onto the new scaling path, but the main palette still mixes in fixed-size typography from
CommandPaletteSearchFieldRepresentable,commandPaletteResultLabelContent(), andCommandPaletteMultilineTextEditorView. The result will still be a mixed-scale command palette after the user changes UI scale.Also applies to: 4108-4147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 3961 - 3962, The command-palette is still mixing scaled SwiftUI Text with fixed-size content from CommandPaletteSearchFieldRepresentable, commandPaletteResultLabelContent(), and CommandPaletteMultilineTextEditorView; move the scaling from individual Texts to the parent surface so everything scales uniformly: wrap the entire palette root view (the container that composes the search field, results list, empty state, and multiline editor) with the scaling modifier you added (e.g., apply the scaledFont/scaleEffect or a unified font environment) and remove per-Text-only scaling, and update the representable components (CommandPaletteSearchFieldRepresentable and CommandPaletteMultilineTextEditorView) to respect the forwarded font/scale (expose and apply the passed UIFont/NSFont or adopt SwiftUI font environment) so commandPaletteResultLabelContent() and the representables all render at the same scale.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ghostty`:
- Line 1: The submodule update points to commit 92cc550 which is not on the
manaflow-ai/ghostty fork's main branch; enter the ghostty submodule, push the
commit 92cc550 to the fork's origin/main branch, then return to the parent repo
and update the submodule pointer (stage the ghostty submodule change and commit)
so the parent repo references the pushed commit; ensure docs/ghostty-fork.md
remains as updated and push the parent repo commit to its remote.
---
Outside diff comments:
In `@Sources/ContentView.swift`:
- Around line 12294-12327: The cachedHintWidths map is keyed only by label but
measurementFont depends on UIScaleSettings.scaled(...), so on scale changes you
must either include the scale in the cache key or clear the cache when scale
changes; update hintWidth/slotWidth logic to incorporate the current scale
(e.g., compute a scaleIdentifier from UIScaleSettings.scaled(10) and use
"\(scaleIdentifier)|\(label)" as the cache key) or add a method that clears
cachedHintWidths and call it whenever UIScaleSettings updates; ensure you
continue to protect cache access with lock and update references to
cachedHintWidths and hintWidth accordingly (symbols: measurementFont,
cachedHintWidths, hintWidth, slotWidth, UIScaleSettings).
- Around line 14602-14612: The NSItemProvider in provider(for:) currently always
uses visibility: .all; change this to use .all only when building/running on
macOS 26+ with the newer compiler support and fall back to .ownProcess on older
builds. Concretely, around the provider.registerDataRepresentation call
determine the visibility value via a conditional compilation/runtime check
(e.g., `#if` compiler(>=6.2) plus a macOS availability check for macOS 26 at
runtime) and pass that visibility variable instead of hardcoding .all; refer to
the provider(for:) function and the typeIdentifier constant to locate and update
the registration call.
---
Nitpick comments:
In `@Sources/ContentView.swift`:
- Around line 11289-11336: FeedbackComposerMessageEditorView is still using a
hard-coded 12pt font for the editor and placeholder, so update the view used by
FeedbackComposerMessageEditor to use the app’s dynamic/scaled font API instead
of fixed 12pt; locate FeedbackComposerMessageEditorView (and any placeholder
text rendering inside it) and replace the fixed Font.system(size: 12) /
hard-coded 12pt usages with the same scaledFont(...) modifier or a dynamic
TextField/TextEditor font (e.g., scaledFont(size: …, weight: …) or .font(.body)
with .dynamicTypeSize support) so the editor text and placeholder respond to UI
scale just like the surrounding labels.
- Around line 3961-3962: The command-palette is still mixing scaled SwiftUI Text
with fixed-size content from CommandPaletteSearchFieldRepresentable,
commandPaletteResultLabelContent(), and CommandPaletteMultilineTextEditorView;
move the scaling from individual Texts to the parent surface so everything
scales uniformly: wrap the entire palette root view (the container that composes
the search field, results list, empty state, and multiline editor) with the
scaling modifier you added (e.g., apply the scaledFont/scaleEffect or a unified
font environment) and remove per-Text-only scaling, and update the representable
components (CommandPaletteSearchFieldRepresentable and
CommandPaletteMultilineTextEditorView) to respect the forwarded font/scale
(expose and apply the passed UIFont/NSFont or adopt SwiftUI font environment) so
commandPaletteResultLabelContent() and the representables all render at the same
scale.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4cfecc21-2738-464e-8544-4b55c939d49c
📒 Files selected for processing (4)
Sources/ContentView.swiftSources/Workspace.swiftghosttyhomebrew-cmux
💤 Files with no reviewable changes (1)
- Sources/Workspace.swift
✅ Files skipped from review due to trivial changes (1)
- homebrew-cmux
| @@ -1 +1 @@ | |||
| Subproject commit 9fa3ab01bb67d5cec4daa358e25509a271af8171 | |||
| Subproject commit 404a3f175ba6baafabc46cac807194883e040980 | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the ghostty submodule commit exists on the remote and check fork documentation
cd ghostty
# Check if the commit is an ancestor of origin/main
git fetch origin main --quiet 2>/dev/null
if git merge-base --is-ancestor HEAD origin/main 2>/dev/null; then
echo "✅ Submodule commit is on origin/main"
else
echo "⚠️ Submodule commit may not be pushed to origin/main yet"
fi
# Show the current commit for reference
echo ""
echo "Current submodule commit:"
git log --oneline -1
cd ..
# Check if fork documentation exists and when it was last updated
echo ""
echo "Fork documentation status:"
if [ -f "docs/ghostty-fork.md" ]; then
echo "docs/ghostty-fork.md exists"
echo "Last modified in this branch:"
git log -1 --format="%h %s" -- docs/ghostty-fork.md 2>/dev/null || echo "(no changes in this branch)"
else
echo "docs/ghostty-fork.md not found"
fiRepository: manaflow-ai/cmux
Length of output: 347
Push the submodule commit to the fork's main branch before merging this PR.
The ghostty commit 92cc550 is not yet on the manaflow-ai/ghostty fork's main branch. Per project conventions, submodule commits must be pushed to the remote before updating the pointer in the parent repo. Push the commit to origin/main in the ghostty submodule, then update the parent repo pointer.
On the positive side, docs/ghostty-fork.md was appropriately updated alongside this change.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ghostty` at line 1, The submodule update points to commit 92cc550 which is
not on the manaflow-ai/ghostty fork's main branch; enter the ghostty submodule,
push the commit 92cc550 to the fork's origin/main branch, then return to the
parent repo and update the submodule pointer (stage the ghostty submodule change
and commit) so the parent repo references the pushed commit; ensure
docs/ghostty-fork.md remains as updated and push the parent repo commit to its
remote.
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
2 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.
I have low vision and have a really hard time reading and using the UI elements outside of the terminal. Hopefully this approach to the code is reasonable. I tried to keep the impact low while still being able to affect all the relevant elements that were difficult to see.
Summary
.font(.system(size:))calls with a.scaledFont()view modifier that reads from a SwiftUI environment valuebonsplitUIScaleenvironment key, so tab titles, icons, and split buttons also scaleFiles changed
Sources/UIScale.swift— environment key,scaledFont()modifier,UIScaleSettingsmodel, AppKit helpervendor/bonsplit/.../UIScaleEnvironment.swift— publicbonsplitUIScaleenvironment key.font(.system(size:))→.scaledFont()replacementcmuxApp.swift— settings UI (stepper + text field + reset), environment injection, reset supportLocalizable.xcstrings— 3 new localization keys (en + ja)Bonsplit submodule
Depends on: manaflow-ai/bonsplit#24
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation