Fix press_key_modifiers and test for more edge cases

crates/egui_kittest/src/event.rs

@@ -4,22 +4,26 @@ use kittest::{ElementState, MouseButton, SimulatedEvent};
 
 #[derive(Default)]
 pub(crate) struct EventState {
-    modifiers: Modifiers,
     last_mouse_pos: Pos2,
 }
 
 impl EventState {
     /// 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(event) {
+            if let Some(event) = self.kittest_event_to_egui(&mut input.modifiers, event) {
                 input.events.push(event);
             }
         }
-        input.modifiers = self.modifiers;
     }
 
-    fn kittest_event_to_egui(&mut self, event: kittest::Event) -> Option<egui::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 {
@@ -33,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),
                     })
@@ -42,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,

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;
@@ -373,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,3 +1,4 @@
+use egui::Modifiers;
 use egui_kittest::Harness;
 use kittest::{Key, Queryable};
 
@@ -17,16 +18,26 @@ fn 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, cmd_clicked| {
+        |ui, state| {
             if ui.button("Click me").clicked() && ui.input(|i| i.modifiers.command) {
-                *cmd_clicked = true;
+                state.cmd_clicked = true;
+            }
+            if ui.input(|i| i.modifiers.command && i.key_pressed(egui::Key::Z)) {
+                state.cmd_z_pressed = true;
             }
         },
-        false,
+        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
@@ -35,5 +46,11 @@ fn test_modifiers() {
 
     harness.run();
 
-    assert!(harness.state(), "The button wasn't command-clicked");
+    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");
 }