From a39d71cbc1249f0037653c8241ee9822d5d824f0 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Thu, 24 Nov 2022 10:26:44 -0600 Subject: [PATCH 1/6] Enable incrementing and decrementing `DragValue` with the keyboard --- crates/egui/src/input_state.rs | 15 ++++++---- crates/egui/src/widgets/drag_value.rs | 42 ++++++++++++++------------- 2 files changed, 32 insertions(+), 25 deletions(-) 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/widgets/drag_value.rs b/crates/egui/src/widgets/drag_value.rs index 55ea13e9970..ae6ecf12f85 100644 --- a/crates/egui/src/widgets/drag_value.rs +++ b/crates/egui/src/widgets/drag_value.rs @@ -371,18 +371,35 @@ 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(); + 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(); + 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 +411,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 @@ -483,18 +497,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 From 52856e281fe7a192e08d582f81cb5ed71415e53d Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Thu, 24 Nov 2022 11:19:09 -0600 Subject: [PATCH 2/6] As soon as a DragValue is focused, render it in edit mode --- crates/egui/src/memory.rs | 7 ++++++- crates/egui/src/widgets/drag_value.rs | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/egui/src/memory.rs b/crates/egui/src/memory.rs index 1d94601b413..40a93e5a136 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 `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 ae6ecf12f85..1bf930a8c01 100644 --- a/crates/egui/src/widgets/drag_value.rs +++ b/crates/egui/src/widgets/drag_value.rs @@ -372,6 +372,7 @@ impl<'a> Widget for DragValue<'a> { let is_slow_speed = shift && ui.memory().is_being_dragged(ui.next_auto_id()); let kb_edit_id = ui.next_auto_id(); + 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); From 2e8a6da8f7cccffcbe0f61cce516fa3d4159c9eb Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Thu, 24 Nov 2022 11:30:27 -0600 Subject: [PATCH 3/6] Simpler, more reliable approach to managing the drag value's edit string --- crates/egui/src/widgets/drag_value.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/crates/egui/src/widgets/drag_value.rs b/crates/egui/src/widgets/drag_value.rs index 1bf930a8c01..b386eecbd23 100644 --- a/crates/egui/src/widgets/drag_value.rs +++ b/crates/egui/src/widgets/drag_value.rs @@ -372,6 +372,10 @@ impl<'a> Widget for DragValue<'a> { 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); @@ -434,14 +438,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(), ) @@ -463,7 +463,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; From 4264862ac04a3e1dfda6998175fb402fd1800680 Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Thu, 24 Nov 2022 12:08:40 -0600 Subject: [PATCH 4/6] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) 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 From a4ad5a1dd627d71744d9f734f99251b30f12545f Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Thu, 24 Nov 2022 13:30:34 -0600 Subject: [PATCH 5/6] Update doc comment Co-authored-by: Emil Ernerfeldt --- crates/egui/src/memory.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/egui/src/memory.rs b/crates/egui/src/memory.rs index 40a93e5a136..73544655d77 100644 --- a/crates/egui/src/memory.rs +++ b/crates/egui/src/memory.rs @@ -403,7 +403,7 @@ impl Memory { /// 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 `DragValue` widget, so a widget can be focused + /// as in the [`crate::DragValue`] widget, so a widget can be focused /// and rendered correctly in a single frame. #[inline(always)] pub fn interested_in_focus(&mut self, id: Id) { From ba8439161149a059ffa8604fc02c51639efe075f Mon Sep 17 00:00:00 2001 From: Matt Campbell Date: Tue, 29 Nov 2022 08:19:27 -0600 Subject: [PATCH 6/6] Add comment explaining why we don't listen for left/right arrow --- crates/egui/src/widgets/drag_value.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/crates/egui/src/widgets/drag_value.rs b/crates/egui/src/widgets/drag_value.rs index b386eecbd23..13e727d7424 100644 --- a/crates/egui/src/widgets/drag_value.rs +++ b/crates/egui/src/widgets/drag_value.rs @@ -390,6 +390,15 @@ impl<'a> Widget for DragValue<'a> { 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;