Add Ui.input_mut & InputState.ignore_key #1212
Conversation
egui/src/input_state.rs
Outdated
| key, | ||
| modifiers: mods, | ||
| .. | ||
| } if *key == ignore_key && *mods == modifiers |
There was a problem hiding this comment.
This will match both on press and release - I think we should only match on pressed: true
There was a problem hiding this comment.
Hmm, what if code depended on presses always being matched with releases? then you'd have an unmatched release instead of as if the event "never happened"
| } | ||
| egui::Modifiers { | ||
| command: true, | ||
| ..Default::default() |
There was a problem hiding this comment.
We can make this even nicer by switching the argument order and adding some helpers to Modifiers:
if ui.input_mut().consume_key(egui::Modifiers::command(), key) {There was a problem hiding this comment.
nice, i added it as a builder api but lmk if you prefer classmethods + operator overloading instead
if ui.input_mut().consume_key(egui::Modifiers::new().command(true).shift(true), key) {vs
if ui.input_mut().consume_key(egui::Modifiers::command(true) | egui::Modifiers::shift(true), key) {There was a problem hiding this comment.
Another way to do this more generally could be to have a consume_event + some helper to easily make the event from key and modifier combo like Event::from_key(modifier, key) or modifier.event_with_key(key) or viceversa. Not sure if that would need a bunch of refactoring of code that makes the convenience properties from the events to keep the invariants maintained
| } | ||
| } | ||
| } | ||
| toggle_surrounding(code, ccursor_range, surrounding); |
There was a problem hiding this comment.
According to the docs of consume_key, it will return true both when the key is pressed and when it is released, so this should toggle twice, which is not intentional. This is why I think consume_key should only detect presses and ignore releases (or the other way around)
There was a problem hiding this comment.
It only returns true when the key was pressed but it will remove the release event too, so triggering only once. Also in the built app it seems to only trigger once (on Windows & Ubuntu) - do you see differently?
There was a problem hiding this comment.
I haven't run in since last review :)
But then the docs for consume_key should be changed to indicate it is only triggered on press, and also match for pressed: true.
| } | ||
| } | ||
| } | ||
| toggle_surrounding(code, ccursor_range, surrounding); |
There was a problem hiding this comment.
I haven't run in since last review :)
But then the docs for consume_key should be changed to indicate it is only triggered on press, and also match for pressed: true.
Improvements and fixes following #1212
This PR pipes through
ctx.input_mut()through to the Uis, allowing control of what events subsequent widgets see, and adds a utility function to the InputState to do hotkeys/shortcuts.Helps with #1124