From 9ecc0b232c02107609d38c517099fc97fe06da4b Mon Sep 17 00:00:00 2001
From: lucasmerlin <lucasmeurer96@gmail.com>
Date: Tue, 26 Nov 2024 14:31:30 +0100
Subject: [PATCH] Fix disabled widgets "eating" focus (#5370)

- fixes https://github.com/emilk/egui/issues/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.
---
 Cargo.lock                            |  1 +
 crates/egui/Cargo.toml                |  4 ++++
 crates/egui/src/context.rs            |  6 ++++--
 crates/egui/tests/regression_tests.rs | 30 +++++++++++++++++++++++++++
 crates/egui_kittest/src/lib.rs        | 19 +++++++++++++++++
 5 files changed, 58 insertions(+), 2 deletions(-)
 create mode 100644 crates/egui/tests/regression_tests.rs

diff --git a/Cargo.lock b/Cargo.lock
index 000165018c6..db2ed667e9f 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1229,6 +1229,7 @@ dependencies = [
  "ahash",
  "backtrace",
  "document-features",
+ "egui_kittest",
  "emath",
  "epaint",
  "log",
diff --git a/crates/egui/Cargo.toml b/crates/egui/Cargo.toml
index 487a4eac6e9..d000c0da5d6 100644
--- a/crates/egui/Cargo.toml
+++ b/crates/egui/Cargo.toml
@@ -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"] }
diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs
index 331430c7865..99a1b536258 100644
--- a/crates/egui/src/context.rs
+++ b/crates/egui/src/context.rs
@@ -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();
@@ -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));
         }
diff --git a/crates/egui/tests/regression_tests.rs b/crates/egui/tests/regression_tests.rs
new file mode 100644
index 00000000000..b24235713b5
--- /dev/null
+++ b/crates/egui/tests/regression_tests.rs
@@ -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());
+}
diff --git a/crates/egui_kittest/src/lib.rs b/crates/egui_kittest/src/lib.rs
index 8e410d84d11..2c75732bc7d 100644
--- a/crates/egui_kittest/src/lib.rs
+++ b/crates/egui_kittest/src/lib.rs
@@ -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.