Skip to content

Commit

Permalink
Fix disabled widgets "eating" focus
Browse files Browse the repository at this point in the history
fixes #5359
  • Loading branch information
lucasmerlin committed Nov 14, 2024
1 parent 3c7ad0e commit 01faa55
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 13 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1229,6 +1229,7 @@ dependencies = [
"ahash",
"backtrace",
"document-features",
"egui_kittest",
"emath",
"epaint",
"log",
Expand Down
4 changes: 4 additions & 0 deletions crates/egui/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -98,3 +98,7 @@ log = { workspace = true, optional = true }
puffin = { workspace = true, optional = true }
ron = { workspace = true, optional = true }
serde = { workspace = true, optional = true, features = ["derive", "rc"] }


[dev-dependencies]
egui_kittest = { workspace = true, features = ["wgpu", "snapshot"] }
6 changes: 4 additions & 2 deletions crates/egui/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1160,6 +1160,8 @@ impl Context {
/// same widget, then `allow_focus` should only be true once (like in [`Ui::new`] (true) and [`Ui::remember_min_rect`] (false)).
#[allow(clippy::too_many_arguments)]
pub(crate) fn create_widget(&self, w: WidgetRect, allow_focus: bool) -> Response {
let interested_in_focus = w.enabled && w.sense.focusable && w.layer_id.allow_interaction();

// Remember this widget
self.write(|ctx| {
let viewport = ctx.viewport();
Expand All @@ -1169,12 +1171,12 @@ impl Context {
// but also to know when we have reached the widget we are checking for cover.
viewport.this_pass.widgets.insert(w.layer_id, w);

if allow_focus && w.sense.focusable {
if allow_focus && interested_in_focus {
ctx.memory.interested_in_focus(w.id);
}
});

if allow_focus && (!w.enabled || !w.sense.focusable || !w.layer_id.allow_interaction()) {
if allow_focus && !interested_in_focus {
// Not interested or allowed input:
self.memory_mut(|mem| mem.surrender_focus(w.id));
}
Expand Down
30 changes: 30 additions & 0 deletions crates/egui/tests/regression_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
use egui::Button;
use egui_kittest::kittest::Queryable;
use egui_kittest::Harness;

#[test]
pub fn focus_should_skip_over_disabled_buttons() {
let mut harness = Harness::new_ui(|ui| {
ui.add(Button::new("Button 1"));
ui.add_enabled(false, Button::new("Button Disabled"));
ui.add(Button::new("Button 3"));
});

harness.press_key(egui::Key::Tab);
harness.run();

let button_1 = harness.get_by_name("Button 1");
assert!(button_1.is_focused());

harness.press_key(egui::Key::Tab);
harness.run();

let button_3 = harness.get_by_name("Button 3");
assert!(button_3.is_focused());

harness.press_key(egui::Key::Tab);
harness.run();

let button_1 = harness.get_by_name("Button 1");
assert!(button_1.is_focused());
}
19 changes: 19 additions & 0 deletions crates/egui_kittest/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,25 @@ impl<'a, State> Harness<'a, State> {
pub fn state_mut(&mut self) -> &mut State {
&mut self.state
}

/// Press a key.
/// This will create a key down event and a key up event.
pub fn press_key(&mut self, key: egui::Key) {
self.input.events.push(egui::Event::Key {
key,
pressed: true,
modifiers: Default::default(),
repeat: false,
physical_key: None,
});
self.input.events.push(egui::Event::Key {
key,
pressed: false,
modifiers: Default::default(),
repeat: false,
physical_key: None,
});
}
}

/// Utilities for stateless harnesses.
Expand Down
20 changes: 9 additions & 11 deletions examples/hello_world_simple/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#![allow(rustdoc::missing_crate_level_docs)] // it's an example

use eframe::egui;
use eframe::egui::Button;

fn main() -> eframe::Result {
env_logger::init(); // Log to stderr (if you run with `RUST_LOG=debug`).
Expand All @@ -15,19 +16,16 @@ fn main() -> eframe::Result {
let mut name = "Arthur".to_owned();
let mut age = 42;

let mut disabled = true;

eframe::run_simple_native("My egui App", options, move |ctx, _frame| {
egui::CentralPanel::default().show(ctx, |ui| {
ui.heading("My egui Application");
ui.horizontal(|ui| {
let name_label = ui.label("Your name: ");
ui.text_edit_singleline(&mut name)
.labelled_by(name_label.id);
});
ui.add(egui::Slider::new(&mut age, 0..=120).text("age"));
if ui.button("Increment").clicked() {
age += 1;
}
ui.label(format!("Hello '{name}', age {age}"));
ui.button("Button 1");
ui.button("Button 2");

ui.add_enabled(false, Button::new("Button Disabled"));

ui.button("Button 3");
});
})
}

0 comments on commit 01faa55

Please sign in to comment.