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

Add Ui.input_mut & InputState.ignore_key #1212

Merged
merged 15 commits into from Feb 15, 2022
2 changes: 1 addition & 1 deletion CHANGELOG.md
Expand Up @@ -8,7 +8,7 @@ NOTE: [`epaint`](epaint/CHANGELOG.md), [`eframe`](eframe/CHANGELOG.md), [`egui_w
## Unreleased

### Added ⭐
* `Ui::input_mut` to modify how subsequent widgets see the `InputState` and a convenience method `InputState::ignore_key` for shortcuts or hotkeys ([#1212](https://github.com/emilk/egui/pull/1212)).
* `Ui::input_mut` to modify how subsequent widgets see the `InputState` and a convenience method `InputState::consume_key` for shortcuts or hotkeys ([#1212](https://github.com/emilk/egui/pull/1212)).
* Much improved font selection ([#1154](https://github.com/emilk/egui/pull/1154)):
* You can now select any font size and family using `RichText::size` amd `RichText::family` and the new `FontId`.
* Easily change text styles with `Style::text_styles`.
Expand Down
65 changes: 17 additions & 48 deletions egui_demo_lib/src/easy_mark/easy_mark_editor.rs
Expand Up @@ -117,59 +117,28 @@ impl EasyMarkEditor {

fn shortcuts(ui: &Ui, code: &mut dyn TextBuffer, ccursor_range: &mut CCursorRange) -> bool {
let mut any_change = false;
for event in &ui.input().events {
if let Event::Key {
for (key, surrounding) in [
(Key::B, "*"),
(Key::C, "`"),
(Key::I, "/"),
(Key::R, "~"),
(Key::U, "_"),
cat-state marked this conversation as resolved.
Show resolved Hide resolved
] {
if ui.input_mut().consume_key(
key,
pressed: true,
modifiers,
} = event
{
if modifiers.command_only() {
match &key {
// toggle *bold*
Key::B => {
toggle_surrounding(code, ccursor_range, "*");
any_change = true;
}
// toggle `code`
Key::C => {
toggle_surrounding(code, ccursor_range, "`");
any_change = true;
}
// toggle /italics/
Key::I => {
toggle_surrounding(code, ccursor_range, "/");
any_change = true;
}
// toggle $lowered$
Key::L => {
toggle_surrounding(code, ccursor_range, "$");
any_change = true;
}
// toggle ^raised^
Key::R => {
toggle_surrounding(code, ccursor_range, "^");
any_change = true;
}
// toggle ~strikethrough~
Key::S => {
toggle_surrounding(code, ccursor_range, "~");
any_change = true;
}
// toggle _underline_
Key::U => {
toggle_surrounding(code, ccursor_range, "_");
any_change = true;
}
_ => {}
}
}
}
egui::Modifiers {
command: true,
..Default::default()
Copy link
Owner

Choose a reason for hiding this comment

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

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) {

Copy link
Contributor Author

@cat-state cat-state Feb 6, 2022

Choose a reason for hiding this comment

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

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) {

Copy link
Contributor Author

@cat-state cat-state Feb 12, 2022

Choose a reason for hiding this comment

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

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);
Copy link
Owner

Choose a reason for hiding this comment

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

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)

Copy link
Contributor Author

@cat-state cat-state Feb 13, 2022

Choose a reason for hiding this comment

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

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?

Copy link
Owner

Choose a reason for hiding this comment

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

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.

any_change = true;
};
}
any_change
}

/// E.g. toggle *strong* with `toggle(&mut text, &mut cursor, "*")`
/// E.g. toggle *strong* with `toggle_surrounding(&mut text, &mut cursor, "*")`
fn toggle_surrounding(
code: &mut dyn TextBuffer,
ccursor_range: &mut CCursorRange,
Expand Down