Skip to content

feat(shortcuts): add duplicate shortcut cycling#1759

Open
tmsdnl wants to merge 1 commit into
rxhanson:mainfrom
tmsdnl:feature/shortcut-cycling
Open

feat(shortcuts): add duplicate shortcut cycling#1759
tmsdnl wants to merge 1 commit into
rxhanson:mainfrom
tmsdnl:feature/shortcut-cycling

Conversation

@tmsdnl
Copy link
Copy Markdown

@tmsdnl tmsdnl commented May 9, 2026

Related Discussion: #1758

Summary

  • Cycle between actions that share the same keyboard shortcut
  • Restart the cycle if the window was moved or resized after the last Rectangle action
  • Keep Rectangle shortcuts suspended while MASShortcut recording is active
  • Reject Todo shortcut conflicts quietly without the MASShortcut "already-used" alert
  • Add coverage for cycling, Todo shortcut validation, and recording state

Verification

  • xcodebuild test -project Rectangle.xcodeproj -scheme Rectangle -destination 'platform=macOS' -derivedDataPath /private/tmp/RectangleDerivedData
  • xcodebuild build -project Rectangle.xcodeproj -scheme Rectangle -destination 'platform=macOS' -derivedDataPath /private/tmp/RectangleDerivedData

Disclaimer
This implementation was Codex-assisted.

- Suspend shortcut bindings while recording shortcuts
- Reject todo shortcut conflicts without already-used alerts
- Cover overlapping shortcut recording state
@rxhanson
Copy link
Copy Markdown
Owner

Thanks for contributing! I quickly glanced through the changes and will give this a more thorough run-through soon.

I didn't quite catch it - how is the ordering determined for which action gets executed first if there are multiple actions with the same keyboard shortcut?

This change sort of recenters the ShortcutManager around this functionality, and I'm thinking there might be a more elegant way to achieve this while keeping it a little more separate. When I dive into this a bit deeper, I think I'll have a more clear picture of how to do this.

@tmsdnl
Copy link
Copy Markdown
Author

tmsdnl commented May 10, 2026

I see what you're saying. This leaks shortcut-recording UI state into ShortcutManager. The cycling logic may belong there, but this pause/resume behavior needs a cleaner boundary.

Let me know your thoughts and preferred direction.

@rxhanson
Copy link
Copy Markdown
Owner

Sorry for the delay. The reason for thinking down the path of reducing some of the responsibility of the ShortcutManager is that I am planning to swap out MASShortcut and would like to keep some of the MASShortcut interaction as isolated as possible. I'm actually finding it difficult to draw the line in ShortcutManager, though - do you have any thoughts on a good way to do this? It's nice to have the struct for keeping the cycling organization separate, but maybe there's a good way to pull more of the ShortcutManager class modifications out. Aside from that things are looking good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants