Fix modifiers not working in kittest

RELEASES.md

@@ -74,13 +74,15 @@ I usually do this all on the `master` branch, but doing it in a release branch i
 (cd crates/egui                 && cargo publish --quiet)  &&  echo "✅ egui"
 (cd crates/egui-winit           && cargo publish --quiet)  &&  echo "✅ egui-winit"
 (cd crates/egui-wgpu            && cargo publish --quiet)  &&  echo "✅ egui-wgpu"
+(cd crates/eframe               && cargo publish --quiet)  &&  echo "✅ eframe"
 (cd crates/egui_kittest         && cargo publish --quiet)  &&  echo "✅ egui_kittest"
 (cd crates/egui_extras          && cargo publish --quiet)  &&  echo "✅ egui_extras"
 (cd crates/egui_demo_lib        && cargo publish --quiet)  &&  echo "✅ egui_demo_lib"
 (cd crates/egui_glow            && cargo publish --quiet)  &&  echo "✅ egui_glow"
-(cd crates/eframe               && cargo publish --quiet)  &&  echo "✅ eframe"
 ```
 
+\<continue with the checklist above\>
+
 ## Announcements
 * [ ] [Bluesky](https://bsky.app/profile/ernerfeldt.bsky.social)
 * [ ] egui discord

crates/egui_kittest/src/event.rs

@@ -4,12 +4,26 @@ use kittest::{ElementState, MouseButton, SimulatedEvent};
 
 #[derive(Default)]
 pub(crate) struct EventState {
-    modifiers: Modifiers,
     last_mouse_pos: Pos2,
 }
 
 impl EventState {
-    pub fn kittest_event_to_egui(&mut self, event: kittest::Event) -> Option<egui::Event> {
+    /// Map the kittest events to egui events, add them to the input and update the modifiers.
+    /// This function accesses `egui::RawInput::modifiers`, make sure it is not reset after each
+    /// frame (Since we use [`egui::RawInput::take`], this should be fine).
+    pub fn update(&mut self, events: Vec<kittest::Event>, input: &mut egui::RawInput) {
+        for event in events {
+            if let Some(event) = self.kittest_event_to_egui(&mut input.modifiers, event) {
+                input.events.push(event);
+            }
+        }
+    }
+
+    fn kittest_event_to_egui(
+        &mut self,
+        modifiers: &mut Modifiers,
+        event: kittest::Event,
+    ) -> Option<egui::Event> {
         match event {
             kittest::Event::ActionRequest(e) => Some(Event::AccessKitActionRequest(e)),
             kittest::Event::Simulated(e) => match e {
@@ -23,7 +37,7 @@ impl EventState {
                 SimulatedEvent::MouseInput { state, button } => {
                     pointer_button_to_egui(button).map(|button| PointerButton {
                         button,
-                        modifiers: self.modifiers,
+                        modifiers: *modifiers,
                         pos: self.last_mouse_pos,
                         pressed: matches!(state, ElementState::Pressed),
                     })
@@ -32,22 +46,22 @@ impl EventState {
                 SimulatedEvent::KeyInput { state, key } => {
                     match key {
                         kittest::Key::Alt => {
-                            self.modifiers.alt = matches!(state, ElementState::Pressed);
+                            modifiers.alt = matches!(state, ElementState::Pressed);
                         }
                         kittest::Key::Command => {
-                            self.modifiers.command = matches!(state, ElementState::Pressed);
+                            modifiers.command = matches!(state, ElementState::Pressed);
                         }
                         kittest::Key::Control => {
-                            self.modifiers.ctrl = matches!(state, ElementState::Pressed);
+                            modifiers.ctrl = matches!(state, ElementState::Pressed);
                         }
                         kittest::Key::Shift => {
-                            self.modifiers.shift = matches!(state, ElementState::Pressed);
+                            modifiers.shift = matches!(state, ElementState::Pressed);
                         }
                         _ => {}
                     }
                     kittest_key_to_egui(key).map(|key| Event::Key {
                         key,
-                        modifiers: self.modifiers,
+                        modifiers: *modifiers,
                         pressed: matches!(state, ElementState::Pressed),
                         repeat: false,
                         physical_key: None,
@@ -58,7 +72,7 @@ impl EventState {
     }
 }
 
-pub fn kittest_key_to_egui(value: kittest::Key) -> Option<egui::Key> {
+fn kittest_key_to_egui(value: kittest::Key) -> Option<egui::Key> {
     use egui::Key as EKey;
     use kittest::Key;
     match value {
@@ -170,7 +184,7 @@ pub fn kittest_key_to_egui(value: kittest::Key) -> Option<egui::Key> {
     }
 }
 
-pub fn pointer_button_to_egui(value: MouseButton) -> Option<egui::PointerButton> {
+fn pointer_button_to_egui(value: MouseButton) -> Option<egui::PointerButton> {
     match value {
         MouseButton::Left => Some(egui::PointerButton::Primary),
         MouseButton::Right => Some(egui::PointerButton::Secondary),

crates/egui_kittest/src/lib.rs

@@ -11,6 +11,7 @@ mod snapshot;
 #[cfg(feature = "snapshot")]
 pub use snapshot::*;
 use std::fmt::{Debug, Display, Formatter};
+use std::mem;
 use std::time::Duration;
 
 mod app_kind;
@@ -227,11 +228,8 @@ impl<'a, State> Harness<'a, State> {
     }
 
     fn _step(&mut self, sizing_pass: bool) {
-        for event in self.kittest.take_events() {
-            if let Some(event) = self.event_state.kittest_event_to_egui(event) {
-                self.input.events.push(event);
-            }
-        }
+        self.event_state
+            .update(self.kittest.take_events(), &mut self.input);
 
         self.input.predicted_dt = self.step_dt;
 
@@ -376,26 +374,49 @@ impl<'a, State> Harness<'a, 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.press_key_modifiers(Modifiers::default(), key);
+        self.input.events.push(egui::Event::Key {
+            key,
+            pressed: true,
+            modifiers: self.input.modifiers,
+            repeat: false,
+            physical_key: None,
+        });
+        self.input.events.push(egui::Event::Key {
+            key,
+            pressed: false,
+            modifiers: self.input.modifiers,
+            repeat: false,
+            physical_key: None,
+        });
     }
 
     /// Press a key with modifiers.
-    /// This will create a key down event and a key up event.
+    /// This will create a key down event, a key up event and update the modifiers.
+    ///
+    /// NOTE: In contrast to the event fns on [`Node`], this will call [`Harness::step`], in
+    /// order to properly update modifiers.
     pub fn press_key_modifiers(&mut self, modifiers: Modifiers, key: egui::Key) {
+        // Combine the modifiers with the current modifiers
+        let modifiers = modifiers | self.input.modifiers;
+        let previous_modifiers = mem::replace(&mut self.input.modifiers, modifiers);
+
         self.input.events.push(egui::Event::Key {
             key,
             pressed: true,
             modifiers,
             repeat: false,
             physical_key: None,
         });
+        self.step();
         self.input.events.push(egui::Event::Key {
             key,
             pressed: false,
             modifiers,
             repeat: false,
             physical_key: None,
         });
+
+        self.input.modifiers = previous_modifiers;
     }
 
     /// Render the last output to an image.

crates/egui_kittest/tests/tests.rs

@@ -1,4 +1,6 @@
-use egui_kittest::{Harness, SnapshotResults};
+use egui::Modifiers;
+use egui_kittest::Harness;
+use kittest::{Key, Queryable};
 
 #[test]
 fn test_shrink() {
@@ -10,8 +12,45 @@ fn test_shrink() {
 
     harness.fit_contents();
 
-    let mut results = SnapshotResults::new();
-
     #[cfg(all(feature = "snapshot", feature = "wgpu"))]
-    results.add(harness.try_snapshot("test_shrink"));
+    harness.snapshot("test_shrink");
+}
+
+#[test]
+fn test_modifiers() {
+    #[derive(Default)]
+    struct State {
+        cmd_clicked: bool,
+        cmd_z_pressed: bool,
+    }
+    let mut harness = Harness::new_ui_state(
+        |ui, state| {
+            if ui.button("Click me").clicked() && ui.input(|i| i.modifiers.command) {
+                state.cmd_clicked = true;
+            }
+            if ui.input(|i| i.modifiers.command && i.key_pressed(egui::Key::Z)) {
+                state.cmd_z_pressed = true;
+            }
+        },
+        State::default(),
+    );
+
+    harness.get_by_label("Click me").key_down(Key::Command);
+    // This run isn't necessary, but allows us to test whether modifiers are remembered between frames
+    harness.run();
+    harness.get_by_label("Click me").click();
+    // TODO(lucasmerlin): Right now the key_up needs to happen on a separate frame or it won't register.
+    // This should be more intuitive
+    harness.run();
+    harness.get_by_label("Click me").key_up(Key::Command);
+
+    harness.run();
+
+    harness.press_key_modifiers(Modifiers::COMMAND, egui::Key::Z);
+    // TODO(lucasmerlin): This should also work (Same problem as above)
+    // harness.node().key_combination(&[Key::Command, Key::Z]);
+
+    let state = harness.state();
+    assert!(state.cmd_clicked, "The button wasn't command-clicked");
+    assert!(state.cmd_z_pressed, "Cmd+Z wasn't pressed");
 }