Clear Dock notifications on keyboard focus - #9427
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe runtime parity test now tracks panel focus calls and validates keyboard entry through ChangesDock focus behavior
Estimated code review effort: 1 (Trivial) | ~5 minutes Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to This change strengthens regression coverage for Dock keyboard focus and unread-notification dismissal without changing production behavior. It is mergeable after normal checks. 🚥 Pre-merge checks | ✅ 23 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (23 passed)
Full details: Linked Issues checkExplanation The regression test covers the requirements in [ Resolution Add the production change that routes right-sidebar keyboard entry through the shared Dock focus transaction and clears the pane unread state, in-app notification indicator, and derived macOS Dock badge. Keep the regression test to verify this behavior through focusFirstControl().
✨ Finishing Touches 💡 1📝 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 |
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 `@cmuxTests/DockRuntimeParityTests.swift`:
- Line 297: Update the assertion in the DockRuntimeParityTests focus test to
verify that the first control in the test panel actually received focus, using
the panel’s focus state or focus callback/count rather than only asserting the
return value of focusFirstControl().
🪄 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 Plus
Run ID: 0ec92e06-2fee-46b4-842b-984f784f7068
📒 Files selected for processing (2)
Sources/DockSplitStore+PaneFocus.swiftcmuxTests/DockRuntimeParityTests.swift
|
Regarding the algorithmic-complexity note: focusFirstControl runs only when focus enters the Dock or its registered focus host, not for ordinary Dock keystrokes. Re-resolving the selected panel inside focusPanelFromDockInteraction keeps one authoritative selection and dismissal transaction and avoids accepting stale pane or tab identities. Dock pane counts are bounded and small, so adding an index or a second pre-resolved mutation seam would increase consistency risk without improving a hot path. |
Resolves conflicts in the Dock keyboard-focus path: - Sources/DockSplitStore+PaneFocus.swift: keep main's focusFirstControl(), which already routes through focusPanelFromDockInteraction (#10340) and focuses the panel via reassertDockPanelInputFocus, so the branch's extra panel.focus() call is dropped. - cmuxTests/DockRuntimeParityTests.swift: keep main's new notificationOpensOnlyRenderedWindowDockPanels test alongside the branch's renamed keyboardEntryIntoWindowDockDismissesUnreadNotification test. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RSMiqTxhCoebgiVkBKgNRm
Clean merge, no conflicts. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
All contributors have signed the CLA ✍️ ✅ |
803dc26 Fix Codex hook injection paths with spaces (manaflow-ai#11968) 1769fd2 Fix Cloud discovery stalls and private address fallback (manaflow-ai#12266) dc5df2b Fix misplaced XCStrings localization entries (manaflow-ai#12171) 02d7597 ci: persist nightly Xcode compilation caches (manaflow-ai#12039) 1216d7c Fix native pane layout sync with bound cloud workspaces (manaflow-ai#12264) 40c1b73 Improve Computer Use onboarding and permission companion lifecycle (manaflow-ai#12265) 8229d75 ci: isolate Computer Use helper notarization tickets (manaflow-ai#12262) 2b75bd1 Fix bash PROMPT_COMMAND export leak (manaflow-ai#11257) (manaflow-ai#11290) e61ac8b Clear Dock notifications on keyboard focus (manaflow-ai#9427) dfccbd1 Fix cloud VM verification fixtures and agent login context (manaflow-ai#12258) 8ba29ea Cloud: one machine, one devbox snapshot ladder with displays; restore the original New Machine modal; refresh the agents to Claude Code 2.1.267 and Codex 0.154.0 (manaflow-ai#12250) 6810da8 cloud: cmux Cloud terminals run as cmux, not root (manaflow-ai#12101)
* test: cover Dock keyboard focus notification dismissal * fix: dismiss notifications on Dock keyboard focus * test: verify Dock panel receives keyboard focus --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Summary
Root cause
PR #9418 added the correct Dock interaction transaction, but the regression test called that helper directly. The real right-sidebar keyboard entry path still selected the Bonsplit pane and focused its panel through focusFirstControl without invoking notification dismissal.
This follow-up makes keyboard entry use the same transaction as Dock pointer clicks, shortcuts, and focus history.
Testing
Follow-up to #9418.
Closes #9415
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Dismisses Dock unread notifications on keyboard entry by routing focus through the Dock interaction path, aligning behavior with clicks, shortcuts, and focus history. Closes #9415.
focusFirstControl()throughfocusPanelFromDockInteractionto trigger dismissal and ensure the panel receives focus.focus()is called on the panel, and verify unread/badge clearing.Written for commit cb74c27. Summary will update on new commits.
Summary by CodeRabbit
Note
Low Risk
Changes are limited to Dock parity tests and a test panel stub; no production behavior is modified in this diff.
Overview
Updates the Dock runtime parity regression so unread dismissal is exercised through
focusFirstControl()(right-sidebar keyboard entry) instead of callingfocusPanelFromDockInteractiondirectly.The test double now counts
focus()invocations, and the case is renamed to keyboard entry into a window Dock so it asserts unread projection, total unread count, dock badge label, and dismiss flash still clear when entry goes through the real first-control path.Reviewed by Cursor Bugbot for commit cb74c27. Bugbot is set up for automated code reviews on this repo. Configure here.