Skip to content
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

Ignore extra SHIFT and ALT when matching modifiers #3769

Merged
merged 5 commits into from Jan 5, 2024

Conversation

emilk
Copy link
Owner

@emilk emilk commented Jan 4, 2024

Basically, egui now ignores extra SHIFT and ALT pressed when matching keyboard shortcuts.
This is because SHIFT and ALT are often requires to produce some logical keys.
For instance, typing + on an English keyboard requires pressing SHIFT =,
so the keyboard shortcut looking for CTRL + should ignore the SHIFT key.

⚠️ When using consume_key and consume_shortcut you should match most specific shortcuts first,
i.e. check for Cmd-Shift-S ("Save as…") before Cmd-S ("Save"),
so that a user pressing Cmd-Shift-S won't trigger the wrong command!


@abey79 You reported problem using Cmd + and Cmd - to zoom - does this fix it for you?

You can run with RUST_LOG=egui_winit=trace cargo run to see a printout of how winit reports the logical and physical keys, and how egui interprets them.

Weirdly, on Mac winit reports SHIFT = as +, but CMD SHIFT = as = (on an English keyboard) so things are… difficult.

@emilk emilk requested a review from abey79 January 4, 2024 21:50
@emilk emilk added bug Something is broken egui labels Jan 4, 2024
@emilk emilk changed the title Improve modifier key matching Ignore extra SHIFT and ALT when matching modifiers Jan 4, 2024
@abey79
Copy link
Collaborator

abey79 commented Jan 5, 2024

Unfortunately no, this doesn't solve the issue for some reason. Here is the trace when I hit:

  • shift-1
  • cmd-shit-1
[2024-01-05T07:58:06Z TRACE egui_winit] Unknown key: Shift
[2024-01-05T07:58:06Z TRACE egui_winit] logical Named(Shift) -> None,  physical Code(ShiftLeft) -> None
[2024-01-05T07:58:07Z TRACE egui_winit] logical Character("+") -> Some(Plus),  physical Code(Digit1) -> Some(Num1)
[2024-01-05T07:58:07Z TRACE egui_winit] logical Character("+") -> Some(Plus),  physical Code(Digit1) -> Some(Num1)
[2024-01-05T07:58:08Z TRACE egui_winit] Unknown key: Shift
[2024-01-05T07:58:08Z TRACE egui_winit] logical Named(Shift) -> None,  physical Code(ShiftLeft) -> None
[2024-01-05T07:58:09Z TRACE egui_winit] Unknown key: Shift
[2024-01-05T07:58:09Z TRACE egui_winit] logical Named(Shift) -> None,  physical Code(ShiftLeft) -> None
[2024-01-05T07:58:09Z TRACE egui_winit] Unknown key: Super
[2024-01-05T07:58:09Z TRACE egui_winit] logical Named(Super) -> None,  physical Code(SuperLeft) -> None
[2024-01-05T07:58:09Z TRACE egui_winit] logical Character("1") -> Some(Num1),  physical Code(Digit1) -> Some(Num1)
[2024-01-05T07:58:09Z TRACE egui_winit] logical Character("1") -> Some(Num1),  physical Code(Digit1) -> Some(Num1)
[2024-01-05T07:58:09Z TRACE egui_winit] Unknown key: Super
[2024-01-05T07:58:09Z TRACE egui_winit] logical Named(Super) -> None,  physical Code(SuperLeft) -> None
[2024-01-05T07:58:09Z TRACE egui_winit] Unknown key: Shift
[2024-01-05T07:58:09Z TRACE egui_winit] logical Named(Shift) -> None,  physical Code(ShiftLeft) -> None

Running on macOS Sonoma 14.1.1

shift-1 is where the + is in the ch-fr keyboard:
image

(Bonus keyboard layout trivia: the weird glyph layout in the "é/ö" keys and neighbours is due to the fact that there are two sub-layouts: swiss-french and swiss-german. The only difference between the two is "éèà" by default / "öäü" with shift for the swiss-french, and the opposite for the swiss-gemeran.)

@emilk
Copy link
Owner Author

emilk commented Jan 5, 2024

Seems like we are hitting this winit bug: rust-windowing/winit#3078

@emilk
Copy link
Owner Author

emilk commented Jan 5, 2024

Fix for winit in rust-windowing/winit#3361

@emilk
Copy link
Owner Author

emilk commented Jan 5, 2024

This should do the right think on Windows and Linux (I hope), but for macOS we have to wait for rust-windowing/winit#3361

@emilk emilk merged commit 9faf4b4 into master Jan 5, 2024
35 checks passed
@emilk emilk deleted the emilk/improve-keyboard-shortcut-matching branch January 5, 2024 09:53
@seanlinsley
Copy link

seanlinsley commented Apr 1, 2024

My application was distinguishing between shortcuts with and without Shift, which this breaks. It also causes surprising behavior:

let change = if i.consume_key(Modifiers::COMMAND | Modifiers::SHIFT, Key::Plus) {
    // correctly captures Command-Shift-Plus, even though it shouldn't anymore?
    10
} else if i.consume_key(Modifiers::COMMAND, Key::Plus) {
    // never triggers
    1
} else if i.consume_key(Modifiers::COMMAND | Modifiers::SHIFT, Key::Minus) {
    // never triggers
    -10
} else if i.consume_key(Modifiers::COMMAND, Key::Minus) {
    // correctly captures Command-Minus
    -1
} else {
    return;
};

If not for consume_key(Modifiers::COMMAND, Key::Plus) no longer working I would be able to replicate the previous behavior by also checking i.modifiers.shift

Possibly related: when upgrading from 0.23.0 to 0.27.1 I had to set zoom_with_keyboard = false to avoid Command-Plus from being turned into a zoom event, despite the consume_key call.

@seanlinsley
Copy link

This almost replicates the previous behavior, except that Command-Shift-Minus doesn't seem to create a InputState event anymore.

let mut change = None;
i.events.retain(|event| {
    match event {
        Event::Key { physical_key: Some(Key::Equals), modifiers, pressed: true, .. } => {
            change = Some(if modifiers.shift { 10 } else { 1 });
            false
        }
        Event::Key { physical_key: Some(Key::Minus), modifiers, pressed: true, .. } => {
            change = Some(if modifiers.shift { -10 } else { -1 });
            false
        }
        _ => true,
    }
});

@emilk
Copy link
Owner Author

emilk commented Apr 2, 2024

Pressing shift and minus at the same time will on my keyboard produce the underscore character: _, i.e not a Key::Minus.

You are trying to check for a physical key, when egui uses logical keys.

See https://docs.rs/egui/latest/egui/enum.Event.html#variant.Key and #3653 for more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken egui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The keyboard shortcut of zoom in does not work on the JIS keyboard
3 participants