Repository navigation
Use pan arrows for canvas scroll hint - #6528
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughAdds a debug command ( ChangesDebug canvas scroll hint command
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (21 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 SummaryReplaces the canvas Command+scroll hint's
Confidence Score: 5/5Safe to merge — the icon swap and keycap refactor are mechanical, and the debug facility is correctly isolated behind #if DEBUG in a dedicated Debug/ folder. All new protocol requirements are inside the existing #if DEBUG block so release builds are unaffected. The debug trigger path does not touch the production one-time discovery flag. Localization covers all 20 supported locales. The only notable concern is a parameter combination in the new presentCommandScrollHint overload that would silently drop the hint, but that combination is never exercised by current callers. Packages/macOS/CmuxCanvasUI/Sources/CmuxCanvasUI/CanvasRootView+CommandScroll.swift — the presentCommandScrollHint(markSessionShown:replacingExisting:) overload has a logic gap worth guarding against in a follow-up. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Menu as Debug Menu
participant Socket as Control Socket
participant TC as TerminalController
participant Coord as ControlCommandCoordinator
participant RV as CanvasRootView
Menu->>TC: debugShowCanvasCommandScrollHint(in: workspace)
TC->>RV: debugShowCommandScrollHint()
RV->>RV: presentCommandScrollHint(markSessionShown: false, replacingExisting: true)
RV-->>Menu: hint shown (session flag untouched)
Socket->>Coord: debug.canvas.command_scroll_hint
Coord->>Coord: debugCanvasCommandScrollHint(params)
Coord->>TC: controlDebugShowCanvasCommandScrollHint(routing:)
TC->>TC: resolveCanvasWorkspace(routing:)
TC->>TC: debugShowCanvasCommandScrollHint(in: workspace)
TC->>RV: debugShowCommandScrollHint()
RV-->>Coord: .ok(mode: "canvas")
Coord-->>Socket: "{"mode": "canvas"}"
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Menu as Debug Menu
participant Socket as Control Socket
participant TC as TerminalController
participant Coord as ControlCommandCoordinator
participant RV as CanvasRootView
Menu->>TC: debugShowCanvasCommandScrollHint(in: workspace)
TC->>RV: debugShowCommandScrollHint()
RV->>RV: presentCommandScrollHint(markSessionShown: false, replacingExisting: true)
RV-->>Menu: hint shown (session flag untouched)
Socket->>Coord: debug.canvas.command_scroll_hint
Coord->>Coord: debugCanvasCommandScrollHint(params)
Coord->>TC: controlDebugShowCanvasCommandScrollHint(routing:)
TC->>TC: resolveCanvasWorkspace(routing:)
TC->>TC: debugShowCanvasCommandScrollHint(in: workspace)
TC->>RV: debugShowCommandScrollHint()
RV-->>Coord: .ok(mode: "canvas")
Coord-->>Socket: "{"mode": "canvas"}"
Reviews (6): Last reviewed commit: "Fix canvas debug hint warnings" | Re-trigger Greptile |
|
Correction to my previous source-artifact note: I checked the branch with git diff --name-only origin/main...HEAD and the PR introduces only Packages/macOS/CmuxCanvasUI/Sources/CmuxCanvasUI/CanvasCommandScrollHint.swift. The .claude and .greptile paths are present in the base tree and are not part of this PR diff. |
| #if DEBUG | ||
| /// Shows the Command+scroll discovery hint for debug-menu and automation | ||
| /// testing, without consuming the production one-time scroll trigger. | ||
| func debugShowCommandScrollHint() | ||
| #endif |
There was a problem hiding this comment.
Debug-named seam added to production public protocol
debugShowCommandScrollHint() is a debug…-prefixed method guarded by #if DEBUG in a production Sources/ protocol — this pattern is explicitly called out as a failure by the cmux-no-test-debug-seam-in-production-source rule. The same issue appears in CanvasRootView+CommandScroll.swift (lines 123-128, public func debugShowCommandScrollHint()) and Sources/Canvas/CanvasAction.swift (lines 25-28, #if DEBUG case showCommandScrollHint). Any external CanvasViewportControlling conformer must now implement a debug-only method in debug builds.
The canonical fix (see PR 6452) is to keep debug-only facilities in a dedicated debug file or folder rather than polluting the production protocol and enum. The debugShowCommandScrollHint call chain and CanvasAction.showCommandScrollHint case should live entirely inside a debug-targeted extension or support file, calling back into the non-debug surface only through already-widened internal APIs. The CanvasViewportControlling protocol and CanvasAction enum should not carry #if DEBUG members.
Rule Used: Flag Swift files under a production Sources path (... (source)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/cmuxApp.swift`:
- Around line 543-547: The string key debug.menu.showCanvasCommandScrollHint
used in the Button in cmuxApp.swift is missing translations for 18 locales in
Resources/Localizable.xcstrings. Open the Localizable.xcstrings file, locate the
debug.menu.showCanvasCommandScrollHint entry, and add translated entries for all
missing locales: ar, bs, da, de, es, fr, it, km, ko, nb, pl, pt-BR, ru, th, tr,
uk, zh-Hans, and zh-Hant. Ensure each locale has an appropriate translation for
the "Show Canvas Scroll Hint" message to meet the full-internationalization
requirement for all 20 supported locales.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5d6a3faa-ebcf-43d9-9cfa-3c5ed7affa7b
📒 Files selected for processing (15)
Packages/macOS/CmuxCanvasUI/Sources/CmuxCanvasUI/CanvasRootView+CommandScroll.swiftPackages/macOS/CmuxCanvasUI/Sources/CmuxCanvasUI/CanvasViewportControlling.swiftPackages/macOS/CmuxCanvasUI/Tests/CmuxCanvasUITests/CanvasCommandScrollHintTests.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Canvas/ControlCanvasActionResolution.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Canvas/ControlCommandCoordinator+Canvas.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Debug/ControlCommandCoordinator+Debug.swiftPackages/macOS/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Debug/ControlDebugContext.swiftPackages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandContextTestStubs+Debug.swiftPackages/macOS/CmuxControlSocket/Tests/CmuxControlSocketTests/ControlCommandCoordinatorDebugCanvasTests.swiftResources/Localizable.xcstringsSources/Canvas/CanvasAction.swiftSources/TerminalController+ControlCanvasContext.swiftSources/TerminalController+ControlDebugContext.swiftSources/TerminalController.swiftSources/cmuxApp.swift
💤 Files with no reviewable changes (1)
- Resources/Localizable.xcstrings
1d1fb43 to
3294d94
Compare
…e silently dropped — #6518 (terminal input after window key restore), #6508 (defer restored WebViews until visible), #6559 (avoid DevTools teardown on redock), #6583 (gate idle port scanning to active workspace), #6528 (canvas scroll-hint debug menu), #6580 (drop re-introduced sideDock promote block). #6582/#6517 deferred (see merge-deferred-gaps)
Summary
Verification
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Replaced the canvas Command+scroll hint icon with pan arrows (
arrow.up.and.down.and.arrow.left.and.right) and styled it as a keycap to match the ⌘ keycap. Added a DEBUG-only way to show the hint via menu and control socket without consuming the one-time flag; dismissal and scroll timing remain unchanged.New Features
debug.canvas.command_scroll_hintroutes through the canvas action path, respects workspace routing selectors, and returnsinvalid_stateif not in canvas mode or if the viewport isn’t attached.Refactors
Written for commit 3841784. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
New Features
Style
Bug Fixes
Tests