[cmux] add global font magnification across terminals and chrome - #3145
[cmux] add global font magnification across terminals and chrome#3145the-gigi wants to merge 5 commits into
Conversation
|
@the-gigi is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
To use Codex here, create a Codex account and connect to github. |
📝 WalkthroughWalkthroughAdds a global font magnification feature (percent-based, 50–200%) with Ghostty/SwiftUI wiring: introduces cmuxFont view modifiers and GlobalFontMagnification storage/notifications, injects scaled terminal font-size into Ghostty configs, migrates many ChangesLocalization
Core Config & Runtime
App Integration & Observers
Settings UI
UI Font Migration
Schema
Sequence DiagramsequenceDiagram
participant User as "User"
participant Settings as "Settings View"
participant Storage as "UserDefaults"
participant Notification as "NotificationCenter"
participant AppDelegate as "AppDelegate"
participant GhosttyConfig as "GhosttyConfig"
participant Terminal as "Terminal View"
User->>Settings: change global font percent (stepper/reset)
Settings->>Storage: write percent to GlobalFontMagnification.percentKey
Storage->>Notification: post GlobalFontMagnification.didChangeNotification
Notification->>AppDelegate: observer receives notification
AppDelegate->>GhosttyConfig: invalidate load cache
AppDelegate->>GhosttyConfig: reloadConfiguration(source: "globalFontMagnificationDidChange", reloadSettingsFromFile: false)
GhosttyConfig->>GhosttyConfig: loadFromDisk() -> applyGlobalMagnificationIfNeeded()
GhosttyConfig->>Terminal: return config with scaled font-size
Terminal->>Terminal: reflow/redraw with new sizes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 1 warning, 1 inconclusive)
✅ Passed checks (14 passed)
✨ 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 introduces a global font magnification setting (50–200%, default 100%, 10% steps) that scales fonts across terminals, tab bar, sidebar, and all SwiftUI chrome. The implementation uses a two-track approach: a
Confidence Score: 5/5Safe to merge — the architecture is sound, the dual-track scaling (Swift-side CmuxFontModifier + Ghostty C-side font-size injection) is correctly separated, and the notification/reload ownership is clearly assigned to AppDelegate with no feedback loops. The change is a well-scoped UI feature that adds a new setting, migrates all font call-sites to a magnification-aware modifier, and wires a single reload path through AppDelegate. The dual-track scaling approaches for Swift chrome and Ghostty terminal rendering operate on independent data (GhosttyConfig Swift struct vs. ghostty_config_t), so there is no double-scaling risk. The only gap found is the JSON schema not constraining multipleOf: 10, which is cosmetic — non-step values pass through silently but cause no crash or data loss. No files require special attention. The JSON schema (web/data/cmux-settings.schema.json) has a minor validation gap for step enforcement, and Sources/KeyboardShortcutSettingsFileStore.swift parses the integer setting via jsonDouble (previously flagged), but neither affects runtime correctness. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant SettingsView
participant AppStorage
participant NotifCenter as NotificationCenter
participant AppDelegate
participant GhosttyConfig
participant GhosttyApp
User->>SettingsView: Adjusts stepper / clicks Reset
SettingsView->>AppStorage: write globalFontMagnificationPercent
SettingsView->>NotifCenter: post(didChangeNotification)
Note over SettingsView: CmuxFontModifier re-renders via @AppStorage
NotifCenter->>AppDelegate: installGlobalFontSizeObserver fires
AppDelegate->>GhosttyConfig: invalidateLoadCache()
AppDelegate->>GhosttyApp: reloadConfiguration()
GhosttyApp->>GhosttyApp: applyGlobalFontMagnificationOverride(to: config)
Note over GhosttyApp: ghostty_config_get font-size → scale → loadInlineGhosttyConfig
GhosttyApp->>GhosttyApp: ghostty_config_finalize(config)
Note over GhosttyConfig: Swift-side: load() → applyGlobalMagnificationIfNeeded()
Note over GhosttyConfig: Scales fontSize and surfaceTabBarFontSize for SwiftUI use
Reviews (2): Last reviewed commit: "[cmux] scale magnification control text ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
Sources/RightSidebarPanelView.swift (1)
65-65:⚠️ Potential issue | 🟡 MinorMode bar has a fixed 31pt height while its contents scale up to 4×.
modeBaris pinned to.frame(height: 31)(line 65), but with.cmuxFont(size: 11, ...)on the icon and label (lines 97, 99), the text/icon scale to ~44pt at the 400% magnification ceiling and will clip against the container. Either drop the fixed height (let the HStack size itself) or scale the frame height by the sameGlobalFontMagnificationfactor that drives the font.This applies wherever fixed-height containers wrap newly-scaled
.cmuxFont(...)content in this PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/RightSidebarPanelView.swift` at line 65, The mode bar HStack currently has a hard-coded .frame(height: 31) which causes clipping when its child views use .cmuxFont(...) and scale with GlobalFontMagnification; remove the fixed height so the HStack can size itself OR multiply the fixed value by the same GlobalFontMagnification used by .cmuxFont (e.g., replace 31 with 31 * GlobalFontMagnification) where the modeBar is configured, and apply the same change to any other fixed-height containers wrapping scaled .cmuxFont content in this file (look for modeBar and other HStacks using .frame(height: ...)).Sources/Panels/BrowserPanelView.swift (1)
1160-1224:⚠️ Potential issue | 🟠 MajorOmnibar typed text still appears to bypass global magnification.
The surrounding omnibar chrome was migrated to
.cmuxFont, but the editable text is rendered byOmnibarTextFieldRepresentablewith a fixed AppKit font (Line 3958:.systemFont(ofSize: 12)). This creates inconsistent scaling in the same control.Please wire the AppKit text field font to the same magnification source and refresh it in
updateNSViewwhen magnification changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Panels/BrowserPanelView.swift` around lines 1160 - 1224, The AppKit text field inside OmnibarTextFieldRepresentable is using a hardcoded .systemFont(ofSize: 12) which bypasses the global magnification; update the representable so its NSTextField/NSTextView font is driven by the same magnification/font source used by .cmuxFont (or a shared FontProvider) and refresh that font in the representable's updateNSView method when magnification changes; specifically, replace the fixed .systemFont(ofSize: 12) initialization in OmnibarTextFieldRepresentable with a lookup to the shared magnification-aware font (or a passed-in Font/scale) and ensure updateNSView reapplies that font whenever the magnification or relevant binding changes so inline editable text scales consistently with the omnibar chrome.Sources/Update/UpdatePopoverView.swift (1)
60-90:⚠️ Potential issue | 🟡 MinorFixed
labelWidth = 60will truncate localized labels at higher magnification.The label column width is hard-coded at 60pt in
UpdateMetadataView(passed aslabelWidthparameter toDetectedBackgroundUpdateViewandUpdateAvailableView), and also inline inDetectedBackgroundUpdatePendingView. The row text is sized via.cmuxFont(size: 11), which scales with global font magnification (100–400%). At 400% magnification, the font becomes 44pt; the localized labels—especially "Released:" and non-English translations like "バージョン:" or "发布日期："—will exceed 60pt and truncate with…, creating misaligned columns.Consider deriving
labelWidthfrom the magnification factor (viaGlobalFontMagnification.scale) or using.fixedSize(horizontal: true, vertical: false)to allow labels to reflow naturally. Apply the same fix to the inlineframe(width: 60)inDetectedBackgroundUpdatePendingView.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Update/UpdatePopoverView.swift` around lines 60 - 90, The hard-coded 60pt label column truncates localized labels at high magnification; update the callers that set labelWidth (in UpdateMetadataView, DetectedBackgroundUpdateView, UpdateAvailableView) to compute it from the global magnification (use GlobalFontMagnification.scale to multiply a base width) or remove the fixed width and apply .fixedSize(horizontal: true, vertical: false) to the label Text views instead; also replace the inline .frame(width: 60) in DetectedBackgroundUpdatePendingView with the same computed width or .fixedSize so all uses of labelWidth (and the inline frame) expand correctly with .cmuxFont scaling.Sources/Update/UpdateTitlebarAccessory.swift (1)
378-393:⚠️ Potential issue | 🟠 MajorTitlebar icon/badge use magnified
.cmuxFont(...)but their frames remain at hard-codedconfig.buttonSize/config.badgeSize.
config.buttonSize,config.badgeSize,config.spacing, etc. are static per-style constants; nothing inTitlebarControlsStyleConfigreads the global magnification. With these changes:
- Line 528: the SF Symbol glyph size is now set by
cmuxFont(size: config.iconSize, ...)— at e.g. 200% it becomes ~30pt — but it's painted into.frame(width: config.buttonSize, height: config.buttonSize)(24pt for.classic), so the icon is clipped/cropped against the button frame. The same scaling mismatch applies to the surroundingtitlebarHintWidth(line 484), which still measures using a non-magnifiedNSFont.systemFont(...), so shortcut hint pills will visually drift from the rendered icons.- Line 383: the unread-count text is magnified via
cmuxFont, but it's clipped into a fixedCircle()ofconfig.badgeSize(12–16pt). At higher magnification "99" will overflow the circle.Verify at 400% magnification that: (1) SF Symbol icons render fully (not clipped), (2) the unread badge stays readable / non-overflowing, and (3) shortcut-hint pill positions stay aligned with the actual icon glyphs.
If
cmuxFontis intended only to scale text inside layouts that are themselves magnified, the cleanest fix is to also derivebuttonSize/badgeSize/spacing(and theNSFontused intitlebarHintWidth) from the same magnification factor, or to leave SF Symbol icon labels on plain.font(...)and only magnify text content.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Update/UpdateTitlebarAccessory.swift` around lines 378 - 393, The icon / badge are being visually scaled via cmuxFont(...) but their containing geometry (TitlebarControlsStyleConfig properties like buttonSize, badgeSize, spacing and the titlebarHintWidth measurement that uses NSFont.systemFont(...)) remains static, causing clipping and misalignment; fix by either (A) making TitlebarControlsStyleConfig compute buttonSize/badgeSize/spacing (and the NSFont used in titlebarHintWidth) from the same magnification factor used by cmuxFont so the frames grow with the glyphs, or (B) stop applying cmuxFont to SF Symbol iconLabel and shortcut hint measurement and instead use unscaled .font(...) / NSFont for layout while keeping cmuxFont only for inner text; update iconLabel usage, the ZStack badge sizing (config.badgeSize and Circle) and the titlebarHintWidth font measurement to use the chosen consistent approach so icons render fully, badges don't overflow, and hint pills stay aligned.
🧹 Nitpick comments (7)
Sources/Settings/ConfigSettingsView.swift (1)
77-77: Editor body font is left out of the global magnification.The path label (Line 77), status caption (Line 111), and banner footnote (Line 283) now scale with
GlobalFontMagnification, but the actualConfigSettingsTextView'sNSTextView.fontis hardcoded at.monospacedSystemFont(ofSize: 12, weight: .regular)(Line 318) and won't follow the magnification setting. For users who enable magnification primarily for readability, the chrome around the editor will grow while the editor contents stay tiny — exactly the inverse of what they want.Consider applying the same scaling factor (
GlobalFontMagnification.storedPercent) to theNSTextView's font size, and observingGlobalFontMagnification.didChangeNotificationto refresh it on changes.Also applies to: 283-283
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Settings/ConfigSettingsView.swift` at line 77, The editor's NSTextView font is hardcoded in ConfigSettingsTextView so it doesn't follow GlobalFontMagnification; update the font-setting logic in ConfigSettingsTextView to multiply the base monospaced font size (currently 12) by GlobalFontMagnification.storedPercent (or an appropriate scaled value) and set NSTextView.font accordingly, and add an observer for GlobalFontMagnification.didChangeNotification to recompute and apply the scaled font when magnification changes; ensure you reference and update the same place where .monospacedSystemFont(ofSize: 12, weight: .regular) is set so the editor body scales in sync with the path label, status caption, and banner footnote.Sources/Panels/MarkdownPanelView.swift (1)
74-100: LGTM — markdown header and unavailable-state fonts now magnification-aware.Header icon (
size: 12) and monospaced path (size: 11, design: .monospaced), plus the file-unavailable icon (size: 40), title (.headline), monospaced path (size: 12, design: .monospaced), and message (.caption) all preserve their prior visual styling while responding to the new global font magnification.Note (non-blocking): The Markdown body itself is rendered through
Markdown(...).markdownTheme(cmuxMarkdownTheme)with hard-codedFontSize(...)values (Lines 121, 129, 141, 152, 161, 170, 179, 190, 204, 219). These won't follow the global magnification, so users who enable magnification for accessibility will see a scaled file-path header but unscaled markdown content. Consider routing thoseFontSizevalues through the same scaling factor in a follow-up so the markdown body stays consistent with the rest of the chrome.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Panels/MarkdownPanelView.swift` around lines 74 - 100, Header and unavailable-state views were updated to be magnification-aware but the Markdown body still uses hard-coded FontSize(...) values inside Markdown(...).markdownTheme(cmuxMarkdownTheme), so the body won't scale with global magnification; update the Markdown theme construction to compute those FontSize values through the same magnification/scaling helper you used for the header (or add a new helper like scaledFontSize(_:)) and replace each hard-coded FontSize(...) used in the Markdown(...).markdownTheme(cmuxMarkdownTheme) calls so that the markdown body sizes are derived from the global magnification factor.Sources/AppDelegate.swift (1)
641-641: Consider renaming observer symbols to “Magnification” for consistency.
globalFontSizeObserver/installGlobalFontSizeObserver()are wired toGlobalFontMagnification.didChangeNotification. Renaming reduces ambiguity with per-pane zoom/font-size logic.✏️ Suggested rename
- private var globalFontSizeObserver: NSObjectProtocol? + private var globalFontMagnificationObserver: NSObjectProtocol? - installGlobalFontSizeObserver() + installGlobalFontMagnificationObserver() - private func installGlobalFontSizeObserver() { - guard globalFontSizeObserver == nil else { return } - globalFontSizeObserver = NotificationCenter.default.addObserver( + private func installGlobalFontMagnificationObserver() { + guard globalFontMagnificationObserver == nil else { return } + globalFontMagnificationObserver = NotificationCenter.default.addObserver(Also applies to: 9044-9045
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/AppDelegate.swift` at line 641, Rename the globalFontSizeObserver property and installGlobalFontSizeObserver() to use "Magnification" for clarity: change globalFontSizeObserver -> globalMagnificationObserver and installGlobalFontSizeObserver() -> installGlobalMagnificationObserver(), update any removes/uses (e.g., removeObserver calls, notification handler registrations) that reference GlobalFontMagnification.didChangeNotification and adjust all call sites including the equivalents around the other occurrence (previously lines ~9044-9045) so names are consistent; also update any comments and tests that reference the old names.Sources/GhosttyConfig.swift (2)
797-801: Verify numeric type compatibility instoredPercent.
UserDefaults.standard.object(forKey: percentKey) as? Intworks for Int values stored viadefaults.set(_:forKey:), but if the same key were ever written with a Double (for example from a future JSON parser path that forwards the rawjsonDoublevalue without rounding), the cast would silently fail and fall back todefaultPercent, masking a managed value.Currently both writers (
setPercenthere andKeyboardShortcutSettingsFileStore.parseAppSectionat Lines 440-442) round toIntbefore storing, so this is safe today. Consider hardening the read to tolerate either numeric form so future writers don't silently regress:🛡️ Proposed defensive read
static var storedPercent: Int { - let value = UserDefaults.standard.object(forKey: percentKey) as? Int - let resolved = value ?? defaultPercent + let object = UserDefaults.standard.object(forKey: percentKey) + let resolved: Int + if let intValue = object as? Int { + resolved = intValue + } else if let number = object as? NSNumber { + resolved = Int(number.doubleValue.rounded()) + } else { + resolved = defaultPercent + } return clamp(resolved) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyConfig.swift` around lines 797 - 801, storedPercent currently reads UserDefaults with `object(forKey: percentKey) as? Int` which will miss values written as Double; update the getter in storedPercent to tolerate both Int and Double (and any NSNumber) by attempting to read as Int first, then as Double/NSNumber and convert/round to Int before passing to clamp; reference the existing symbols `storedPercent`, `percentKey`, `defaultPercent`, `clamp`, and the writers `setPercent` and `KeyboardShortcutSettingsFileStore.parseAppSection` to ensure the read logic matches how values may be written and preserves current rounding behavior.
822-832: Gate cache invalidation and notification on actual value change.
setPercent(_:)andresetToDefault()unconditionally invalidate theGhosttyConfigload cache and postdidChangeNotification, even when the effective stored percent is unchanged (e.g., rapid Stepper clicks that land on the same clamped value, or repeated resets while already at default). InAppDelegate.swift, this notification triggersGhosttyApp.shared.reloadConfiguration(...)per the AI summary, so duplicate posts cause redundant terminal reloads.♻️ Proposed refinement
static func setPercent(_ percent: Int) { - UserDefaults.standard.set(clamp(percent), forKey: percentKey) - GhosttyConfig.invalidateLoadCache() - NotificationCenter.default.post(name: didChangeNotification, object: nil) + let clamped = clamp(percent) + let previous = storedPercent + UserDefaults.standard.set(clamped, forKey: percentKey) + guard clamped != previous else { return } + GhosttyConfig.invalidateLoadCache() + NotificationCenter.default.post(name: didChangeNotification, object: nil) } static func resetToDefault() { + let previous = storedPercent UserDefaults.standard.removeObject(forKey: percentKey) + guard previous != defaultPercent else { return } GhosttyConfig.invalidateLoadCache() NotificationCenter.default.post(name: didChangeNotification, object: nil) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyConfig.swift` around lines 822 - 832, setPercent(_:) and resetToDefault() currently always call GhosttyConfig.invalidateLoadCache() and post didChangeNotification even when the effective stored percent doesn’t change; update both to first compute the new effective value (use clamp(percent) in setPercent(_:)) and compare it against the current stored value (reading UserDefaults.standard.integer(forKey: percentKey) or checking object(forKey:) for absence) and only proceed to write to UserDefaults, call GhosttyConfig.invalidateLoadCache(), and post NotificationCenter.default.post(name: didChangeNotification, ...) if the value actually changed; do the analogous check in resetToDefault() (only removeObject/notify if the stored value is not already the default).Sources/KeyboardShortcutSettingsFileStore.swift (1)
437-446: Consider addingglobalFontMagnificationto the default JSONC template.The key is parsed and added to
supportedSettingsJSONPaths, butdefaultTemplateSections()(Lines 1229-1245) does not include it in the"app"section. Users who bootstrap a freshsettings.jsonwon't see a commented example for this new setting, reducing discoverability compared to peers likeminimalMode,preferredEditor, etc.💡 Proposed addition in defaultTemplateSections
"renameSelectsExistingName": CommandPaletteRenameSelectionSettings.defaultSelectAllOnFocus, "commandPaletteSearchesAllSurfaces": CommandPaletteSwitcherSearchSettings.defaultSearchAllSurfaces, + "globalFontMagnification": GlobalFontMagnification.defaultPercent, ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/KeyboardShortcutSettingsFileStore.swift` around lines 437 - 446, The default JSONC template is missing the "globalFontMagnification" example under the "app" section; update the defaultTemplateSections() implementation to add a commented example entry for "globalFontMagnification" (using the percent form and referencing GlobalFontMagnification.defaultPercent or GlobalFontMagnification.percentKey for the example value) so it matches the key already parsed in supportedSettingsJSONPaths and the parsing logic in the globalFontMagnification block; locate defaultTemplateSections() and insert the commented example in the "app" section alongside peers like "minimalMode" and "preferredEditor".Sources/cmuxApp.swift (1)
7153-7199: Route this control throughGlobalFontMagnification’s helper APIs.This view writes the raw
@AppStoragevalue directly and replays side effects viaonApply(). The enum inSources/GhosttyConfig.swiftalready centralizes clamping, cache invalidation, notification posting, and true reset semantics, so usingsetPercent(_:)/resetToDefault()here would avoid drift and keep the Reset button from persisting an explicit100override.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/cmuxApp.swift` around lines 7153 - 7199, The view currently mutates the raw percent binding and calls onApply(); instead route all changes through GlobalFontMagnification's helpers: in the Stepper binding call GlobalFontMagnification.setPercent(_:) with the clamped new value (then call onApply()), and replace the Reset button body to call GlobalFontMagnification.resetToDefault() (then call onApply()) instead of writing percent = ...; keep using GlobalFontMagnification.clamp(...) only for display if needed, and avoid persisting an explicit 100 override by using resetToDefault().
🤖 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`:
- Line 4737: The SettingsView.resetAllSettings() function does not reset the
`@AppStorage-backed` globalFontMagnificationPercent to
GlobalFontMagnification.defaultPercent, so add a step in resetAllSettings() to
explicitly set globalFontMagnificationPercent =
GlobalFontMagnification.defaultPercent (or call a restore method on
GlobalFontMagnification if one exists). Locate the AppStorage property
globalFontMagnificationPercent in cmuxApp.swift and update the
SettingsView.resetAllSettings() implementation to restore that value to the
default constant GlobalFontMagnification.defaultPercent so the “Reset All
Settings” flow clears the magnification toggle.
In `@Sources/ContentView.swift`:
- Line 2634: Several UI chrome elements use hard-coded frame heights while their
labels use the new .cmuxFont(size:_, weight:_) scaled fonts, causing clipping at
large dynamic type sizes; update those chrome sizes (titlebar row,
command-palette rows, help-menu items, unread badge, close affordance) to either
compute their heights from the effective scaled font metrics (use
UIFontMetrics/FontMetrics to get scaled lineHeight or use a scaled font size via
Font.scaled(...) and derive frame height from its lineHeight) or explicitly opt
those chrome components out of scaling (wrap with .dynamicTypeSize(.medium) or
fix their font to an unscaled size) so the frame heights track the .cmuxFont
scaling. Ensure you change the hardcoded numeric heights to calculations based
on the scaled font for the identifiers mentioned rather than leaving the fixed
constants.
In `@Sources/GhosttyTerminalView.swift`:
- Around line 2122-2129: You're currently reading the font-size from
GhosttyConfig.load() which uses the parallel Swift parser; instead derive the
magnified size from the same config being finalized in this method (or accept an
explicit uncached value), e.g. replace the GhosttyConfig.load() call and compute
scaled from the local config state (use the appropriate field or getter on the
`config`/`ghostty_config_t` being built) and pass that `scaled` into
loadInlineGhosttyConfig("font-size = \(scaled)", into: config, prefix:
"cmux-global-font-magnification", logLabel: "global font magnification") so the
preview/reload path uses the single source-of-truth (the `config` passed into
this function) rather than GlobalFontMagnification/ GhosttyConfig.load().
In `@Sources/Panels/BrowserPanelView.swift`:
- Around line 883-885: The controls use scaled fonts via .cmuxFont(...) but are
locked to fixed .frame(width: addressBarButtonHitSize, height:
addressBarButtonHitSize) which causes clipping at high magnification; update
these frames (and the other occurrences you listed) to allow expansion by using
adaptive constraints such as .frame(minWidth: addressBarButtonHitSize,
minHeight: addressBarButtonHitSize, alignment: .center) or .frame(minWidth:...,
idealWidth:..., minHeight:..., idealHeight:...) (or remove fixed width/height
and rely on .fixedSize() / intrinsic sizing) so the .cmuxFont text/icons can
grow without clipping while preserving the same minimum touch target.
---
Outside diff comments:
In `@Sources/Panels/BrowserPanelView.swift`:
- Around line 1160-1224: The AppKit text field inside
OmnibarTextFieldRepresentable is using a hardcoded .systemFont(ofSize: 12) which
bypasses the global magnification; update the representable so its
NSTextField/NSTextView font is driven by the same magnification/font source used
by .cmuxFont (or a shared FontProvider) and refresh that font in the
representable's updateNSView method when magnification changes; specifically,
replace the fixed .systemFont(ofSize: 12) initialization in
OmnibarTextFieldRepresentable with a lookup to the shared magnification-aware
font (or a passed-in Font/scale) and ensure updateNSView reapplies that font
whenever the magnification or relevant binding changes so inline editable text
scales consistently with the omnibar chrome.
In `@Sources/RightSidebarPanelView.swift`:
- Line 65: The mode bar HStack currently has a hard-coded .frame(height: 31)
which causes clipping when its child views use .cmuxFont(...) and scale with
GlobalFontMagnification; remove the fixed height so the HStack can size itself
OR multiply the fixed value by the same GlobalFontMagnification used by
.cmuxFont (e.g., replace 31 with 31 * GlobalFontMagnification) where the modeBar
is configured, and apply the same change to any other fixed-height containers
wrapping scaled .cmuxFont content in this file (look for modeBar and other
HStacks using .frame(height: ...)).
In `@Sources/Update/UpdatePopoverView.swift`:
- Around line 60-90: The hard-coded 60pt label column truncates localized labels
at high magnification; update the callers that set labelWidth (in
UpdateMetadataView, DetectedBackgroundUpdateView, UpdateAvailableView) to
compute it from the global magnification (use GlobalFontMagnification.scale to
multiply a base width) or remove the fixed width and apply
.fixedSize(horizontal: true, vertical: false) to the label Text views instead;
also replace the inline .frame(width: 60) in DetectedBackgroundUpdatePendingView
with the same computed width or .fixedSize so all uses of labelWidth (and the
inline frame) expand correctly with .cmuxFont scaling.
In `@Sources/Update/UpdateTitlebarAccessory.swift`:
- Around line 378-393: The icon / badge are being visually scaled via
cmuxFont(...) but their containing geometry (TitlebarControlsStyleConfig
properties like buttonSize, badgeSize, spacing and the titlebarHintWidth
measurement that uses NSFont.systemFont(...)) remains static, causing clipping
and misalignment; fix by either (A) making TitlebarControlsStyleConfig compute
buttonSize/badgeSize/spacing (and the NSFont used in titlebarHintWidth) from the
same magnification factor used by cmuxFont so the frames grow with the glyphs,
or (B) stop applying cmuxFont to SF Symbol iconLabel and shortcut hint
measurement and instead use unscaled .font(...) / NSFont for layout while
keeping cmuxFont only for inner text; update iconLabel usage, the ZStack badge
sizing (config.badgeSize and Circle) and the titlebarHintWidth font measurement
to use the chosen consistent approach so icons render fully, badges don't
overflow, and hint pills stay aligned.
---
Nitpick comments:
In `@Sources/AppDelegate.swift`:
- Line 641: Rename the globalFontSizeObserver property and
installGlobalFontSizeObserver() to use "Magnification" for clarity: change
globalFontSizeObserver -> globalMagnificationObserver and
installGlobalFontSizeObserver() -> installGlobalMagnificationObserver(), update
any removes/uses (e.g., removeObserver calls, notification handler
registrations) that reference GlobalFontMagnification.didChangeNotification and
adjust all call sites including the equivalents around the other occurrence
(previously lines ~9044-9045) so names are consistent; also update any comments
and tests that reference the old names.
In `@Sources/cmuxApp.swift`:
- Around line 7153-7199: The view currently mutates the raw percent binding and
calls onApply(); instead route all changes through GlobalFontMagnification's
helpers: in the Stepper binding call GlobalFontMagnification.setPercent(_:) with
the clamped new value (then call onApply()), and replace the Reset button body
to call GlobalFontMagnification.resetToDefault() (then call onApply()) instead
of writing percent = ...; keep using GlobalFontMagnification.clamp(...) only for
display if needed, and avoid persisting an explicit 100 override by using
resetToDefault().
In `@Sources/GhosttyConfig.swift`:
- Around line 797-801: storedPercent currently reads UserDefaults with
`object(forKey: percentKey) as? Int` which will miss values written as Double;
update the getter in storedPercent to tolerate both Int and Double (and any
NSNumber) by attempting to read as Int first, then as Double/NSNumber and
convert/round to Int before passing to clamp; reference the existing symbols
`storedPercent`, `percentKey`, `defaultPercent`, `clamp`, and the writers
`setPercent` and `KeyboardShortcutSettingsFileStore.parseAppSection` to ensure
the read logic matches how values may be written and preserves current rounding
behavior.
- Around line 822-832: setPercent(_:) and resetToDefault() currently always call
GhosttyConfig.invalidateLoadCache() and post didChangeNotification even when the
effective stored percent doesn’t change; update both to first compute the new
effective value (use clamp(percent) in setPercent(_:)) and compare it against
the current stored value (reading UserDefaults.standard.integer(forKey:
percentKey) or checking object(forKey:) for absence) and only proceed to write
to UserDefaults, call GhosttyConfig.invalidateLoadCache(), and post
NotificationCenter.default.post(name: didChangeNotification, ...) if the value
actually changed; do the analogous check in resetToDefault() (only
removeObject/notify if the stored value is not already the default).
In `@Sources/KeyboardShortcutSettingsFileStore.swift`:
- Around line 437-446: The default JSONC template is missing the
"globalFontMagnification" example under the "app" section; update the
defaultTemplateSections() implementation to add a commented example entry for
"globalFontMagnification" (using the percent form and referencing
GlobalFontMagnification.defaultPercent or GlobalFontMagnification.percentKey for
the example value) so it matches the key already parsed in
supportedSettingsJSONPaths and the parsing logic in the globalFontMagnification
block; locate defaultTemplateSections() and insert the commented example in the
"app" section alongside peers like "minimalMode" and "preferredEditor".
In `@Sources/Panels/MarkdownPanelView.swift`:
- Around line 74-100: Header and unavailable-state views were updated to be
magnification-aware but the Markdown body still uses hard-coded FontSize(...)
values inside Markdown(...).markdownTheme(cmuxMarkdownTheme), so the body won't
scale with global magnification; update the Markdown theme construction to
compute those FontSize values through the same magnification/scaling helper you
used for the header (or add a new helper like scaledFontSize(_:)) and replace
each hard-coded FontSize(...) used in the
Markdown(...).markdownTheme(cmuxMarkdownTheme) calls so that the markdown body
sizes are derived from the global magnification factor.
In `@Sources/Settings/ConfigSettingsView.swift`:
- Line 77: The editor's NSTextView font is hardcoded in ConfigSettingsTextView
so it doesn't follow GlobalFontMagnification; update the font-setting logic in
ConfigSettingsTextView to multiply the base monospaced font size (currently 12)
by GlobalFontMagnification.storedPercent (or an appropriate scaled value) and
set NSTextView.font accordingly, and add an observer for
GlobalFontMagnification.didChangeNotification to recompute and apply the scaled
font when magnification changes; ensure you reference and update the same place
where .monospacedSystemFont(ofSize: 12, weight: .regular) is set so the editor
body scales in sync with the path label, status caption, and banner footnote.
🪄 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: 181ec36a-45da-4e77-b897-b5707475f607
📒 Files selected for processing (21)
Resources/Localizable.xcstringsSources/AppDelegate.swiftSources/ContentView.swiftSources/Find/BrowserSearchOverlay.swiftSources/Find/SurfaceSearchOverlay.swiftSources/GhosttyConfig.swiftSources/GhosttyTerminalView.swiftSources/KeyboardShortcutSettings.swiftSources/KeyboardShortcutSettingsFileStore.swiftSources/NotificationsPage.swiftSources/Panels/BrowserPanelView.swiftSources/Panels/MarkdownPanelView.swiftSources/RightSidebarPanelView.swiftSources/SessionIndexView.swiftSources/Settings/ConfigSettingsView.swiftSources/ShortcutHintPill.swiftSources/Update/UpdatePopoverView.swiftSources/Update/UpdateTitlebarAccessory.swiftSources/WorkspaceContentView.swiftSources/cmuxApp.swiftweb/data/cmux-settings.schema.json
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 21 files
You’re at about 93% of the monthly review 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.
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
Sources/GhosttyConfig.swift (1)
265-272: MakeapplyGlobalMagnificationIfNeeded()idempotent or rename to reflect single-shot semantics.This mutates
selfin place and is not idempotent — invoking it twice on the sameGhosttyConfigvalue would compound the scale (e.g., 150% → 225%). It is currently only invoked fromloadFromDisk, but the publicmutatingsurface combined with a generic name is an easy footgun for future callers (e.g., someone re-applying after a partial reload). Consider either gating it on a stored "already-applied" flag, or making the helper non-mutating (func scaledFontSizes() -> (CGFloat, CGFloat)) and applying once at the call site.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyConfig.swift` around lines 265 - 272, applyGlobalMagnificationIfNeeded() on GhosttyConfig mutates fontSize and surfaceTabBarFontSize and is non‑idempotent; change it to a non‑mutating helper (e.g., func scaledFontSizes() -> (CGFloat, CGFloat)) that returns scaled values using GlobalFontMagnification.scale, then update callers (like loadFromDisk) to assign those returned values once, or alternatively implement an internal "hasAppliedGlobalMagnification" Bool on GhosttyConfig and early‑return when already applied; reference applyGlobalMagnificationIfNeeded(), GhosttyConfig, GlobalFontMagnification, fontSize, surfaceTabBarFontSize and update loadFromDisk to use the new, safe behavior.
🤖 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 7175-7191: The Stepper currently uses an empty label and
.labelsHidden(), leaving VoiceOver without a name; update the Stepper (symbol:
Stepper with the Binding get/set to percent and clamp via
GlobalFontMagnification.clamp, and onApply) to set an explicit accessibility
label by adding .accessibilityLabel(...) (use a localized string like "Global
Font Magnification" or the existing localized key) on the same view chain, and
optionally include the current percent in .accessibilityValue(...) so the
control announces its value (retain
.accessibilityIdentifier("SettingsGlobalFontMagnificationStepper") and
.labelsHidden()).
- Around line 2713-2715: The Text view using Text(String(localized:
"about.appName", defaultValue: "cmux")) currently applies .bold() before
.cmuxFont(.title), which is overridden by cmuxFont; fix by ensuring the weight
is applied after or inside the custom font—either move the weight call after
cmuxFont (e.g., apply .bold() or .fontWeight(.bold) after cmuxFont(.title)) or
update cmuxFont to accept and apply a weight parameter for the .title style so
the title remains bold (reference the Text view and cmuxFont(.title) usage).
In `@Sources/GhosttyConfig.swift`:
- Around line 797-801: The computed property storedPercent currently casts
UserDefaults.standard.object(forKey: percentKey) as? Int which fails for
non-integer NSNumbers; update storedPercent to first try casting the stored
object to NSNumber (or use value(forKey:) as? NSNumber), extract an Int (e.g.
using intValue or rounding if needed), fall back to defaultPercent if conversion
fails, and then return clamp(resolved); reference storedPercent, percentKey,
defaultPercent and clamp when locating where to change.
---
Nitpick comments:
In `@Sources/GhosttyConfig.swift`:
- Around line 265-272: applyGlobalMagnificationIfNeeded() on GhosttyConfig
mutates fontSize and surfaceTabBarFontSize and is non‑idempotent; change it to a
non‑mutating helper (e.g., func scaledFontSizes() -> (CGFloat, CGFloat)) that
returns scaled values using GlobalFontMagnification.scale, then update callers
(like loadFromDisk) to assign those returned values once, or alternatively
implement an internal "hasAppliedGlobalMagnification" Bool on GhosttyConfig and
early‑return when already applied; reference applyGlobalMagnificationIfNeeded(),
GhosttyConfig, GlobalFontMagnification, fontSize, surfaceTabBarFontSize and
update loadFromDisk to use the new, safe behavior.
🪄 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: 1442c33d-ce27-4054-8fe0-3116e4b84567
📒 Files selected for processing (4)
Sources/GhosttyConfig.swiftSources/GhosttyTerminalView.swiftSources/KeyboardShortcutSettingsFileStore.swiftSources/cmuxApp.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- Sources/KeyboardShortcutSettingsFileStore.swift
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 7237-7243: The subtitle Text inside SettingsCardRow currently
forces .lineLimit(2) which clips scaled subtitles; locate the subtitle handling
(the if let subtitle { Text(subtitle) .cmuxFont(.caption)
.foregroundColor(.secondary) .lineLimit(2) } block) and remove the .lineLimit(2)
(or replace it with .lineLimit(nil)) so the subtitle can wrap freely at larger
accessibility/text-scaling settings.
- Around line 7167-7207: The percent readout Text and the Reset Button aren’t
using the same scaled font as the row (they remain at default size), so apply
the same font-scaling modifier used by the row (e.g. cmuxFont or the
GlobalFontMagnification-based font modifier) to the percent Text (the
Text(String(format: "%d%%", GlobalFontMagnification.clamp(percent))) with
.monospacedDigit()) and to the Reset Button’s label (String(localized:
"settings.app.globalFontMagnification.reset"...)) so both the numeric percent
and the button text scale with GlobalFontMagnification; ensure any explicit
accessibilityValue/Label strings remain correct after scaling.
🪄 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: 57556dfe-13d1-4bc2-931b-7ffb722ab2b3
📒 Files selected for processing (3)
Sources/GhosttyConfig.swiftSources/cmuxApp.swiftweb/data/cmux-settings.schema.json
🚧 Files skipped from review as they are similar to previous changes (2)
- web/data/cmux-settings.schema.json
- Sources/GhosttyConfig.swift
|
+1 for the global approach — sidebar-only fixes the visible symptom, but tab bar / workspace titles / command palette have the same problem and your modifier solves all of them at once. |
|
Would love to see this merged — the lack of any sidebar/UI zoom is a real accessibility pain point. Happy to test the build if helpful. Thanks for putting this together! |
3 participants
Summary
Closes #3147
Added a global font magnification setting (50–200%, default 100%, 10% steps) that scales every font in cmux: terminals, tab bar, sidebar, workspace titles, and all SwiftUI chrome. Per-pane zoom (Cmd+=/-/0) is unchanged and still overrides the global value for the focused pane.
Note that browser panes are not impacted by auto magnification because this is content we don't control and maybe it doesn't look right with magnification. For now, the user can use the stadnard browser zoom if needed. If magnification should be applied to browser panes too it can be added later in a subsequent PR.
The fixed font size is too small for people with poor vision — workspace names, tab titles, and chrome are hard to read, and zooming each terminal pane individually isn't a viable workaround.
cmux's chrome has fixed-size budgets in places (titlebar height, command palette rows, badge frames, button hit areas). At ≤200% these absorb the scaled labels without clipping. For larger zoom users should reach for macOS Accessibility → Zoom, which scales the whole screen.
Testing
Built a dev build and verified that the global font magnification works, that pane zoom overrides work, that reset works, and that "Reset All Settings" returns the magnification to 100%.
See above.
Demo Video
For UI or behavior changes, include a short demo video (GitHub upload, Loom, or other direct link).
https://www.youtube.com/watch?v=kFxnzqIJqV4
Review Trigger (Copy/Paste as PR comment)
Checklist
Summary by CodeRabbit
New Features
Style
Localization