From 2d68f7151e204ed20bb79fdb083051fe916401a0 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Mon, 18 Sep 2023 11:44:22 +0200 Subject: [PATCH] Bug fix: arrow keys won't move focus away from TextEdit --- crates/egui/src/context.rs | 2 +- crates/egui/src/data/input.rs | 53 +++++++ crates/egui/src/input_state.rs | 9 ++ crates/egui/src/memory.rs | 156 +++++++++++-------- crates/egui/src/widgets/text_edit/builder.rs | 42 ++--- 5 files changed, 176 insertions(+), 86 deletions(-) diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index f87ce43b6c5..c3c99b2a5b6 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -1276,7 +1276,7 @@ impl Context { nodes, tree: Some(accesskit::Tree::new(root_id)), focus: has_focus.then(|| { - let focus_id = self.memory(|mem| mem.interaction.focus.id); + let focus_id = self.memory(|mem| mem.focus()); focus_id.map_or(root_id, |id| id.accesskit_id()) }), }); diff --git a/crates/egui/src/data/input.rs b/crates/egui/src/data/input.rs index 61eb152d8e2..fa7e8e27ba4 100644 --- a/crates/egui/src/data/input.rs +++ b/crates/egui/src/data/input.rs @@ -1028,3 +1028,56 @@ impl From for TouchId { Self(id as u64) } } + +// ---------------------------------------------------------------------------- + +// TODO(emilk): generalize this to a proper event filter. +/// Controls which events that a focused widget will have exclusive access to. +/// +/// Currently this only controls a few special keyboard events, +/// but in the future this `struct` should be extended into a full callback thing. +/// +/// Any events not covered by the filter are given to the widget, but are not exclusive. +#[derive(Clone, Copy, Debug)] +pub struct EventFilter { + /// If `true`, pressing tab will act on the widget, + /// and NOT move focus away from the focused widget. + pub tab: bool, + + /// If `true`, pressing arrows will act on the widget, + /// and NOT move focus away from the focused widget. + pub arrows: bool, + + /// If `true`, pressing escape will act on the widget, + /// and NOT surrender focus from the focused widget. + pub escape: bool, +} + +#[allow(clippy::derivable_impls)] // let's be explicit +impl Default for EventFilter { + fn default() -> Self { + Self { + tab: false, + arrows: false, + escape: false, + } + } +} + +impl EventFilter { + pub fn matches(&self, event: &Event) -> bool { + if let Event::Key { key, .. } = event { + match key { + crate::Key::Tab => self.tab, + crate::Key::ArrowUp + | crate::Key::ArrowRight + | crate::Key::ArrowDown + | crate::Key::ArrowLeft => self.arrows, + crate::Key::Escape => self.escape, + _ => true, + } + } else { + true + } + } +} diff --git a/crates/egui/src/input_state.rs b/crates/egui/src/input_state.rs index 7f94cf16a2a..47b8544f2be 100644 --- a/crates/egui/src/input_state.rs +++ b/crates/egui/src/input_state.rs @@ -461,6 +461,15 @@ impl InputState { pub fn num_accesskit_action_requests(&self, id: crate::Id, action: accesskit::Action) -> usize { self.accesskit_action_requests(id, action).count() } + + /// Get all events that matches the given filter. + pub fn filtered_events(&self, filter: &EventFilter) -> Vec { + self.events + .iter() + .filter(|event| filter.matches(event)) + .cloned() + .collect() + } } // ---------------------------------------------------------------------------- diff --git a/crates/egui/src/memory.rs b/crates/egui/src/memory.rs index a997914e911..d39b4d6e196 100644 --- a/crates/egui/src/memory.rs +++ b/crates/egui/src/memory.rs @@ -1,6 +1,6 @@ use epaint::{emath::Rangef, vec2, Vec2}; -use crate::{area, window, Id, IdMap, InputState, LayerId, Pos2, Rect, Style}; +use crate::{area, window, EventFilter, Id, IdMap, InputState, LayerId, Pos2, Rect, Style}; // ---------------------------------------------------------------------------- @@ -219,7 +219,7 @@ pub(crate) struct Interaction { #[derive(Clone, Debug, Default)] pub(crate) struct Focus { /// The widget with keyboard focus (i.e. a text input field). - pub(crate) id: Option, + focused_widget: Option, /// What had keyboard focus previous frame? id_previous_frame: Option, @@ -237,9 +237,6 @@ pub(crate) struct Focus { /// The last widget interested in focus. last_interested: Option, - /// If `true`, pressing tab will NOT move focus away from the current widget. - is_focus_locked: bool, - /// Set when looking for widget with navigational keys like arrows, tab, shift+tab focus_direction: FocusDirection, @@ -247,6 +244,22 @@ pub(crate) struct Focus { focus_widgets_cache: IdMap, } +/// The widget with focus. +#[derive(Clone, Copy, Debug)] +struct FocusWidget { + pub id: Id, + pub filter: EventFilter, +} + +impl FocusWidget { + pub fn new(id: Id) -> Self { + Self { + id, + filter: Default::default(), + } + } +} + impl Interaction { /// Are we currently clicking or dragging an egui widget? pub fn is_using_pointer(&self) -> bool { @@ -278,14 +291,15 @@ impl Interaction { impl Focus { /// Which widget currently has keyboard focus? pub fn focused(&self) -> Option { - self.id + self.focused_widget.as_ref().map(|w| w.id) } fn begin_frame(&mut self, new_input: &crate::data::input::RawInput) { - self.id_previous_frame = self.id; + self.id_previous_frame = self.focused(); if let Some(id) = self.id_next_frame.take() { - self.id = Some(id); + self.focused_widget = Some(FocusWidget::new(id)); } + let event_filter = self.focused_widget.map(|w| w.filter).unwrap_or_default(); #[cfg(feature = "accesskit")] { @@ -295,37 +309,35 @@ impl Focus { self.focus_direction = FocusDirection::None; for event in &new_input.events { - if let crate::Event::Key { - key, - pressed: true, - modifiers, - .. - } = event - { - if let Some(cardinality) = match key { - crate::Key::ArrowUp => Some(FocusDirection::Up), - crate::Key::ArrowRight => Some(FocusDirection::Right), - crate::Key::ArrowDown => Some(FocusDirection::Down), - crate::Key::ArrowLeft => Some(FocusDirection::Left), - crate::Key::Tab => { - if !self.is_focus_locked { + if !event_filter.matches(event) { + if let crate::Event::Key { + key, + pressed: true, + modifiers, + .. + } = event + { + if let Some(cardinality) = match key { + crate::Key::ArrowUp => Some(FocusDirection::Up), + crate::Key::ArrowRight => Some(FocusDirection::Right), + crate::Key::ArrowDown => Some(FocusDirection::Down), + crate::Key::ArrowLeft => Some(FocusDirection::Left), + + crate::Key::Tab => { if modifiers.shift { Some(FocusDirection::Previous) } else { Some(FocusDirection::Next) } - } else { - None } + crate::Key::Escape => { + self.focused_widget = None; + Some(FocusDirection::None) + } + _ => None, + } { + self.focus_direction = cardinality; } - crate::Key::Escape => { - self.id = None; - self.is_focus_locked = false; - Some(FocusDirection::None) - } - _ => None, - } { - self.focus_direction = cardinality; } } @@ -346,17 +358,17 @@ impl Focus { pub(crate) fn end_frame(&mut self, used_ids: &IdMap) { if self.focus_direction.is_cardinal() { if let Some(found_widget) = self.find_widget_in_direction(used_ids) { - self.id = Some(found_widget); + self.focused_widget = Some(FocusWidget::new(found_widget)); } } - if let Some(id) = self.id { + if let Some(focused_widget) = self.focused_widget { // Allow calling `request_focus` one frame and not using it until next frame - let recently_gained_focus = self.id_previous_frame != Some(id); + let recently_gained_focus = self.id_previous_frame != Some(focused_widget.id); - if !recently_gained_focus && !used_ids.contains_key(&id) { + if !recently_gained_focus && !used_ids.contains_key(&focused_widget.id) { // Dead-mans-switch: the widget with focus has disappeared! - self.id = None; + self.focused_widget = None; } } } @@ -369,7 +381,7 @@ impl Focus { #[cfg(feature = "accesskit")] { if self.id_requested_by_accesskit == Some(id.accesskit_id()) { - self.id = Some(id); + self.focused_widget = Some(FocusWidget::new(id)); self.id_requested_by_accesskit = None; self.give_to_next = false; self.reset_focus(); @@ -382,23 +394,23 @@ impl Focus { .or_insert(Rect::EVERYTHING); if self.give_to_next && !self.had_focus_last_frame(id) { - self.id = Some(id); + self.focused_widget = Some(FocusWidget::new(id)); self.give_to_next = false; - } else if self.id == Some(id) { - if self.focus_direction == FocusDirection::Next && !self.is_focus_locked { - self.id = None; + } else if self.focused() == Some(id) { + if self.focus_direction == FocusDirection::Next { + self.focused_widget = None; self.give_to_next = true; self.reset_focus(); - } else if self.focus_direction == FocusDirection::Previous && !self.is_focus_locked { + } else if self.focus_direction == FocusDirection::Previous { self.id_next_frame = self.last_interested; // frame-delay so gained_focus works self.reset_focus(); } } else if self.focus_direction == FocusDirection::Next - && self.id.is_none() + && self.focused_widget.is_none() && !self.give_to_next { // nothing has focus and the user pressed tab - give focus to the first widgets that wants it: - self.id = Some(id); + self.focused_widget = Some(FocusWidget::new(id)); self.reset_focus(); } @@ -424,7 +436,7 @@ impl Focus { } } - let Some(focus_id) = self.id else { + let Some(current_focused) = self.focused_widget else { return None; }; @@ -449,7 +461,7 @@ impl Focus { } }); - let Some(current_rect) = self.focus_widgets_cache.get(&focus_id) else { + let Some(current_rect) = self.focus_widgets_cache.get(¤t_focused.id) else { return None; }; @@ -457,7 +469,7 @@ impl Focus { let mut best_id = None; for (candidate_id, candidate_rect) in &self.focus_widgets_cache { - if Some(*candidate_id) == self.id { + if *candidate_id == current_focused.id { continue; } @@ -542,46 +554,58 @@ impl Memory { /// from the window and back. #[inline(always)] pub fn has_focus(&self, id: Id) -> bool { - self.interaction.focus.id == Some(id) + self.interaction.focus.focused() == Some(id) } /// Which widget has keyboard focus? pub fn focus(&self) -> Option { - self.interaction.focus.id + self.interaction.focus.focused() } - /// Prevent keyboard focus from moving away from this widget even if users presses the tab key. + /// Set an event filter for a widget. + /// + /// This allows you to control wether the widget will loose focus + /// when the user presses tab, arrow keys, or escape. + /// /// You must first give focus to the widget before calling this. - pub fn lock_focus(&mut self, id: Id, lock_focus: bool) { + pub fn set_focus_lock_filter(&mut self, id: Id, event_filter: EventFilter) { if self.had_focus_last_frame(id) && self.has_focus(id) { - self.interaction.focus.is_focus_locked = lock_focus; + if let Some(focused) = &mut self.interaction.focus.focused_widget { + if focused.id == id { + focused.filter = event_filter; + } + } } } - /// Is the keyboard focus locked on this widget? If so the focus won't move even if the user presses the tab key. - pub fn has_lock_focus(&self, id: Id) -> bool { - if self.had_focus_last_frame(id) && self.has_focus(id) { - self.interaction.focus.is_focus_locked - } else { - false - } + /// Set an event filter for a widget. + /// + /// You must first give focus to the widget before calling this. + #[deprecated = "Use set_focus_lock_filter instead"] + pub fn lock_focus(&mut self, id: Id, lock_focus: bool) { + self.set_focus_lock_filter( + id, + EventFilter { + tab: lock_focus, + arrows: lock_focus, + escape: false, + }, + ); } /// Give keyboard focus to a specific widget. /// See also [`crate::Response::request_focus`]. #[inline(always)] pub fn request_focus(&mut self, id: Id) { - self.interaction.focus.id = Some(id); - self.interaction.focus.is_focus_locked = false; + self.interaction.focus.focused_widget = Some(FocusWidget::new(id)); } /// Surrender keyboard focus for a specific widget. /// See also [`crate::Response::surrender_focus`]. #[inline(always)] pub fn surrender_focus(&mut self, id: Id) { - if self.interaction.focus.id == Some(id) { - self.interaction.focus.id = None; - self.interaction.focus.is_focus_locked = false; + if self.interaction.focus.focused() == Some(id) { + self.interaction.focus.focused_widget = None; } } @@ -600,7 +624,7 @@ impl Memory { /// Stop editing of active [`TextEdit`](crate::TextEdit) (if any). #[inline(always)] pub fn stop_text_input(&mut self) { - self.interaction.focus.id = None; + self.interaction.focus.focused_widget = None; } #[inline(always)] diff --git a/crates/egui/src/widgets/text_edit/builder.rs b/crates/egui/src/widgets/text_edit/builder.rs index 3019fa7fd92..e1969897718 100644 --- a/crates/egui/src/widgets/text_edit/builder.rs +++ b/crates/egui/src/widgets/text_edit/builder.rs @@ -65,7 +65,7 @@ pub struct TextEdit<'t> { interactive: bool, desired_width: Option, desired_height_rows: usize, - lock_focus: bool, + event_filter: EventFilter, cursor_at_end: bool, min_size: Vec2, align: Align2, @@ -115,7 +115,11 @@ impl<'t> TextEdit<'t> { interactive: true, desired_width: None, desired_height_rows: 4, - lock_focus: false, + event_filter: EventFilter { + arrows: true, // moving the cursor is really important + tab: false, // tab is used to change focus, not to insert a tab character + ..Default::default() + }, cursor_at_end: true, min_size: Vec2::ZERO, align: Align2::LEFT_TOP, @@ -127,7 +131,7 @@ impl<'t> TextEdit<'t> { /// Build a [`TextEdit`] focused on code editing. /// By default it comes with: /// - monospaced font - /// - focus lock + /// - focus lock (tab will insert a tab character instead of moving focus) pub fn code_editor(self) -> Self { self.font(TextStyle::Monospace).lock_focus(true) } @@ -266,8 +270,8 @@ impl<'t> TextEdit<'t> { /// /// When `true`, the widget will keep the focus and pressing TAB /// will insert the `'\t'` character. - pub fn lock_focus(mut self, b: bool) -> Self { - self.lock_focus = b; + pub fn lock_focus(mut self, tab_will_indent: bool) -> Self { + self.event_filter.tab = tab_will_indent; self } @@ -352,7 +356,9 @@ impl<'t> TextEdit<'t> { let margin = self.margin; let max_rect = ui.available_rect_before_wrap().shrink2(margin); let mut content_ui = ui.child_ui(max_rect, *ui.layout()); + let mut output = self.show_content(&mut content_ui); + let id = output.response.id; let frame_rect = output.response.rect.expand2(margin); ui.allocate_space(frame_rect.size()); @@ -413,7 +419,7 @@ impl<'t> TextEdit<'t> { interactive, desired_width, desired_height_rows, - lock_focus, + event_filter, cursor_at_end, min_size, align, @@ -569,7 +575,7 @@ impl<'t> TextEdit<'t> { let mut cursor_range = None; let prev_cursor_range = state.cursor_range(&galley); if interactive && ui.memory(|mem| mem.has_focus(id)) { - ui.memory_mut(|mem| mem.lock_focus(id, lock_focus)); + ui.memory_mut(|mem| mem.set_focus_lock_filter(id, event_filter)); let default_cursor_range = if cursor_at_end { CursorRange::one(galley.end()) @@ -589,6 +595,7 @@ impl<'t> TextEdit<'t> { password, default_cursor_range, char_limit, + event_filter, ); if changed { @@ -880,6 +887,7 @@ fn events( password: bool, default_cursor_range: CursorRange, char_limit: usize, + event_filter: EventFilter, ) -> (bool, CursorRange) { let mut cursor_range = state.cursor_range(galley).unwrap_or(default_cursor_range); @@ -898,7 +906,7 @@ fn events( let mut any_change = false; - let events = ui.input(|i| i.events.clone()); // avoid dead-lock by cloning. TODO(emilk): optimize + let events = ui.input(|i| i.filtered_events(&event_filter)); for event in &events { let did_mutate_text = match event { Event::Copy => { @@ -946,19 +954,15 @@ fn events( pressed: true, modifiers, .. - } => { - if multiline && ui.memory(|mem| mem.has_lock_focus(id)) { - let mut ccursor = delete_selected(text, &cursor_range); - if modifiers.shift { - // TODO(emilk): support removing indentation over a selection? - decrease_indentation(&mut ccursor, text); - } else { - insert_text(&mut ccursor, text, "\t", char_limit); - } - Some(CCursorRange::one(ccursor)) + } if multiline => { + let mut ccursor = delete_selected(text, &cursor_range); + if modifiers.shift { + // TODO(emilk): support removing indentation over a selection? + decrease_indentation(&mut ccursor, text); } else { - None + insert_text(&mut ccursor, text, "\t", char_limit); } + Some(CCursorRange::one(ccursor)) } Event::Key { key: Key::Enter,