Add more ergonomic keybindings for navigation#225
Conversation
Add viewer-scoped keybindings and hold-to-repeat frame navigation: introduce NEXT_FRAME/PREV_FRAME actions and repeating callbacks using QTimer (A/D now navigate frames while held). Add install_viewer_keybindings and pass the Viewer into install_points_layer_keybindings so viewer-scoped and layer-scoped bindings are both registered. Also add alternate Up/Down keys (W/S) and change several tracking shortcut keys to uppercase (H/J/K/L/I/U). Update LayerLifecycleManager to install viewer bindings when resources are initialized.
There was a problem hiding this comment.
Pull request overview
This PR expands and reorganizes napari-deeplabcut’s keyboard shortcut system to make annotation navigation more ergonomic, including adding viewer-scoped frame navigation (A/D with hold-to-repeat) and alternate keypoint navigation keys (W/S).
Changes:
- Adds viewer-scoped navigation actions for previous/next frame, including a QTimer-based hold-to-repeat implementation.
- Extends keypoint navigation shortcuts to include W/S as alternates for Up/Down.
- Updates tracking shortcut keys to uppercase and installs viewer keybindings from the lifecycle manager.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/napari_deeplabcut/core/layer_lifecycle/manager.py |
Installs viewer keybindings alongside points-layer keybindings during points runtime attachment. |
src/napari_deeplabcut/config/keybinds.py |
Adds viewer-scoped shortcuts (A/D), repeating callbacks via QTimer, W/S alternates, and updates tracking shortcut keys. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Update test_shortcuts_registry to include 'viewer' as an allowed spec.scope value for points-layer/global-points entries. This accommodates a newly introduced 'viewer' scope in the shortcuts registry so the test no longer fails when callbacks are registered under that scope.
deruyter92
left a comment
deruyter92
left a comment
There was a problem hiding this comment.
Good PR. Just one question about re-installing bindings for viewer for every managed layer, otherwise looks good to me!
| if not resources.keybindings_installed: | ||
| install_points_layer_keybindings(layer, controls, store) | ||
| install_points_layer_keybindings(layer, controls, store, self.viewer) | ||
| install_viewer_keybindings(self.viewer) | ||
| resources.keybindings_installed = True | ||
|
|
There was a problem hiding this comment.
Not entirely familiar with how Napari keybindings work, but does this mean that the viewer keybindings are re-installed for every layer? Do we need a viewer-level guard that checks if they are allready installed? or is this harmless?
There was a problem hiding this comment.
To my knowledge it is safe and repeated rebinds calling bound functions several times should not occur.
Still, very easy to guard this, since the manager, once created, is expected to survive until the viewer is closed. So tracking if viewer keybinds have been installed for the viewer already is easy and has been added in 4883cfc
Update the descriptions for the Prev/Next frame shortcuts to indicate that holding the A or D key advances frames quickly ("fast on hold"). This makes the behavior clearer to users. Modified src/napari_deeplabcut/config/keybinds.py.
Add a viewer_keybinds_installed flag to LayerLifecycleManager and initialize it to False. Only call install_viewer_keybindings(self.viewer) when that flag is False, then set it to True. This prevents duplicate viewer keybinding installations when resources.keybindings_installed is shared across layers.
Prevent dropdowns from interacting with single-key 'w'/'s' presses used for frame seeking. Added IGNORED_FOR_DROPDOWN_MENU_SEEKING_KEYS to settings (imports Qt), and updated labels_and_dropdown to install an event filter on the combo view and override keyPressEvent to ignore those keys (preserving modifier combos). Left eventFilter handling for the opened list commented for now.
|
@deruyter92 Thanks a lot for the review, another issue I found while checking for your comment: a default behavior of dropdown menus (when a letter key is pressed, jump to the next entry that starts with that letter) was making the interaction a bit confusing when the menu is focused. A small fix was added in ffd7eb4, however it sorts of spreads out the shortcuts across config/keybinds. I could make it closer to the keybind registry only but that would add some slight boilerplate. What would you prefer between the current solution and something more integrated but heavier? I can also move the const to the keybinds file instead but I felt maybe it is best in config? |
Motivation
To make labeling easier, bind W/S to prev/next keypoint in list, and A/D to prev/next frame (works while held).
Scope