Skip to content

Commit

Permalink
Fix disabled widgets "eating" focus (#5370)
Browse files Browse the repository at this point in the history
- fixes #5359

For the test I added a `Harness::press_key` function. We should
eventually add these to kittest, probably via a trait one can implement
for the `Harness` but for now this should do.
  • Loading branch information
lucasmerlin authored Nov 26, 2024
1 parent 83a3006 commit 9ecc0b2
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 2 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

0 comments on commit 9ecc0b2

Please sign in to comment.