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

The keyboard shortcut of zoom in does not work on the JIS keyboard #3626

Closed
Umio-Yasuno opened this issue Nov 24, 2023 · 3 comments · Fixed by #3769
Closed

The keyboard shortcut of zoom in does not work on the JIS keyboard #3626

Umio-Yasuno opened this issue Nov 24, 2023 · 3 comments · Fixed by #3769
Labels
bug Something is broken

Comments

@Umio-Yasuno
Copy link

Umio-Yasuno commented Nov 24, 2023

Describe the bug
The keyboard shortcut of zoom in (Ctrl+PlusEqual) does not work on the JIS keyboard layout.
On the JIS keyboard layout, we need the shift key to type + (Shift+;) or = (Shift+-).

https://en.wikipedia.org/wiki/Japanese_input_method
https://en.wikipedia.org/wiki/Keyboard_layout#Japanese

I have confirmed that this issue will be fixed with the following patch.

diff --git a/crates/egui/src/gui_zoom.rs b/crates/egui/src/gui_zoom.rs
index fb4f9bd..9cb8165 100644
--- a/crates/egui/src/gui_zoom.rs
+++ b/crates/egui/src/gui_zoom.rs
@@ -8,6 +8,8 @@ pub mod kb_shortcuts {

     pub const ZOOM_IN: KeyboardShortcut =
         KeyboardShortcut::new(Modifiers::COMMAND, Key::PlusEquals);
+    pub const ZOOM_IN_WITH_SHIFT: KeyboardShortcut =
+        KeyboardShortcut::new(Modifiers::COMMAND.plus(Modifiers::SHIFT), Key::PlusEquals);
     pub const ZOOM_OUT: KeyboardShortcut = KeyboardShortcut::new(Modifiers::COMMAND, Key::Minus);
     pub const ZOOM_RESET: KeyboardShortcut = KeyboardShortcut::new(Modifiers::COMMAND, Key::Num0);
 }
@@ -21,7 +23,10 @@ pub(crate) fn zoom_with_keyboard(ctx: &Context) {
     if ctx.input_mut(|i| i.consume_shortcut(&kb_shortcuts::ZOOM_RESET)) {
         ctx.set_zoom_factor(1.0);
     } else {
-        if ctx.input_mut(|i| i.consume_shortcut(&kb_shortcuts::ZOOM_IN)) {
+        if ctx.input_mut(|i| {
+            i.consume_shortcut(&kb_shortcuts::ZOOM_IN)
+                || i.consume_shortcut(&kb_shortcuts::ZOOM_IN_WITH_SHIFT)
+        }) {
             zoom_in(ctx);
         }
         if ctx.input_mut(|i| i.consume_shortcut(&kb_shortcuts::ZOOM_OUT)) {
@Umio-Yasuno Umio-Yasuno added the bug Something is broken label Nov 24, 2023
@emilk emilk added this to the 0.24.1 milestone Nov 27, 2023
@emilk
Copy link
Owner

emilk commented Nov 27, 2023

Thanks for the report!

I wonder what a good an general solution to this would be.

Sometimes you want to ignore the SHIFT key when matching a KeyboardShortcut, and sometimes not. In particular, you want to ignore it for Keys that are not always found on a normal keyboard, so that when check for Key::Plus we would ignore SHIFT, but not when checking for Key::A.

In any case, it would be a breaking change

@emilk
Copy link
Owner

emilk commented Jan 4, 2024

Please try #3769 and see if it helps!

@Umio-Yasuno
Copy link
Author

Please try #3769 and see if it helps!

I confirmed that the ZOOM_IN and ZOOM_OUT shortcuts work in #3769.
Thanks!

[2024-01-04T23:06:26Z DEBUG egui_winit::clipboard] Initializing arboard clipboard…
[2024-01-04T23:06:26Z DEBUG egui_winit::clipboard] Cannot init smithay clipboard: the 'wayland' feature of 'egui-winit' is not enabled
[2024-01-04T23:06:28Z TRACE egui_winit] Unknown key: Control
[2024-01-04T23:06:28Z TRACE egui_winit] logical Named(Control) -> None,  physical Code(ControlLeft) -> None
[2024-01-04T23:06:28Z TRACE egui_winit] Unknown key: Shift
[2024-01-04T23:06:28Z TRACE egui_winit] logical Named(Shift) -> None,  physical Code(ShiftLeft) -> None
[2024-01-04T23:06:29Z TRACE egui_winit] logical Character("=") -> Some(Equals),  physical Code(Minus) -> Some(Minus)
[2024-01-04T23:06:29Z TRACE egui_winit] logical Character("=") -> Some(Equals),  physical Code(Minus) -> Some(Minus)
[2024-01-04T23:06:29Z TRACE egui_winit] logical Character("=") -> Some(Equals),  physical Code(Minus) -> Some(Minus)
[2024-01-04T23:06:30Z TRACE egui_winit] logical Character("=") -> Some(Equals),  physical Code(Minus) -> Some(Minus)
[2024-01-04T23:06:31Z TRACE egui_winit] Unknown key: Shift
[2024-01-04T23:06:31Z TRACE egui_winit] logical Named(Shift) -> None,  physical Code(ShiftLeft) -> None
[2024-01-04T23:06:31Z TRACE egui_winit] logical Character("-") -> Some(Minus),  physical Code(Minus) -> Some(Minus)
[2024-01-04T23:06:31Z TRACE egui_winit] logical Character("-") -> Some(Minus),  physical Code(Minus) -> Some(Minus)
[2024-01-04T23:06:31Z TRACE egui_winit] logical Character("-") -> Some(Minus),  physical Code(Minus) -> Some(Minus)
[2024-01-04T23:06:32Z TRACE egui_winit] logical Character("-") -> Some(Minus),  physical Code(Minus) -> Some(Minus)
[2024-01-04T23:06:32Z TRACE egui_winit] Unknown key: Shift
[2024-01-04T23:06:32Z TRACE egui_winit] logical Named(Shift) -> None,  physical Code(ShiftLeft) -> None
[2024-01-04T23:06:33Z TRACE egui_winit] logical Character("+") -> Some(Plus),  physical Code(Semicolon) -> Some(Semicolon)
[2024-01-04T23:06:33Z TRACE egui_winit] logical Character("+") -> Some(Plus),  physical Code(Semicolon) -> Some(Semicolon)
[2024-01-04T23:06:33Z TRACE egui_winit] logical Character("+") -> Some(Plus),  physical Code(Semicolon) -> Some(Semicolon)
[2024-01-04T23:06:33Z TRACE egui_winit] logical Character("+") -> Some(Plus),  physical Code(Semicolon) -> Some(Semicolon)
[2024-01-04T23:06:34Z TRACE egui_winit] Unknown key: Shift
[2024-01-04T23:06:34Z TRACE egui_winit] logical Named(Shift) -> None,  physical Code(ShiftLeft) -> None
[2024-01-04T23:06:34Z TRACE egui_winit] logical Character("0") -> Some(Num0),  physical Code(Digit0) -> Some(Num0)
[2024-01-04T23:06:34Z TRACE egui_winit] logical Character("0") -> Some(Num0),  physical Code(Digit0) -> Some(Num0)
[2024-01-04T23:06:34Z TRACE egui_winit] Unknown key: Control
[2024-01-04T23:06:34Z TRACE egui_winit] logical Named(Control) -> None,  physical Code(ControlLeft) -> None

emilk added a commit that referenced this issue Jan 5, 2024
* Closes #3626

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.

@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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants