Make View menu zoom items context-aware for terminal font size#1989
Open
anthhub wants to merge 4 commits intomanaflow-ai:mainfrom
Open
Make View menu zoom items context-aware for terminal font size#1989anthhub wants to merge 4 commits intomanaflow-ai:mainfrom
anthhub wants to merge 4 commits intomanaflow-ai:mainfrom
Conversation
The Zoom In/Out/Actual Size menu items previously only controlled browser zoom. Now they dispatch to terminal font size when a terminal panel is focused, matching the existing Cmd+=/−/0 keyboard shortcut behavior that was already routed to Ghostty at the key event level. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@anthhub is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
Greptile SummaryThis PR makes the View menu zoom actions context-aware: when a terminal panel is focused, Zoom In/Out/Actual Size now adjust the terminal font size via Ghostty binding actions instead of the browser zoom level. When a browser panel is focused the existing behavior is preserved. The change is minimal and well-scoped. Key changes:
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["User clicks View > Zoom In / Out / Actual Size\n(or Cmd+= / − / 0)"] --> B{selectedTerminalPanel != nil?}
B -- "Yes (terminal focused)" --> C["manager.increaseFontSizeFocusedTerminal()\nmanager.decreaseFontSizeFocusedTerminal()\nmanager.resetFontSizeFocusedTerminal()"]
B -- "No (browser or no panel focused)" --> D["manager.zoomInFocusedBrowser()\nmanager.zoomOutFocusedBrowser()\nmanager.resetZoomFocusedBrowser()"]
C --> E["TerminalPanel.performBindingAction()\nincrease_font_size:1 / decrease_font_size:1 / reset_font_size"]
D --> F["BrowserPanel.zoomIn() / .zoomOut() / .resetZoom()"]
E --> G["Ghostty surface updates font size"]
F --> H["Browser engine updates zoom level"]
Reviews (1): Last reviewed commit: "Make View menu zoom items context-aware ..." | Re-trigger Greptile |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 2 files
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThree TabManager methods were added to adjust/reset the focused terminal panel's font size. Menu handlers for "Zoom In", "Zoom Out", and "Actual Size" now route to terminal font-size methods when a terminal is focused and fall back to browser zoom otherwise; handlers beep if terminal action fails. Changes
Sequence DiagramsequenceDiagram
actor User
participant Menu as Menu Command Handler
participant TabMgr as TabManager
participant Term as TerminalPanel
participant Browser as BrowserView
User->>Menu: Trigger Zoom In / Out / Actual Size
Menu->>TabMgr: check activeTabManager.selectedTerminalPanel
alt Terminal focused
Menu->>TabMgr: increase/decrease/resetFontSizeFocusedTerminal()
TabMgr->>Term: performBindingAction("increase_font_size:1"/"decrease_font_size:1"/"reset_font_size")
Term-->>TabMgr: success/failure (Bool)
TabMgr-->>Menu: Bool
else Browser focused
Menu->>TabMgr: zoomIn/zoomOut/resetZoomFocusedBrowser()
TabMgr->>Browser: apply zoom change
end
alt returned false
Menu->>Menu: NSSound.beep()
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Sources/cmuxApp.swift (1)
745-773: Optional cleanup: extract shared zoom dispatch helper to reduce repetition.All three handlers duplicate the same branch shape; a tiny helper would reduce maintenance risk.
♻️ Proposed refactor
+ private func performContextAwareZoom( + terminalAction: (TabManager) -> Bool, + browserAction: (TabManager) -> Bool + ) { + let manager = activeTabManager + if manager.selectedTerminalPanel != nil { + _ = terminalAction(manager) + } else { + _ = browserAction(manager) + } + } ... Button(String(localized: "menu.view.zoomIn", defaultValue: "Zoom In")) { - let manager = activeTabManager - if manager.selectedTerminalPanel != nil { - _ = manager.increaseFontSizeFocusedTerminal() - } else { - _ = manager.zoomInFocusedBrowser() - } + performContextAwareZoom( + terminalAction: { $0.increaseFontSizeFocusedTerminal() }, + browserAction: { $0.zoomInFocusedBrowser() } + ) } ... Button(String(localized: "menu.view.zoomOut", defaultValue: "Zoom Out")) { - let manager = activeTabManager - if manager.selectedTerminalPanel != nil { - _ = manager.decreaseFontSizeFocusedTerminal() - } else { - _ = manager.zoomOutFocusedBrowser() - } + performContextAwareZoom( + terminalAction: { $0.decreaseFontSizeFocusedTerminal() }, + browserAction: { $0.zoomOutFocusedBrowser() } + ) } ... Button(String(localized: "menu.view.actualSize", defaultValue: "Actual Size")) { - let manager = activeTabManager - if manager.selectedTerminalPanel != nil { - _ = manager.resetFontSizeFocusedTerminal() - } else { - _ = manager.resetZoomFocusedBrowser() - } + performContextAwareZoom( + terminalAction: { $0.resetFontSizeFocusedTerminal() }, + browserAction: { $0.resetZoomFocusedBrowser() } + ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/cmuxApp.swift` around lines 745 - 773, These three Button actions duplicate the same branching logic; extract a small helper function (e.g., dispatchZoomAction(on activeTabManager: TabManager, terminalAction: ()->Void, browserAction: ()->Void) or a method on activeTabManager) that checks selectedTerminalPanel and calls either increaseFontSizeFocusedTerminal / decreaseFontSizeFocusedTerminal / resetFontSizeFocusedTerminal or zoomInFocusedBrowser / zoomOutFocusedBrowser / resetZoomFocusedBrowser accordingly, then replace each Button body to call that helper with the appropriate terminal and browser action identifiers (increaseFontSizeFocusedTerminal, zoomInFocusedBrowser, decreaseFontSizeFocusedTerminal, zoomOutFocusedBrowser, resetFontSizeFocusedTerminal, resetZoomFocusedBrowser).
🤖 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/cmuxApp.swift`:
- Around line 745-773: These three Button actions duplicate the same branching
logic; extract a small helper function (e.g., dispatchZoomAction(on
activeTabManager: TabManager, terminalAction: ()->Void, browserAction: ()->Void)
or a method on activeTabManager) that checks selectedTerminalPanel and calls
either increaseFontSizeFocusedTerminal / decreaseFontSizeFocusedTerminal /
resetFontSizeFocusedTerminal or zoomInFocusedBrowser / zoomOutFocusedBrowser /
resetZoomFocusedBrowser accordingly, then replace each Button body to call that
helper with the appropriate terminal and browser action identifiers
(increaseFontSizeFocusedTerminal, zoomInFocusedBrowser,
decreaseFontSizeFocusedTerminal, zoomOutFocusedBrowser,
resetFontSizeFocusedTerminal, resetZoomFocusedBrowser).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4d7503c4-8653-44d0-a5d6-e9d93576fad6
📒 Files selected for processing (2)
Sources/TabManager.swiftSources/cmuxApp.swift
Matches the pattern used by other View menu items (Toggle Developer Tools, Show JavaScript Console) that beep on failure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Only add beep feedback for terminal font size actions (new behavior). Browser zoom branches preserve the original silent-on-failure behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
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:750">
P2: Browser zoom View-menu actions now ignore failure return values, causing silent no-ops instead of user feedback when no focused browser can handle zoom.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Browser zoom operations silently discarded failures while terminal zoom operations beeped. Unify both paths to beep on failure using a shared success/fail pattern, which also reduces the branching duplication. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Hey @lawrencecchen @austinywang — just wanted to check in on this one. I have a handful of open PRs stacking up (17 total at this point 😅) and would love to get some of the smaller ones moving if you have a chance to take a look. Happy to rebase or address any feedback. No rush on the bigger ones, but stuff like this, #1998, and #2144 should be pretty straightforward to review. Thanks! |
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
1 participant
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.
Summary
increaseFontSizeFocusedTerminal(),decreaseFontSizeFocusedTerminal(),resetFontSizeFocusedTerminal()methods toTabManager(parallel to existing browser zoom methods)AppDelegateTest plan
./scripts/reload.sh --tag font-zoom🤖 Generated with Claude Code
Summary by cubic
Make the View > Zoom menu context-aware: Zoom In/Out/Actual Size change terminal font size when a terminal is focused, or browser zoom when a browser is focused. All Zoom menu actions now beep on failure; keyboard shortcuts (Cmd+=, Cmd−, Cmd+0) are unchanged.
New Features
TabManagermethods:increaseFontSizeFocusedTerminal,decreaseFontSizeFocusedTerminal,resetFontSizeFocusedTerminal.Bug Fixes
Written for commit 3ea6c5d. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes