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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions crates/egui-winit/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,15 @@ impl State {

let logical_key = key_from_winit_key(logical_key);

// Helpful logging to enable when adding new key support
log::trace!(
"logical {:?} -> {:?}, physical {:?} -> {:?}",
event.logical_key,
logical_key,
event.physical_key,
physical_key
);

if let Some(logical_key) = logical_key {
if pressed {
if is_cut_command(self.egui_input.modifiers, logical_key) {
Expand Down Expand Up @@ -1064,9 +1073,8 @@ fn key_from_key_code(key: winit::keyboard::KeyCode) -> Option<egui::Key> {

KeyCode::Minus | KeyCode::NumpadSubtract => Key::Minus,

// Using Mac the key with the Plus sign on it is reported as the Equals key
// (with both English and Swedish keyboard).
KeyCode::Equal | KeyCode::NumpadAdd => Key::PlusEquals,
KeyCode::NumpadAdd => Key::Plus,
KeyCode::Equal => Key::Equals,

KeyCode::Digit0 | KeyCode::Numpad0 => Key::Num0,
KeyCode::Digit1 | KeyCode::Numpad1 => Key::Num1,
Expand Down
110 changes: 96 additions & 14 deletions crates/egui/src/data/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -655,13 +655,68 @@ impl Modifiers {
!self.alt && !self.shift && self.command
}

/// Checks that the `ctrl/cmd` matches, and that the `shift/alt` of the argument is a subset
/// of the pressed ksey (`self`).
///
/// This means that if the pattern has not set `shift`, then `self` can have `shift` set or not.
///
/// The reason is that many logical keys require `shift` or `alt` on some keyboard layouts.
/// For instance, in order to press `+` on an English keyboard, you need to press `shift` and `=`,
/// but a Swedish keyboard has dedicated `+` key.
/// So if you want to make a [`KeyboardShortcut`] looking for `Cmd` + `+`, it makes sense
/// to ignore the shift key.
/// Similarly, the `Alt` key is sometimes used to type special characters.
///
/// However, if the pattern (the argument) explicitly requires the `shift` or `alt` keys
/// to be pressed, then they must be pressed.
///
/// # Example:
/// ```
/// # use egui::Modifiers;
/// # let pressed_modifiers = Modifiers::default();
/// if pressed_modifiers.matches(Modifiers::ALT | Modifiers::SHIFT) {
/// // Alt and Shift are pressed, and nothing else
/// }
/// ```
///
/// ## Behavior:
/// ```
/// # use egui::Modifiers;
/// assert!(Modifiers::CTRL.matches_logically(Modifiers::CTRL));
/// assert!(!Modifiers::CTRL.matches_logically(Modifiers::CTRL | Modifiers::SHIFT));
/// assert!((Modifiers::CTRL | Modifiers::SHIFT).matches_logically(Modifiers::CTRL));
/// assert!((Modifiers::CTRL | Modifiers::COMMAND).matches_logically(Modifiers::CTRL));
/// assert!((Modifiers::CTRL | Modifiers::COMMAND).matches_logically(Modifiers::COMMAND));
/// assert!((Modifiers::MAC_CMD | Modifiers::COMMAND).matches_logically(Modifiers::COMMAND));
/// assert!(!Modifiers::COMMAND.matches_logically(Modifiers::MAC_CMD));
/// ```
pub fn matches_logically(&self, pattern: Modifiers) -> bool {
if pattern.alt && !self.alt {
return false;
}
if pattern.shift && !self.shift {
return false;
}

self.cmd_ctrl_matches(pattern)
}

/// Check for equality but with proper handling of [`Self::command`].
///
/// `self` here are the currently pressed modifiers,
/// and the argument the pattern we are testing for.
///
/// Note that this will require the `shift` and `alt` keys match, even though
/// these modifiers are sometimes required to produce some logical keys.
/// For instance, to press `+` on an English keyboard, you need to press `shift` and `=`,
/// but on a Swedish keyboard you can press the dedicated `+` key.
/// Therefore, you often want to use [`Self::matches_logically`] instead.
///
/// # Example:
/// ```
/// # use egui::Modifiers;
/// # let current_modifiers = Modifiers::default();
/// if current_modifiers.matches(Modifiers::ALT | Modifiers::SHIFT) {
/// # let pressed_modifiers = Modifiers::default();
/// if pressed_modifiers.matches(Modifiers::ALT | Modifiers::SHIFT) {
/// // Alt and Shift are pressed, and nothing else
/// }
/// ```
Expand All @@ -677,12 +732,28 @@ impl Modifiers {
/// assert!((Modifiers::MAC_CMD | Modifiers::COMMAND).matches(Modifiers::COMMAND));
/// assert!(!Modifiers::COMMAND.matches(Modifiers::MAC_CMD));
/// ```
pub fn matches(&self, pattern: Modifiers) -> bool {
pub fn matches_exact(&self, pattern: Modifiers) -> bool {
// alt and shift must always match the pattern:
if pattern.alt != self.alt || pattern.shift != self.shift {
return false;
}

self.cmd_ctrl_matches(pattern)
}

#[deprecated = "Renamed `matches_exact`, but maybe you want to use `matches_logically` instead"]
pub fn matches(&self, pattern: Modifiers) -> bool {
self.matches_exact(pattern)
}

/// Checks only cmd/ctrl, not alt/shift.
///
/// `self` here are the currently pressed modifiers,
/// and the argument the pattern we are testing for.
///
/// This takes care to properly handle the difference between
/// [`Self::ctrl`], [`Self::command`] and [`Self::mac_cmd`].
pub fn cmd_ctrl_matches(&self, pattern: Modifiers) -> bool {
if pattern.mac_cmd {
// Mac-specific match:
if !self.mac_cmd {
Expand Down Expand Up @@ -897,8 +968,11 @@ pub enum Key {
/// `.`
Period,

/// The for the Plus/Equals key.
PlusEquals,
/// `+`
Plus,

/// `=`
Equals,

/// `;`
Semicolon,
Expand Down Expand Up @@ -1017,7 +1091,8 @@ impl Key {
Self::Comma,
Self::Minus,
Self::Period,
Self::PlusEquals,
Self::Plus,
Self::Equals,
Self::Semicolon,
// Digits:
Self::Num0,
Expand Down Expand Up @@ -1117,7 +1192,8 @@ impl Key {
"Comma" | "," => Self::Comma,
"Minus" | "-" | "−" => Self::Minus,
"Period" | "." => Self::Period,
"Plus" | "+" | "Equals" | "=" => Self::PlusEquals,
"Plus" | "+" => Self::Plus,
"Equals" | "=" => Self::Equals,
"Semicolon" | ";" => Self::Semicolon,

"0" => Self::Num0,
Expand Down Expand Up @@ -1194,7 +1270,8 @@ impl Key {
Key::ArrowRight => "⏵",
Key::ArrowUp => "⏶",
Key::Minus => crate::MINUS_CHAR_STR,
Key::PlusEquals => "+",
Key::Plus => "+",
Key::Equals => "=",
_ => self.name(),
}
}
Expand Down Expand Up @@ -1228,7 +1305,8 @@ impl Key {
Key::Comma => "Comma",
Key::Minus => "Minus",
Key::Period => "Period",
Key::PlusEquals => "Plus",
Key::Plus => "Plus",
Key::Equals => "Equals",
Key::Semicolon => "Semicolon",

Key::Num0 => "0",
Expand Down Expand Up @@ -1327,12 +1405,16 @@ fn test_key_from_name() {
#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
pub struct KeyboardShortcut {
pub modifiers: Modifiers,
pub key: Key,

pub logical_key: Key,
}

impl KeyboardShortcut {
pub const fn new(modifiers: Modifiers, key: Key) -> Self {
Self { modifiers, key }
pub const fn new(modifiers: Modifiers, logical_key: Key) -> Self {
Self {
modifiers,
logical_key,
}
}

pub fn format(&self, names: &ModifierNames<'_>, is_mac: bool) -> String {
Expand All @@ -1341,9 +1423,9 @@ impl KeyboardShortcut {
s += names.concat;
}
if names.is_short {
s += self.key.symbol_or_name();
s += self.logical_key.symbol_or_name();
} else {
s += self.key.name();
s += self.logical_key.name();
}
s
}
Expand Down
21 changes: 18 additions & 3 deletions crates/egui/src/gui_zoom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,22 @@ use crate::*;
pub mod kb_shortcuts {
use super::*;

pub const ZOOM_IN: KeyboardShortcut =
KeyboardShortcut::new(Modifiers::COMMAND, Key::PlusEquals);
/// Primary keyboard shortcut for zooming in (`Cmd` + `+`).
pub const ZOOM_IN: KeyboardShortcut = KeyboardShortcut::new(Modifiers::COMMAND, Key::Plus);

/// Secondary keyboard shortcut for zooming in (`Cmd` + `=`).
///
/// On an English keyboard `+` is `Shift` + `=`,
/// but it is annoying to have to press shift.
/// So most browsers also allow `Cmd` + `=` for zooming in.
/// We do the same.
pub const ZOOM_IN_SECONDARY: KeyboardShortcut =
KeyboardShortcut::new(Modifiers::COMMAND, Key::Equals);

/// Keyboard shortcut for zooming in (`Cmd` + `-`).
pub const ZOOM_OUT: KeyboardShortcut = KeyboardShortcut::new(Modifiers::COMMAND, Key::Minus);

/// Keyboard shortcut for resetting zoom in (`Cmd` + `0`).
pub const ZOOM_RESET: KeyboardShortcut = KeyboardShortcut::new(Modifiers::COMMAND, Key::Num0);
}

Expand All @@ -21,7 +34,9 @@ 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))
|| ctx.input_mut(|i| i.consume_shortcut(&kb_shortcuts::ZOOM_IN_SECONDARY))
{
zoom_in(ctx);
}
if ctx.input_mut(|i| i.consume_shortcut(&kb_shortcuts::ZOOM_OUT)) {
Expand Down
21 changes: 15 additions & 6 deletions crates/egui/src/input_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,13 @@ impl InputState {
/// Count presses of a key. If non-zero, the presses are consumed, so that this will only return non-zero once.
///
/// Includes key-repeat events.
pub fn count_and_consume_key(&mut self, modifiers: Modifiers, key: Key) -> usize {
///
/// This uses [`Modifiers::matches_logically`] to match modifiers.
/// This means that e.g. the shortcut `Ctrl` + `Key::Plus` will be matched
/// as long as `Ctrl` and `Plus` are pressed, ignoring if
/// `Shift` or `Alt` are also pressed (because those modifiers might
/// be required to produce the logical `Key::Plus`).
pub fn count_and_consume_key(&mut self, modifiers: Modifiers, logical_key: Key) -> usize {
let mut count = 0usize;

self.events.retain(|event| {
Expand All @@ -300,7 +306,7 @@ impl InputState {
modifiers: ev_mods,
pressed: true,
..
} if *ev_key == key && ev_mods.matches(modifiers)
} if *ev_key == logical_key && ev_mods.matches_logically(modifiers)
);

count += is_match as usize;
Expand All @@ -314,8 +320,8 @@ impl InputState {
/// Check for a key press. If found, `true` is returned and the key pressed is consumed, so that this will only return `true` once.
///
/// Includes key-repeat events.
pub fn consume_key(&mut self, modifiers: Modifiers, key: Key) -> bool {
self.count_and_consume_key(modifiers, key) > 0
pub fn consume_key(&mut self, modifiers: Modifiers, logical_key: Key) -> bool {
self.count_and_consume_key(modifiers, logical_key) > 0
}

/// Check if the given shortcut has been pressed.
Expand All @@ -324,8 +330,11 @@ impl InputState {
///
/// Includes key-repeat events.
pub fn consume_shortcut(&mut self, shortcut: &KeyboardShortcut) -> bool {
let KeyboardShortcut { modifiers, key } = *shortcut;
self.consume_key(modifiers, key)
let KeyboardShortcut {
modifiers,
logical_key,
} = *shortcut;
self.consume_key(modifiers, logical_key)
}

/// Was the given key pressed this frame?
Expand Down
7 changes: 4 additions & 3 deletions crates/egui/src/widgets/text_edit/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -990,7 +990,7 @@ fn events(
pressed: true,
modifiers,
..
} if modifiers.matches(Modifiers::COMMAND) => {
} if modifiers.matches_logically(Modifiers::COMMAND) => {
if let Some((undo_ccursor_range, undo_txt)) = state
.undoer
.lock()
Expand All @@ -1007,8 +1007,9 @@ fn events(
pressed: true,
modifiers,
..
} if (modifiers.matches(Modifiers::COMMAND) && *key == Key::Y)
|| (modifiers.matches(Modifiers::SHIFT | Modifiers::COMMAND) && *key == Key::Z) =>
} if (modifiers.matches_logically(Modifiers::COMMAND) && *key == Key::Y)
|| (modifiers.matches_logically(Modifiers::SHIFT | Modifiers::COMMAND)
&& *key == Key::Z) =>
{
if let Some((redo_ccursor_range, redo_txt)) = state
.undoer
Expand Down
Loading