Fix keyboard support in DragValue (#2342)

* Enable incrementing and decrementing `DragValue` with the keyboard * As soon as a DragValue is focused, render it in edit mode * Simpler, more reliable approach to managing the drag value's edit string * Add changelog entry * Update doc comment Co-authored-by: Emil Ernerfeldt <[email protected]> * Add comment explaining why we don't listen for left/right arrow Co-authored-by: Emil Ernerfeldt <[email protected]>
emilk · Dec 2, 2022 · 2793de5 · 2793de5
1 parent e8dab33
commit 2793de5
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 33 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -33,6 +33,7 @@ NOTE: [`epaint`](crates/epaint/CHANGELOG.md), [`eframe`](crates/eframe/CHANGELOG
 * Improved text rendering ([#2071](https://github.com/emilk/egui/pull/2071)).
 * Less jitter when calling `Context::set_pixels_per_point` ([#2239](https://github.com/emilk/egui/pull/2239)).
 * Fixed popups and color edit going outside the screen.
+* Fixed keyboard support in `DragValue`. ([#2342](https://github.com/emilk/egui/pull/2342)).
 
 
 ## 0.19.0 - 2022-08-20

diff --git a/crates/egui/src/input_state.rs b/crates/egui/src/input_state.rs
@@ -245,9 +245,9 @@ impl InputState {
         self.pointer.wants_repaint() || self.scroll_delta != Vec2::ZERO || !self.events.is_empty()
     }
 
-    /// Check for a key press. If found, `true` is returned and the key pressed is consumed, so that this will only return `true` once.
-    pub fn consume_key(&mut self, modifiers: Modifiers, key: Key) -> bool {
-        let mut match_found = false;
+    /// Count presses of a key. If non-zero, the presses are consumed, so that this will only return non-zero once.
+    pub fn count_and_consume_key(&mut self, modifiers: Modifiers, key: Key) -> usize {
+        let mut count = 0usize;
 
         self.events.retain(|event| {
             let is_match = matches!(
@@ -259,12 +259,17 @@ impl InputState {
                 } if *ev_key == key && ev_mods.matches(modifiers)
             );
 
-            match_found |= is_match;
+            count += is_match as usize;
 
             !is_match
         });
 
-        match_found
+        count
+    }
+
+    /// Check for a key press. If found, `true` is returned and the key pressed is consumed, so that this will only return `true` once.
+    pub fn consume_key(&mut self, modifiers: Modifiers, key: Key) -> bool {
+        self.count_and_consume_key(modifiers, key) > 0
     }
 
     /// Check if the given shortcut has been pressed.

diff --git a/crates/egui/src/memory.rs b/crates/egui/src/memory.rs
@@ -400,8 +400,13 @@ impl Memory {
 
     /// Register this widget as being interested in getting keyboard focus.
     /// This will allow the user to select it with tab and shift-tab.
+    /// This is normally done automatically when handling interactions,
+    /// but it is sometimes useful to pre-register interest in focus,
+    /// e.g. before deciding which type of underlying widget to use,
+    /// as in the [`crate::DragValue`] widget, so a widget can be focused
+    /// and rendered correctly in a single frame.
     #[inline(always)]
-    pub(crate) fn interested_in_focus(&mut self, id: Id) {
+    pub fn interested_in_focus(&mut self, id: Id) {
         self.interaction.focus.interested_in_focus(id);
     }
 

diff --git a/crates/egui/src/widgets/drag_value.rs b/crates/egui/src/widgets/drag_value.rs
@@ -371,18 +371,49 @@ impl<'a> Widget for DragValue<'a> {
         let shift = ui.input().modifiers.shift_only();
         let is_slow_speed = shift && ui.memory().is_being_dragged(ui.next_auto_id());
 
+        let kb_edit_id = ui.next_auto_id();
+        // The following call ensures that when a `DragValue` receives focus,
+        // it is immediately rendered in edit mode, rather than being rendered
+        // in button mode for just one frame. This is important for
+        // screen readers.
+        ui.memory().interested_in_focus(kb_edit_id);
+        let is_kb_editing = ui.memory().has_focus(kb_edit_id);
+
         let old_value = get(&mut get_set_value);
-        let value = clamp_to_range(old_value, clamp_range.clone());
-        if old_value != value {
-            set(&mut get_set_value, value);
-        }
+        let mut value = old_value;
         let aim_rad = ui.input().aim_radius() as f64;
 
         let auto_decimals = (aim_rad / speed.abs()).log10().ceil().clamp(0.0, 15.0) as usize;
         let auto_decimals = auto_decimals + is_slow_speed as usize;
-
         let max_decimals = max_decimals.unwrap_or(auto_decimals + 2);
         let auto_decimals = auto_decimals.clamp(min_decimals, max_decimals);
+
+        if is_kb_editing {
+            let mut input = ui.input_mut();
+            // This deliberately doesn't listen for left and right arrow keys,
+            // because when editing, these are used to move the caret.
+            // This behavior is consistent with other editable spinner/stepper
+            // implementations, such as Chromium's (for HTML5 number input).
+            // It is also normal for such controls to go directly into edit mode
+            // when they receive keyboard focus, and some screen readers
+            // assume this behavior, so having a separate mode for incrementing
+            // and decrementing, that supports all arrow keys, would be
+            // problematic.
+            let change = input.count_and_consume_key(Modifiers::NONE, Key::ArrowUp) as f64
+                - input.count_and_consume_key(Modifiers::NONE, Key::ArrowDown) as f64;
+
+            if change != 0.0 {
+                value += speed * change;
+                value = emath::round_to_decimals(value, auto_decimals);
+            }
+        }
+
+        value = clamp_to_range(value, clamp_range.clone());
+        if old_value != value {
+            set(&mut get_set_value, value);
+            ui.memory().drag_value.edit_string = None;
+        }
+
         let value_text = match custom_formatter {
             Some(custom_formatter) => custom_formatter(value, auto_decimals..=max_decimals),
             None => {
@@ -394,9 +425,6 @@ impl<'a> Widget for DragValue<'a> {
             }
         };
 
-        let kb_edit_id = ui.next_auto_id();
-        let is_kb_editing = ui.memory().has_focus(kb_edit_id);
-
         let mut response = if is_kb_editing {
             let button_width = ui.spacing().interact_size.x;
             let mut value_text = ui
@@ -419,14 +447,10 @@ impl<'a> Widget for DragValue<'a> {
                 let parsed_value = clamp_to_range(parsed_value, clamp_range);
                 set(&mut get_set_value, parsed_value);
             }
-            if ui.input().key_pressed(Key::Enter) {
-                ui.memory().surrender_focus(kb_edit_id);
-                ui.memory().drag_value.edit_string = None;
-            } else {
-                ui.memory().drag_value.edit_string = Some(value_text);
-            }
+            ui.memory().drag_value.edit_string = Some(value_text);
             response
         } else {
+            ui.memory().drag_value.edit_string = None;
             let button = Button::new(
                 RichText::new(format!("{}{}{}", prefix, value_text, suffix)).monospace(),
             )
@@ -448,7 +472,6 @@ impl<'a> Widget for DragValue<'a> {
 
             if response.clicked() {
                 ui.memory().request_focus(kb_edit_id);
-                ui.memory().drag_value.edit_string = None; // Filled in next frame
             } else if response.dragged() {
                 ui.output().cursor_icon = CursorIcon::ResizeHorizontal;
 
@@ -483,18 +506,6 @@ impl<'a> Widget for DragValue<'a> {
                     drag_state.last_dragged_value = Some(stored_value);
                     ui.memory().drag_value = drag_state;
                 }
-            } else if response.has_focus() {
-                let change = ui.input().num_presses(Key::ArrowUp) as f64
-                    + ui.input().num_presses(Key::ArrowRight) as f64
-                    - ui.input().num_presses(Key::ArrowDown) as f64
-                    - ui.input().num_presses(Key::ArrowLeft) as f64;
-
-                if change != 0.0 {
-                    let new_value = value + speed * change;
-                    let new_value = emath::round_to_decimals(new_value, auto_decimals);
-                    let new_value = clamp_to_range(new_value, clamp_range);
-                    set(&mut get_set_value, new_value);
-                }
             }
 
             response