From 2793de5056624c930065fab3343822990e284c4b Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Tue, 29 Nov 2022 08:27:14 -0600 Subject: [PATCH] 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 * Add comment explaining why we don't listen for left/right arrow Co-authored-by: Emil Ernerfeldt --- CHANGELOG.md | 1 + crates/egui/src/input_state.rs | 15 ++++--- crates/egui/src/memory.rs | 7 ++- crates/egui/src/widgets/drag_value.rs | 65 ++++++++++++++++----------- 4 files changed, 55 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 80ed7f1adb6..8d3ebfc9b63 100644 --- 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 index 1140bb16830..ab0d321df36 100644 --- 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 index 1d94601b413..73544655d77 100644 --- 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 index 55ea13e9970..13e727d7424 100644 --- 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