-
Notifications
You must be signed in to change notification settings - Fork 565
Add shortcuts to switch between recordings #11637
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Web viewer built successfully.
View image diff on kitdiff. Note: This comment is updated whenever you push a commit. |
| Self::NextRecording => smallvec![cmd(Key::CloseBracket)], | ||
| Self::PreviousRecording => smallvec![cmd(Key::OpenBracket)], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There needs to be some bike shedding on these shortcuts.
- @pweids suggested it should be cmd +
[/] - @lucasmerlin thinks it should be
ctrl + tab/ctrl + shift + tab(which switches tabs in my browser, finder and recent files in my ide) - @abey79 would like something different
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: we can support multiple keyboard shortcuts (a primary and a backup; that's why this is a vec).
on macOS:
in my browser, I can cycle tabs with (at least) ctrl+[shift]+tab plus cmd+option+side-arrows plus ctrl+pageup/down.
on Windows and Linux, I think ctrl+[shift]+tab is the common one for switching between open tabs in an application?
Based on this, I suggest ctrl+[shift]+tab as the primary on all platforms, with cmd+option+down/up arrows as secondary on macOS.
But I'd like to know where @pweids got cmd + [ from? Is there some precedence there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ctrl-tab is application switching in Windows, isn't it? I guess we could check what's the browser tab shortcut there.
I'm not sure where cmd + [ comes from, but I don't like it because it's not intl keyboard layout friendly. I need to do alt + 5 to get a [ on my swiss keyboard layout. (I guess never migrating to the US layout is top-3 reason I'll never be a 10x dev.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, in Windows Alt-Tab is application switching (like cmd-tab in Mac)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right, my bad!
crates/viewer/re_recording_panel/src/recording_panel_command.rs
Outdated
Show resolved
Hide resolved
# Conflicts: # crates/viewer/re_viewer/src/app.rs
|
had a huddle with @abey79: we have now 2 ways going between recordings, by ↑ and ↓ and CMD+[ /CMD+] .
With this we keep it, and add a separate task for keeping a selection. |
Related
What
Adds global hotkeys to switch through everything in the recording panel (except for the homepage and anything without it's own page, e.g. app ids).
This was surprisingly deep, the implementation feels quite complex but is the best thing I could come up with. Its the result of the following things:
UiCommandfor discoverability (so it can be found in the command palette)=> We introduce a new
RecordingPanelCommandthat can be send from aUiCommandand executed in the context of the recording panel. The latter is now a struct to hold the command queue.Note: