Skip to content

Commit

Permalink
Change "Save Selection" command to Cmd+Alt+S (#2631)
Browse files Browse the repository at this point in the history
Closes #2605
* #2605

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/2631) (if
applicable)

- [PR Build Summary](https://build.rerun.io/pr/2631)
- [Docs
preview](https://rerun.io/preview/pr%3Aemilk%2Ffix-overlapping-keyboard-shortcuts/docs)
- [Examples
preview](https://rerun.io/preview/pr%3Aemilk%2Ffix-overlapping-keyboard-shortcuts/examples)
  • Loading branch information
emilk authored Jul 7, 2023
1 parent 2d8fd0a commit 97f154c
Showing 1 changed file with 43 additions and 3 deletions.
46 changes: 43 additions & 3 deletions crates/re_ui/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,8 @@ impl UICommand {
}

#[cfg(not(target_arch = "wasm32"))]
fn cmd_shift(key: Key) -> KeyboardShortcut {
KeyboardShortcut::new(Modifiers::COMMAND.plus(Modifiers::SHIFT), key)
fn cmd_alt(key: Key) -> KeyboardShortcut {
KeyboardShortcut::new(Modifiers::COMMAND.plus(Modifiers::ALT), key)
}

fn ctrl_shift(key: Key) -> KeyboardShortcut {
Expand All @@ -176,7 +176,7 @@ impl UICommand {
#[cfg(not(target_arch = "wasm32"))]
UICommand::Save => Some(cmd(Key::S)),
#[cfg(not(target_arch = "wasm32"))]
UICommand::SaveSelection => Some(cmd_shift(Key::S)),
UICommand::SaveSelection => Some(cmd_alt(Key::S)),
#[cfg(not(target_arch = "wasm32"))]
UICommand::Open => Some(cmd(Key::O)),

Expand Down Expand Up @@ -275,3 +275,43 @@ impl UICommand {
}
}
}

#[test]
fn check_for_clashing_command_shortcuts() {
fn clashes(a: KeyboardShortcut, b: KeyboardShortcut) -> bool {
if a.key != b.key {
return false;
}

if a.modifiers.alt != b.modifiers.alt {
return false;
}

if a.modifiers.shift != b.modifiers.shift {
return false;
}

// On Non-Mac, command is interpreted as ctrl!
(a.modifiers.command || a.modifiers.ctrl) == (b.modifiers.command || b.modifiers.ctrl)
}

use strum::IntoEnumIterator as _;

for a_cmd in UICommand::iter() {
if let Some(a_shortcut) = a_cmd.kb_shortcut() {
for b_cmd in UICommand::iter() {
if a_cmd == b_cmd {
continue;
}
if let Some(b_shortcut) = b_cmd.kb_shortcut() {
assert!(
!clashes(a_shortcut, b_shortcut),
"Command '{a_cmd:?}' and '{b_cmd:?}' have overlapping keyboard shortcuts: {:?} vs {:?}",
a_shortcut.format(&egui::ModifierNames::NAMES, true),
b_shortcut.format(&egui::ModifierNames::NAMES, true),
);
}
}
}
}
}

1 comment on commit 97f154c

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Rust Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.25.

Benchmark suite Current: 97f154c Previous: 2d8fd0a Ratio
batch_points_arrow/encode_log_msg 91122 ns/iter (± 234) 46815 ns/iter (± 865) 1.95

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.