Skip to content

feat: add UI zoom scaling for all non-terminal UI elements #1074

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
moyashin63 wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat: add UI zoom scaling for all non-terminal UI elements #1074

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fb05987
feat: extend UI zoom scaling to all non-terminal UI elements
moyashin63 Mar 7, 2026
e3aefbd
fix: prevent sidebar drag resize from breaking at max UI zoom
moyashin63 Mar 8, 2026
3c2b1a5
fix: address PR review comments for UI zoom scaling
moyashin63 Mar 8, 2026
a7649b7
fix: scale omnibar text field height with UI zoom
moyashin63 Mar 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions GhosttyTabs.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
A5001209 /* WindowToolbarController.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5001219 /* WindowToolbarController.swift */; };
A5001240 /* WindowDecorationsController.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5001241 /* WindowDecorationsController.swift */; };
A5001610 /* SessionPersistence.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5001611 /* SessionPersistence.swift */; };
A5D39BD8 /* UIZoomMetrics.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5036EBB /* UIZoomMetrics.swift */; };
A5001100 /* Assets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = A5001101 /* Assets.xcassets */; };
A5001230 /* Sparkle in Frameworks */ = {isa = PBXBuildFile; productRef = A5001231 /* Sparkle */; };
B9000002A1B2C3D4E5F60719 /* cmux.swift in Sources */ = {isa = PBXBuildFile; fileRef = B9000001A1B2C3D4E5F60719 /* cmux.swift */; };
Expand Down Expand Up @@ -204,6 +205,7 @@
A5001223 /* UpdateLogStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Update/UpdateLogStore.swift; sourceTree = "<group>"; };
A5001241 /* WindowDecorationsController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WindowDecorationsController.swift; sourceTree = "<group>"; };
A5001611 /* SessionPersistence.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SessionPersistence.swift; sourceTree = "<group>"; };
A5036EBB /* UIZoomMetrics.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UIZoomMetrics.swift; sourceTree = "<group>"; };
818DBCD4AB69EB72573E8138 /* SidebarResizeUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarResizeUITests.swift; sourceTree = "<group>"; };
B8F266256A1A3D9A45BD840F /* SidebarHelpMenuUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarHelpMenuUITests.swift; sourceTree = "<group>"; };
C0B4D9B1A1B2C3D4E5F60718 /* UpdatePillUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UpdatePillUITests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -395,6 +397,7 @@
A5001241 /* WindowDecorationsController.swift */,
A5001222 /* WindowAccessor.swift */,
A5001611 /* SessionPersistence.swift */,
A5036EBB /* UIZoomMetrics.swift */,
);
path = Sources;
sourceTree = "<group>";
Expand Down Expand Up @@ -662,6 +665,7 @@
A5001240 /* WindowDecorationsController.swift in Sources */,
A500120C /* WindowAccessor.swift in Sources */,
A5001610 /* SessionPersistence.swift in Sources */,
A5D39BD8 /* UIZoomMetrics.swift in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down
61 changes: 61 additions & 0 deletions Sources/AppDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1163,6 +1163,46 @@ struct CommandPaletteDebugSnapshot {
static let empty = CommandPaletteDebugSnapshot(query: "", mode: "commands", results: [])
}

enum UIZoomShortcutAction: Equatable {
case zoomIn
case zoomOut
case reset
}

func uiZoomShortcutAction(
flags: NSEvent.ModifierFlags,
chars: String,
keyCode: UInt16,
literalChars: String? = nil
) -> UIZoomShortcutAction? {
let normalizedFlags = flags
.intersection(.deviceIndependentFlagsMask)
.subtracting([.numericPad, .function])
// Require exactly Cmd+Shift (no Control, no Option)
guard normalizedFlags.contains(.command),
normalizedFlags.contains(.shift),
normalizedFlags.isDisjoint(with: [.control, .option]) else { return nil }
let keys = browserZoomShortcutKeyCandidates(
chars: chars,
literalChars: literalChars,
keyCode: keyCode
)

if keys.contains("=") || keys.contains("+") || keyCode == 24 || keyCode == 69 { // kVK_ANSI_Equal / kVK_ANSI_KeypadPlus
return .zoomIn
}

if keys.contains("-") || keys.contains("_") || keyCode == 27 || keyCode == 78 { // kVK_ANSI_Minus / kVK_ANSI_KeypadMinus
return .zoomOut
}

if keys.contains("0") || keyCode == 29 || keyCode == 82 { // kVK_ANSI_0 / kVK_ANSI_Keypad0
return .reset
}

return nil
}

func browserZoomShortcutAction(
flags: NSEvent.ModifierFlags,
chars: String,
Expand Down Expand Up @@ -7480,6 +7520,27 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
}
}

// UI zoom: Cmd+Shift+=/Cmd+Shift+-/Cmd+Shift+0
if let uiZoomAction = uiZoomShortcutAction(
flags: flags,
chars: chars,
keyCode: event.keyCode,
literalChars: event.characters
) {
let current = UIZoomMetrics.effectiveScale()
let next: Double
switch uiZoomAction {
case .zoomIn:
next = UIZoomMetrics.clamped(current + UIZoomMetrics.step)
case .zoomOut:
next = UIZoomMetrics.clamped(current - UIZoomMetrics.step)
case .reset:
next = UIZoomMetrics.defaultScale
}
UserDefaults.standard.set(next, forKey: UIZoomMetrics.appStorageKey)
return true
}
Comment on lines +7523 to +7545
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

This shortcut is unreachable while the command palette is open.

Line 6999 returns early for almost every Command shortcut while the palette is visible, so this block never runs for Cmd+Shift+=/-/0 in that state. That leaves part of the “all non-terminal UI elements” zoom flow non-functional. Please exempt UI zoom from shouldConsumeShortcutWhileCommandPaletteVisible(...) or evaluate uiZoomShortcutAction(...) before that guard.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Sources/AppDelegate.swift` around lines 7523 - 7542, The UI zoom shortcut
(uiZoomShortcutAction) is never reached because
shouldConsumeShortcutWhileCommandPaletteVisible prevents handling when the
command palette is visible; modify the shortcut handling so
uiZoomShortcutAction(flags:chars:keyCode:literalChars:) is evaluated before or
exempted from shouldConsumeShortcutWhileCommandPaletteVisible(...)—i.e., check
uiZoomShortcutAction first (or add a special-case that skips the command-palette
guard for zoom actions) and then apply the existing UIZoomMetrics logic
(clamped/step/defaultScale and UserDefaults.standard.set with
UIZoomMetrics.appStorageKey) so Cmd+Shift+=/-/0 works while the palette is open.

⚠️ Potential issue | 🟡 Minor

Add DEBUG tracing for the new UI zoom shortcut path.

The adjacent browser zoom path logs match/dispatch in DEBUG, but this new key route does not. Please add dlog() calls behind #if DEBUG so zoom shortcut routing stays debuggable.

As per coding guidelines, "All debug events (keys, mouse, focus, splits, tabs) must be logged to the unified debug event log in DEBUG builds using the dlog() free function".

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@Sources/AppDelegate.swift` around lines 7523 - 7542, Add DEBUG-only dlog()
tracing to the UI zoom shortcut path handled by uiZoomShortcutAction: inside the
if let block that computes current/next (referencing UIZoomMetrics and
uiZoomShortcutAction) add `#if` DEBUG / `#endif` guarded dlog() calls that record
the resolved uiZoomAction (zoomIn/zoomOut/reset), the previous scale (current)
and the computed next scale before calling UserDefaults.standard.set; use the
same dlog() style/payload as the adjacent browser zoom path so the event appears
in the unified debug event log.

Copy link
Copy Markdown
Author

@moyashin63 moyashin63 Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DEBUG tracing addressed in 3c2b1a5 — added dlog("uiZoom.\(uiZoomAction) current=\(current) next=\(next)").

Regarding the command palette guard: shouldConsumeShortcutWhileCommandPaletteVisible does block Cmd+Shift shortcuts when the palette is visible, so UI zoom won't work while the palette is open. This is a minor edge case (unlikely to zoom while typing in the palette) but will be addressed separately.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, something went wrong! Please try again later. 🐰 💔

Copy link
Copy Markdown
Author

@moyashin63 moyashin63 Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't fix. This is intentional. shouldConsumeShortcutWhileCommandPaletteVisible (line 1078) guards .command flag, then consumes all Cmd shortcuts except an explicit allow-list (Cmd+A/C/V/X/Z/Y, arrows, Delete) to protect the text input field. UI zoom shortcuts (Cmd+Shift+=/−/0) being blocked while the command palette is open is correct behavior — the palette is a text input context and should not fire unrelated shortcuts. Users close the palette first, then zoom.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@moyashin63 Acknowledged. Thanks for confirming the intent here and for adding the DEBUG trace — I see the dlog("uiZoom.(…) current=… next=…") in place.

I’ll treat the “UI zoom is intentionally blocked while the command palette is open” behavior as by-design for cmux and won’t flag it again.

Optional (no action required): consider dropping a brief inline comment near shouldConsumeShortcutWhileCommandPaletteVisible to document that UI zoom is deliberately consumed while the palette is visible, to preempt future refactors.

✏️ Learnings added 
Learnt from: moyashin63
Repo: manaflow-ai/cmux PR: 1074
File: Sources/AppDelegate.swift:7523-7545
Timestamp: 2026-03-09T01:38:24.337Z
Learning: In manaflow-ai/cmux (Sources/AppDelegate.swift), shouldConsumeShortcutWhileCommandPaletteVisible(...) intentionally consumes most Command shortcuts while the command palette is visible to protect its text input. As a result, UI zoom shortcuts (Cmd+Shift+= / Cmd+Shift+− / Cmd+Shift+0) must not fire while the palette is open. Do not reorder handlers (e.g., uiZoomShortcutAction(...)) to bypass this guard; users close the palette before zooming.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used 
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 762
File: Sources/ContentView.swift:5784-5788
Timestamp: 2026-03-06T02:00:30.333Z
Learning: In manaflow-ai/cmux (Sources/ContentView.swift), ShortcutHintModifierPolicy.shouldShowHints is Command-only: it returns true only when modifierFlags normalized to [.command]. Control-hold must not show sidebar/titlebar shortcut hints, and close-button suppression in TabItemView must remain tied to showsModifierShortcutHints (Command hold) or the always-show debug flag.

Learnt from: SuperManfred
Repo: manaflow-ai/cmux PR: 803
File: Sources/Panels/BrowserPopupWindowController.swift:36-38
Timestamp: 2026-03-04T05:11:56.373Z
Learning: In Sources/Panels/BrowserPopupWindowController.swift and Sources/Panels/BrowserPanel.swift, `webView.isInspectable = true` (guarded by `#available(macOS 13.3, *)`) is intentionally enabled in all builds — not just DEBUG — because cmux is a developer tool and full Web Inspector access is desired in production builds as well. Do not flag this as a security concern.

Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/ContentView.swift:3194-3196
Timestamp: 2026-03-04T14:05:48.668Z
Learning: In manaflow-ai/cmux (PR `#819`), Sources/ContentView.swift: The command palette’s external window labels intentionally use the global window index from the full orderedSummaries (index + 1), matching the Window menu in AppDelegate. Do not reindex after filtering out the current window to avoid mismatches (“Window 2” for an external window is expected).

Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/AppDelegate.swift:3491-3493
Timestamp: 2026-03-04T14:06:12.296Z
Learning: In PRs affecting this repository, limit the scope of localization (i18n) changes to Japanese translations for the file Sources/AppDelegate.swift. Do not include UX enhancements (e.g., preferring workspace.customTitle in workspaceDisplayName() or altering move-target labels) in this PR. Open a separate follow-up issue to address any UX-related changes to avoid scope creep and keep localization review focused.

Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-03-04T14:05:42.574Z
Learning: Guideline: In Swift files (cmux project), when handling pluralized strings, prefer using localization keys with the ICU-style plural forms .one and .other. For example, use keys like statusMenu.unreadCount.one for the singular case (1) and statusMenu.unreadCount.other for all other counts, and similarly for statusMenu.tooltip.unread.one/other. Rationale: ensures correct pluralization across locales and makes localization keys explicit. Review code to ensure any unread count strings and related tooltips follow this .one/.other key pattern and verify the correct value is chosen based on the count.

Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 954
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-05T22:04:34.712Z
Learning: Adopt the convention: for health/telemetry tri-state values in Swift, prefer Optionals (Bool?) over sentinel booleans. In TerminalController.swift, socketConnectable is Bool? and only set when socketProbePerformed is true; downstream logic must treat nil as 'not probed'. Ensure downstream code checks for nil before using a value and uses explicit non-nil checks to determine state, improving clarity and avoiding misinterpretation of default false.


#if DEBUG
logBrowserZoomShortcutTrace(stage: "probe", event: event, flags: flags, chars: chars)
#endif
Expand Down
Loading