From b80bfc117063982a8c92edefc0239a0a6a28f56f Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Thu, 30 Sep 2021 09:23:49 +0100 Subject: [PATCH 01/12] Menubar: close menu with click on bar --- crates/kas-widgets/src/menu/menubar.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/kas-widgets/src/menu/menubar.rs b/crates/kas-widgets/src/menu/menubar.rs index ab77a0e86..078e642da 100644 --- a/crates/kas-widgets/src/menu/menubar.rs +++ b/crates/kas-widgets/src/menu/menubar.rs @@ -116,6 +116,8 @@ impl, D: Directional, M: 'static> event::Handler for MenuBar Date: Thu, 30 Sep 2021 09:43:10 +0100 Subject: [PATCH 02/12] Close popups on focus lost --- crates/kas-core/src/event/manager/mgr_shell.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/kas-core/src/event/manager/mgr_shell.rs b/crates/kas-core/src/event/manager/mgr_shell.rs index 0aead57fd..13ec0d5d8 100644 --- a/crates/kas-core/src/event/manager/mgr_shell.rs +++ b/crates/kas-core/src/event/manager/mgr_shell.rs @@ -457,6 +457,12 @@ impl<'a> Manager<'a> { } } } + Focused(false) => { + // Window focus lost: close all popups + while let Some(id) = self.state.popups.last().map(|(id, _, _)| *id) { + self.close_window(id); + } + } KeyboardInput { input, is_synthetic, From 531f7576f6c806efcd5323017099cd6db55e6af7 Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Thu, 30 Sep 2021 10:24:10 +0100 Subject: [PATCH 03/12] Remove Event::NewPopup: directly close non-ancestor popups instead --- crates/kas-core/src/event/events.rs | 6 ------ .../kas-core/src/event/manager/mgr_shell.rs | 19 +++++++++++-------- crates/kas-widgets/src/combobox.rs | 8 -------- crates/kas-widgets/src/menu/menu_entry.rs | 3 --- crates/kas-widgets/src/menu/submenu.rs | 8 -------- 5 files changed, 11 insertions(+), 33 deletions(-) diff --git a/crates/kas-core/src/event/events.rs b/crates/kas-core/src/event/events.rs index 8e6122685..e4ba579ff 100644 --- a/crates/kas-core/src/event/events.rs +++ b/crates/kas-core/src/event/events.rs @@ -153,12 +153,6 @@ pub enum Event { /// A user-defined payload is passed. Interpretation of this payload is /// user-defined and unfortunately not type safe. HandleUpdate { handle: UpdateHandle, payload: u64 }, - /// Notification that a new popup has been created - /// - /// This is sent to the parent of each open popup when a new popup is - /// created. This enables parents to close their popups when the new popup - /// is not a descendant of itself. The `WidgetId` is that of the popup. - NewPopup(WidgetId), /// Notification that a popup has been destroyed /// /// This is sent to the popup's parent after a popup has been removed. diff --git a/crates/kas-core/src/event/manager/mgr_shell.rs b/crates/kas-core/src/event/manager/mgr_shell.rs index 13ec0d5d8..c9febd6c3 100644 --- a/crates/kas-core/src/event/manager/mgr_shell.rs +++ b/crates/kas-core/src/event/manager/mgr_shell.rs @@ -313,14 +313,17 @@ impl ManagerState { mgr.send_event(widget, parent, Event::PopupRemoved(wid)); } while let Some(id) = mgr.state.new_popups.pop() { - for parent in mgr - .state - .popups - .iter() - .map(|(_, popup, _)| popup.parent) - .collect::>() - { - mgr.send_event(widget, parent, Event::NewPopup(id)); + while let Some((_, popup, _)) = mgr.state.popups.last() { + if widget + .find_leaf(popup.parent) + .map(|w| w.is_ancestor_of(id)) + .unwrap_or(false) + { + break; + } + let (wid, popup, _old_nav_focus) = mgr.state.popups.pop().unwrap(); + mgr.send_event(widget, popup.parent, Event::PopupRemoved(wid)); + // Don't restore old nav focus: assume new focus will be set by new popup } } diff --git a/crates/kas-widgets/src/combobox.rs b/crates/kas-widgets/src/combobox.rs index 345367b0e..8d2d86fa6 100644 --- a/crates/kas-widgets/src/combobox.rs +++ b/crates/kas-widgets/src/combobox.rs @@ -379,14 +379,6 @@ impl event::Handler for ComboBox { mgr.close_window(id); } } - Event::NewPopup(id) => { - // For a ComboBox, for any new Popup we should close self - if id != self.popup.id() { - if let Some(id) = self.popup_id { - mgr.close_window(id); - } - } - } Event::PopupRemoved(id) => { debug_assert_eq!(Some(id), self.popup_id); self.popup_id = None; diff --git a/crates/kas-widgets/src/menu/menu_entry.rs b/crates/kas-widgets/src/menu/menu_entry.rs index a5e69b135..e6dba57be 100644 --- a/crates/kas-widgets/src/menu/menu_entry.rs +++ b/crates/kas-widgets/src/menu/menu_entry.rs @@ -33,9 +33,6 @@ impl WidgetConfig for MenuEntry { fn key_nav(&self) -> bool { true } - fn hover_highlight(&self) -> bool { - true - } } impl Layout for MenuEntry { diff --git a/crates/kas-widgets/src/menu/submenu.rs b/crates/kas-widgets/src/menu/submenu.rs index 95f7c3df8..e04534252 100644 --- a/crates/kas-widgets/src/menu/submenu.rs +++ b/crates/kas-widgets/src/menu/submenu.rs @@ -102,9 +102,6 @@ impl WidgetConfig for SubMenu { fn key_nav(&self) -> bool { self.key_nav } - fn hover_highlight(&self) -> bool { - true - } } impl kas::Layout for SubMenu { @@ -158,11 +155,6 @@ impl> event::Handler for SubMenu { - if self.popup_id.is_some() && !self.is_ancestor_of(id) { - self.close_menu(mgr); - } - } Event::PopupRemoved(id) => { debug_assert_eq!(Some(id), self.popup_id); self.popup_id = None; From 569abef09fdb55aaa136183c01517b2da9853ded Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Thu, 30 Sep 2021 10:39:46 +0100 Subject: [PATCH 04/12] TkAction::CLOSE is not a valid way to close a popup --- crates/kas-core/src/core/data.rs | 6 ------ crates/kas-core/src/event/manager/mgr_pub.rs | 11 ----------- crates/kas-core/src/toolkit.rs | 2 +- crates/kas-widgets/src/combobox.rs | 3 --- crates/kas-widgets/src/menu/submenu.rs | 11 ----------- 5 files changed, 1 insertion(+), 32 deletions(-) diff --git a/crates/kas-core/src/core/data.rs b/crates/kas-core/src/core/data.rs index 0fc3dae91..cb88d07c7 100644 --- a/crates/kas-core/src/core/data.rs +++ b/crates/kas-core/src/core/data.rs @@ -156,12 +156,6 @@ pub trait LayoutData { /// A pop-up widget's rect is not contained by its parent, therefore the parent /// must not call any [`Layout`] methods on the pop-up (whether or not it is /// visible). The window is responsible for calling these methods. -/// -/// Other methods on the pop-up, including event handlers, should be called -/// normally, with one exception: after calling an event handler on the pop-up, -/// the parent should invoke [`Manager::pop_action`] and handle the action -/// itself, where possible (using [`Manager::close_window`] to close it). -/// Remaining actions should be added back to the [`Manager`]. // // NOTE: it's tempting to include a pointer to the widget here. There are two // options: (a) an unsafe aliased pointer or (b) Rc>. diff --git a/crates/kas-core/src/event/manager/mgr_pub.rs b/crates/kas-core/src/event/manager/mgr_pub.rs index 2218ad08d..de683333a 100644 --- a/crates/kas-core/src/event/manager/mgr_pub.rs +++ b/crates/kas-core/src/event/manager/mgr_pub.rs @@ -216,17 +216,6 @@ impl<'a> Manager<'a> { self.action |= action; } - /// Get the current [`TkAction`], replacing with `None` - /// - /// The caller is responsible for ensuring the action is handled correctly; - /// generally this means matching only actions which can be handled locally - /// and downgrading the action, adding the result back to the [`Manager`]. - pub fn pop_action(&mut self) -> TkAction { - let action = self.action; - self.action = TkAction::empty(); - action - } - /// Add an overlay (pop-up) /// /// A pop-up is a box used for things like tool-tips and menus which is diff --git a/crates/kas-core/src/toolkit.rs b/crates/kas-core/src/toolkit.rs index 92924ba15..bf3293239 100644 --- a/crates/kas-core/src/toolkit.rs +++ b/crates/kas-core/src/toolkit.rs @@ -80,7 +80,7 @@ bitflags! { /// /// [`WidgetId`]: crate::WidgetId const RECONFIGURE = 1 << 16; - /// The current window or pop-up should be closed + /// The current window should be closed const CLOSE = 1 << 30; /// Close all windows and exit const EXIT = 1 << 31; diff --git a/crates/kas-widgets/src/combobox.rs b/crates/kas-widgets/src/combobox.rs index 8d2d86fa6..4e2ccbb57 100644 --- a/crates/kas-widgets/src/combobox.rs +++ b/crates/kas-widgets/src/combobox.rs @@ -299,9 +299,6 @@ impl ComboBox { } } } - // NOTE: as part of the Popup API we are expected to trap - // TkAction::CLOSE here, but we know our widget doesn't generate - // this action. } } diff --git a/crates/kas-widgets/src/menu/submenu.rs b/crates/kas-widgets/src/menu/submenu.rs index e04534252..12f09f79f 100644 --- a/crates/kas-widgets/src/menu/submenu.rs +++ b/crates/kas-widgets/src/menu/submenu.rs @@ -181,17 +181,6 @@ impl event::SendEvent for SubMenu { if id <= self.list.id() { let r = self.list.send(mgr, id, event.clone()); - // The pop-up API expects us to check actions here - // But NOTE: we don't actually use this. Should we remove from API? - match mgr.pop_action() { - TkAction::CLOSE => { - if let Some(id) = self.popup_id { - mgr.close_window(id); - } - } - other => mgr.send_action(other), - } - match r { Response::None => Response::None, Response::Pan(delta) => Response::Pan(delta), From 8a1090a35dd1f30f9179f2784f97709e6fd2732a Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Fri, 1 Oct 2021 17:07:39 +0100 Subject: [PATCH 05/12] When closing a popup close all descendants first --- crates/kas-core/src/event/manager/mgr_pub.rs | 16 ++++++++++------ crates/kas-widgets/src/menu/submenu.rs | 6 +++--- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/crates/kas-core/src/event/manager/mgr_pub.rs b/crates/kas-core/src/event/manager/mgr_pub.rs index de683333a..96bf8cdb1 100644 --- a/crates/kas-core/src/event/manager/mgr_pub.rs +++ b/crates/kas-core/src/event/manager/mgr_pub.rs @@ -255,6 +255,9 @@ impl<'a> Manager<'a> { } /// Close a window or pop-up + /// + /// In the case of a pop-up, all pop-ups created after this will also be + /// removed (on the assumption they are a descendant of the first popup). #[inline] pub fn close_window(&mut self, id: WindowId) { if let Some(index) = @@ -268,18 +271,19 @@ impl<'a> Manager<'a> { }, ) { - let (_, popup, old_nav_focus) = self.state.popups.remove(index); - self.state.popup_removed.push((popup.parent, id)); + let mut old_nav_focus = None; + while self.state.popups.len() > index { + let (wid, popup, onf) = self.state.popups.pop().unwrap(); + self.state.popup_removed.push((popup.parent, wid)); + self.shell.close_window(wid); + old_nav_focus = onf; + } if let Some(id) = old_nav_focus { self.set_nav_focus(id, true); } } - // For popups, we need to update mouse/keyboard focus. - // (For windows, focus gained/lost events do this job.) - self.state.send_action(TkAction::REGION_MOVED); - self.shell.close_window(id); } diff --git a/crates/kas-widgets/src/menu/submenu.rs b/crates/kas-widgets/src/menu/submenu.rs index 12f09f79f..1703c665d 100644 --- a/crates/kas-widgets/src/menu/submenu.rs +++ b/crates/kas-widgets/src/menu/submenu.rs @@ -214,13 +214,13 @@ impl event::SendEvent for SubMenu { } _ => Response::Unhandled, }, - Response::Update | Response::Select => { + Response::Select => { self.set_menu_path(mgr, Some(id)); Response::None } - Response::Msg(msg) => { + r @ (Response::Update | Response::Msg(_)) => { self.close_menu(mgr); - Response::Msg(msg) + r } } } else { From 59c71e0d578124b971e21cdb2d90369d3a70d558 Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Wed, 6 Oct 2021 14:09:46 +0100 Subject: [PATCH 06/12] When opening a menu, do not set navigation focus if activated via mouse --- crates/kas-widgets/src/menu.rs | 12 ++++++++--- crates/kas-widgets/src/menu/menubar.rs | 24 ++++++++++----------- crates/kas-widgets/src/menu/submenu.rs | 30 ++++++++++++++------------ 3 files changed, 37 insertions(+), 29 deletions(-) diff --git a/crates/kas-widgets/src/menu.rs b/crates/kas-widgets/src/menu.rs index 392186bb2..d81a109c6 100644 --- a/crates/kas-widgets/src/menu.rs +++ b/crates/kas-widgets/src/menu.rs @@ -33,7 +33,13 @@ pub trait Menu: Widget { /// are menus, these should open. /// /// `target == None` implies that all menus should close. - fn set_menu_path(&mut self, _mgr: &mut Manager, _target: Option) {} + /// + /// When opening menus and `set_focus` is true, the first navigable child + /// of the newly opened menu will be given focus. This is used for keyboard + /// navigation only. + fn set_menu_path(&mut self, mgr: &mut Manager, target: Option, set_focus: bool) { + let _ = (mgr, target, set_focus); + } } impl WidgetCore for Box> { @@ -156,8 +162,8 @@ impl Menu for Box> { fn menu_is_open(&self) -> bool { self.deref().menu_is_open() } - fn set_menu_path(&mut self, mgr: &mut Manager, target: Option) { - self.deref_mut().set_menu_path(mgr, target) + fn set_menu_path(&mut self, mgr: &mut Manager, target: Option, set_focus: bool) { + self.deref_mut().set_menu_path(mgr, target, set_focus) } } diff --git a/crates/kas-widgets/src/menu/menubar.rs b/crates/kas-widgets/src/menu/menubar.rs index 078e642da..1a57c26f5 100644 --- a/crates/kas-widgets/src/menu/menubar.rs +++ b/crates/kas-widgets/src/menu/menubar.rs @@ -93,7 +93,7 @@ impl, D: Directional, M: 'static> event::Handler for MenuBar { if let Some(id) = self.delayed_open { - self.set_menu_path(mgr, Some(id)); + self.set_menu_path(mgr, Some(id), false); } } Event::PressStart { @@ -115,9 +115,9 @@ impl, D: Directional, M: 'static> event::Handler for MenuBar, D: Directional, M: 'static> event::Handler for MenuBar, D: Directional, M: 'static> event::Handler for MenuBar, D: Directional, M: 'static> event::Handler for MenuBar { @@ -192,9 +192,9 @@ impl, D: Directional, M: 'static> event::Handler for MenuBar event::SendEvent for MenuBar { } impl Menu for MenuBar { - fn set_menu_path(&mut self, mgr: &mut Manager, target: Option) { + fn set_menu_path(&mut self, mgr: &mut Manager, target: Option, set_focus: bool) { self.delayed_open = None; if let Some(id) = target { // We should close other sub-menus before opening @@ -241,15 +241,15 @@ impl Menu for MenuBar { if self.bar[i].is_ancestor_of(id) { child = Some(i); } else { - self.bar[i].set_menu_path(mgr, None); + self.bar[i].set_menu_path(mgr, None, set_focus); } } if let Some(i) = child { - self.bar[i].set_menu_path(mgr, target); + self.bar[i].set_menu_path(mgr, target, set_focus); } } else { for i in 0..self.bar.len() { - self.bar[i].set_menu_path(mgr, None); + self.bar[i].set_menu_path(mgr, None, set_focus); } } } diff --git a/crates/kas-widgets/src/menu/submenu.rs b/crates/kas-widgets/src/menu/submenu.rs index 1703c665d..dd7df2f51 100644 --- a/crates/kas-widgets/src/menu/submenu.rs +++ b/crates/kas-widgets/src/menu/submenu.rs @@ -72,14 +72,16 @@ impl SubMenu { } } - fn open_menu(&mut self, mgr: &mut Manager) { + fn open_menu(&mut self, mgr: &mut Manager, set_focus: bool) { if self.popup_id.is_none() { self.popup_id = mgr.add_popup(kas::Popup { id: self.list.id(), parent: self.id(), direction: self.direction.as_direction(), }); - mgr.next_nav_focus(self, false, true); + if set_focus { + mgr.next_nav_focus(self, false, true); + } } } fn close_menu(&mut self, mgr: &mut Manager) { @@ -152,7 +154,7 @@ impl> event::Handler for SubMenu { if self.popup_id.is_none() { - self.open_menu(mgr); + self.open_menu(mgr, true); } } Event::PopupRemoved(id) => { @@ -160,10 +162,10 @@ impl> event::Handler for SubMenu match (self.direction.as_direction(), cmd) { - (Direction::Left, Command::Left) => self.open_menu(mgr), - (Direction::Right, Command::Right) => self.open_menu(mgr), - (Direction::Up, Command::Up) => self.open_menu(mgr), - (Direction::Down, Command::Down) => self.open_menu(mgr), + (Direction::Left, Command::Left) => self.open_menu(mgr, true), + (Direction::Right, Command::Right) => self.open_menu(mgr, true), + (Direction::Up, Command::Up) => self.open_menu(mgr, true), + (Direction::Down, Command::Down) => self.open_menu(mgr, true), _ => return Response::Unhandled, }, _ => return Response::Unhandled, @@ -215,7 +217,7 @@ impl event::SendEvent for SubMenu { _ => Response::Unhandled, }, Response::Select => { - self.set_menu_path(mgr, Some(id)); + self.set_menu_path(mgr, Some(id), true); Response::None } r @ (Response::Update | Response::Msg(_)) => { @@ -234,7 +236,7 @@ impl Menu for SubMenu { self.popup_id.is_some() } - fn set_menu_path(&mut self, mgr: &mut Manager, target: Option) { + fn set_menu_path(&mut self, mgr: &mut Manager, target: Option, set_focus: bool) { match target { Some(id) if self.is_ancestor_of(id) => { if self.popup_id.is_some() { @@ -244,17 +246,17 @@ impl Menu for SubMenu { if self.list[i].is_ancestor_of(id) { child = Some(i); } else { - self.list[i].set_menu_path(mgr, None); + self.list[i].set_menu_path(mgr, None, set_focus); } } if let Some(i) = child { - self.list[i].set_menu_path(mgr, target); + self.list[i].set_menu_path(mgr, target, set_focus); } } else { - self.open_menu(mgr); + self.open_menu(mgr, set_focus); if id != self.id() { for i in 0..self.list.len() { - self.list[i].set_menu_path(mgr, target); + self.list[i].set_menu_path(mgr, target, set_focus); } } } @@ -262,7 +264,7 @@ impl Menu for SubMenu { _ => { if self.popup_id.is_some() { for i in 0..self.list.len() { - self.list[i].set_menu_path(mgr, None); + self.list[i].set_menu_path(mgr, None, set_focus); } self.close_menu(mgr); } From 07f0946021782840e9b61a92760b8209f6fb0673 Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Wed, 6 Oct 2021 14:52:05 +0100 Subject: [PATCH 07/12] Clear char focus when clearing nav focus --- crates/kas-core/src/event/manager/mgr_pub.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/kas-core/src/event/manager/mgr_pub.rs b/crates/kas-core/src/event/manager/mgr_pub.rs index 96bf8cdb1..edecacaaf 100644 --- a/crates/kas-core/src/event/manager/mgr_pub.rs +++ b/crates/kas-core/src/event/manager/mgr_pub.rs @@ -618,6 +618,7 @@ impl<'a> Manager<'a> { self.redraw(id); } self.state.nav_focus = None; + self.clear_char_focus(); trace!("Manager: nav_focus = None"); } From 44bb7904004c832a80e3f814d7b8bbb20ad3a72f Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Wed, 6 Oct 2021 18:04:29 +0100 Subject: [PATCH 08/12] Fix crash: text editing due to index-out-of-range after EditField::set_string Also remove some useless code. --- crates/kas-core/src/event/manager/mgr_shell.rs | 16 ---------------- crates/kas-core/src/text/selection.rs | 5 +++++ crates/kas-widgets/src/editbox.rs | 1 + 3 files changed, 6 insertions(+), 16 deletions(-) diff --git a/crates/kas-core/src/event/manager/mgr_shell.rs b/crates/kas-core/src/event/manager/mgr_shell.rs index c9febd6c3..7070efd7c 100644 --- a/crates/kas-core/src/event/manager/mgr_shell.rs +++ b/crates/kas-core/src/event/manager/mgr_shell.rs @@ -240,22 +240,6 @@ impl ManagerState { trace!("Manager::region_moved"); // Note: redraw is already implied. - self.nav_focus = self - .nav_focus - .and_then(|id| widget.find_leaf(id).map(|w| w.id())); - if let Some(id) = self.sel_focus { - if let Some(new_id) = widget.find_leaf(id).map(|w| w.id()) { - self.sel_focus = Some(new_id); - } else { - if self.char_focus { - self.pending.push(Pending::LostCharFocus(id)); - self.char_focus = false; - } - self.pending.push(Pending::LostSelFocus(id)); - self.sel_focus = None; - } - } - // Update hovered widget let hover = widget.find_id(self.last_mouse_coord); self.with(shell, |mgr| mgr.set_hover(widget, hover)); diff --git a/crates/kas-core/src/text/selection.rs b/crates/kas-core/src/text/selection.rs index 83f44fce5..83c039675 100644 --- a/crates/kas-core/src/text/selection.rs +++ b/crates/kas-core/src/text/selection.rs @@ -34,6 +34,11 @@ impl SelectionHelper { } } + /// Reset to the default state + pub fn clear(&mut self) { + *self = Self::default(); + } + /// True if the edit pos equals the selection pos pub fn is_empty(&self) -> bool { self.edit_pos == self.sel_pos diff --git a/crates/kas-widgets/src/editbox.rs b/crates/kas-widgets/src/editbox.rs index da062a76b..df2c51a2d 100644 --- a/crates/kas-widgets/src/editbox.rs +++ b/crates/kas-widgets/src/editbox.rs @@ -1044,6 +1044,7 @@ impl HasString for EditField { } self.text.set_string(string); + self.selection.clear(); if kas::text::fonts::fonts().num_faces() > 0 { if let Some(req) = self.text.prepare() { self.required = req.into(); From 4bddd761a9910f0466993fcee79cc6ea2ef83d93 Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Mon, 11 Oct 2021 14:00:59 +0100 Subject: [PATCH 09/12] Remove empty file --- crates/kas-core/src/data/shared_data.rs | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 crates/kas-core/src/data/shared_data.rs diff --git a/crates/kas-core/src/data/shared_data.rs b/crates/kas-core/src/data/shared_data.rs deleted file mode 100644 index 6196a72cf..000000000 --- a/crates/kas-core/src/data/shared_data.rs +++ /dev/null @@ -1,6 +0,0 @@ -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License in the LICENSE-APACHE file or at: -// https://www.apache.org/licenses/LICENSE-2.0 - -//! Traits for shared data objects From b2f69d28c428138662db73cded2548c0dd373654 Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Mon, 11 Oct 2021 14:33:53 +0100 Subject: [PATCH 10/12] Add as_direction() for Command, impl Display for Direction --- crates/kas-core/src/dir.rs | 17 +++++++++++++++++ crates/kas-core/src/event/events.rs | 13 ++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/crates/kas-core/src/dir.rs b/crates/kas-core/src/dir.rs index 7ba79190d..3ec613427 100644 --- a/crates/kas-core/src/dir.rs +++ b/crates/kas-core/src/dir.rs @@ -5,6 +5,8 @@ //! Direction types +use std::fmt; + /// Trait over directional types /// /// This trait has a variable implementation, [`Direction`], and several fixed @@ -92,6 +94,21 @@ pub enum Direction { Up = 3, } +impl fmt::Display for Direction { + fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { + write!( + f, + "{}", + match self { + Direction::Right => "Right", + Direction::Down => "Down", + Direction::Left => "Left", + Direction::Up => "Up", + } + ) + } +} + impl Directional for Direction { type Flipped = Self; type Reversed = Self; diff --git a/crates/kas-core/src/event/events.rs b/crates/kas-core/src/event/events.rs index e4ba579ff..5c1c72605 100644 --- a/crates/kas-core/src/event/events.rs +++ b/crates/kas-core/src/event/events.rs @@ -13,7 +13,7 @@ use super::{GrabMode, Manager, Response, SendEvent}; // for doc-links use super::{MouseButton, UpdateHandle, VirtualKeyCode}; use crate::geom::{Coord, DVec2, Offset}; -use crate::{WidgetId, WindowId}; +use crate::{dir::Direction, WidgetId, WindowId}; /// Events addressed to a widget #[non_exhaustive] @@ -382,6 +382,17 @@ impl Command { use Command::*; matches!(self, Escape | Cut | Copy | Deselect) } + + /// Convert arrow keys to a direction + pub fn as_direction(self) -> Option { + match self { + Command::Left => Some(Direction::Left), + Command::Right => Some(Direction::Right), + Command::Up => Some(Direction::Up), + Command::Down => Some(Direction::Down), + _ => None, + } + } } /// Source of `EventChild::Press` From 41e556c95158ffb972baa3dd2b8c27ca04886102 Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Mon, 11 Oct 2021 14:37:24 +0100 Subject: [PATCH 11/12] Rewrite menu key handling --- crates/kas-widgets/src/menu/menubar.rs | 38 +++++++-------- crates/kas-widgets/src/menu/submenu.rs | 64 +++++++++++++------------- 2 files changed, 52 insertions(+), 50 deletions(-) diff --git a/crates/kas-widgets/src/menu/menubar.rs b/crates/kas-widgets/src/menu/menubar.rs index 1a57c26f5..29ce3e431 100644 --- a/crates/kas-widgets/src/menu/menubar.rs +++ b/crates/kas-widgets/src/menu/menubar.rs @@ -177,27 +177,29 @@ impl, D: Directional, M: 'static> event::Handler for MenuBar { - // Arrow keys can switch to the next / previous menu. + // Arrow keys can switch to the next / previous menu + // as well as to the first / last item of an open menu. + use Command::{Left, Up}; let is_vert = self.bar.direction().is_vertical(); - let reverse = self.bar.direction().is_reversed() - ^ match cmd { - Command::Left if !is_vert => true, - Command::Right if !is_vert => false, - Command::Up if is_vert => true, - Command::Down if is_vert => false, - _ => return Response::Unhandled, - }; - - for i in 0..self.bar.len() { - if self.bar[i].menu_is_open() { - let index = if reverse { i.wrapping_sub(1) } else { i + 1 }; - if index < self.bar.len() { - self.bar[i].set_menu_path(mgr, None, true); - let w = &mut self.bar[index]; - w.set_menu_path(mgr, Some(w.id()), true); + let reverse = self.bar.direction().is_reversed() ^ matches!(cmd, Left | Up); + match cmd.as_direction().map(|d| d.is_vertical()) { + Some(v) if v == is_vert => { + for i in 0..self.bar.len() { + if self.bar[i].menu_is_open() { + let mut j = isize::conv(i); + j = if reverse { j - 1 } else { j + 1 }; + j = j.rem_euclid(self.bar.len().cast()); + self.bar[i].set_menu_path(mgr, None, true); + let w = &mut self.bar[usize::conv(j)]; + w.set_menu_path(mgr, Some(w.id()), true); + break; + } } - break; } + Some(_) => { + mgr.next_nav_focus(self, reverse, true); + } + None => return Response::Unhandled, } } _ => return Response::Unhandled, diff --git a/crates/kas-widgets/src/menu/submenu.rs b/crates/kas-widgets/src/menu/submenu.rs index dd7df2f51..076a00417 100644 --- a/crates/kas-widgets/src/menu/submenu.rs +++ b/crates/kas-widgets/src/menu/submenu.rs @@ -89,6 +89,35 @@ impl SubMenu { mgr.close_window(id); } } + + fn handle_dir_key(&mut self, mgr: &mut Manager, cmd: Command) -> Response { + if self.menu_is_open() { + if let Some(dir) = cmd.as_direction() { + if dir.is_vertical() == self.list.direction().is_vertical() { + let rev = dir.is_reversed() ^ self.list.direction().is_reversed(); + mgr.next_nav_focus(self, rev, true); + Response::None + } else if dir == self.direction.as_direction().reversed() { + self.close_menu(mgr); + Response::None + } else { + Response::Unhandled + } + } else if matches!(cmd, Command::Home | Command::End) { + mgr.clear_nav_focus(); + let rev = cmd == Command::End; + mgr.next_nav_focus(self, rev, true); + Response::None + } else { + Response::Unhandled + } + } else if Some(self.direction.as_direction()) == cmd.as_direction() { + self.open_menu(mgr, true); + Response::None + } else { + Response::Unhandled + } + } } impl WidgetConfig for SubMenu { @@ -161,13 +190,7 @@ impl> event::Handler for SubMenu match (self.direction.as_direction(), cmd) { - (Direction::Left, Command::Left) => self.open_menu(mgr, true), - (Direction::Right, Command::Right) => self.open_menu(mgr, true), - (Direction::Up, Command::Up) => self.open_menu(mgr, true), - (Direction::Down, Command::Down) => self.open_menu(mgr, true), - _ => return Response::Unhandled, - }, + Event::Command(cmd, _) => return self.handle_dir_key(mgr, cmd), _ => return Response::Unhandled, } Response::None @@ -188,31 +211,8 @@ impl event::SendEvent for SubMenu { Response::Pan(delta) => Response::Pan(delta), Response::Focus(rect) => Response::Focus(rect), Response::Unhandled => match event { - Event::Command(key, _) if self.popup_id.is_some() => { - let dir = self.direction.as_direction(); - let inner_vert = self.list.direction().is_vertical(); - let next = |mgr: &mut Manager, s, clr, rev| { - if clr { - mgr.clear_nav_focus(); - } - mgr.next_nav_focus(s, rev, true); - }; - let rev = self.list.direction().is_reversed(); - use Direction::*; - match key { - Command::Left if !inner_vert => next(mgr, self, false, !rev), - Command::Right if !inner_vert => next(mgr, self, false, rev), - Command::Up if inner_vert => next(mgr, self, false, !rev), - Command::Down if inner_vert => next(mgr, self, false, rev), - Command::Home => next(mgr, self, true, false), - Command::End => next(mgr, self, true, true), - Command::Left if dir == Right => self.close_menu(mgr), - Command::Right if dir == Left => self.close_menu(mgr), - Command::Up if dir == Down => self.close_menu(mgr), - Command::Down if dir == Up => self.close_menu(mgr), - _ => return Response::Unhandled, - } - Response::None + Event::Command(cmd, _) if self.popup_id.is_some() => { + self.handle_dir_key(mgr, cmd) } _ => Response::Unhandled, }, From d26c4aa26d7f34d6cf94b9905a390f9cf1c16c9a Mon Sep 17 00:00:00 2001 From: Diggory Hardy Date: Mon, 11 Oct 2021 14:52:42 +0100 Subject: [PATCH 12/12] Fine-tune restore-focus-on-popup-close behaviour --- crates/kas-core/src/event/manager.rs | 6 +++--- crates/kas-core/src/event/manager/mgr_pub.rs | 11 ++++++++++- crates/kas-core/src/event/manager/mgr_shell.rs | 2 +- crates/kas-widgets/src/combobox.rs | 10 +++++----- crates/kas-widgets/src/menu/submenu.rs | 10 +++++----- 5 files changed, 24 insertions(+), 15 deletions(-) diff --git a/crates/kas-core/src/event/manager.rs b/crates/kas-core/src/event/manager.rs index 87cb15853..5b6031071 100644 --- a/crates/kas-core/src/event/manager.rs +++ b/crates/kas-core/src/event/manager.rs @@ -307,7 +307,7 @@ impl<'a> Manager<'a> { return; } else if vkey == VK::Escape { if let Some(id) = self.state.popups.last().map(|(id, _, _)| *id) { - self.close_window(id); + self.close_window(id, true); return; } } else if !self.state.char_focus { @@ -390,7 +390,7 @@ impl<'a> Manager<'a> { let last = self.state.popups.len() - 1; for i in 0..n { let id = self.state.popups[last - i].0; - self.close_window(id); + self.close_window(id, false); } } @@ -516,7 +516,7 @@ impl<'a> Manager<'a> { Response::Unhandled => (), _ => return, } - self.close_window(wid); + self.close_window(wid, false); } self.send_event(widget, id, event); } diff --git a/crates/kas-core/src/event/manager/mgr_pub.rs b/crates/kas-core/src/event/manager/mgr_pub.rs index edecacaaf..dd9dddd85 100644 --- a/crates/kas-core/src/event/manager/mgr_pub.rs +++ b/crates/kas-core/src/event/manager/mgr_pub.rs @@ -258,8 +258,12 @@ impl<'a> Manager<'a> { /// /// In the case of a pop-up, all pop-ups created after this will also be /// removed (on the assumption they are a descendant of the first popup). + /// + /// If `restore_focus` then navigation focus will return to whichever widget + /// had focus before the popup was open. (Usually this is true excepting + /// where focus has already been changed.) #[inline] - pub fn close_window(&mut self, id: WindowId) { + pub fn close_window(&mut self, id: WindowId, restore_focus: bool) { if let Some(index) = self.state.popups.iter().enumerate().find_map( |(i, p)| { @@ -279,9 +283,14 @@ impl<'a> Manager<'a> { old_nav_focus = onf; } + if !restore_focus { + old_nav_focus = None + } if let Some(id) = old_nav_focus { self.set_nav_focus(id, true); } + // TODO: if popup.id is an ancestor of self.nav_focus then clear + // focus if not setting (currently we cannot test this) } self.shell.close_window(id); diff --git a/crates/kas-core/src/event/manager/mgr_shell.rs b/crates/kas-core/src/event/manager/mgr_shell.rs index 7070efd7c..0f2b7a165 100644 --- a/crates/kas-core/src/event/manager/mgr_shell.rs +++ b/crates/kas-core/src/event/manager/mgr_shell.rs @@ -447,7 +447,7 @@ impl<'a> Manager<'a> { Focused(false) => { // Window focus lost: close all popups while let Some(id) = self.state.popups.last().map(|(id, _, _)| *id) { - self.close_window(id); + self.close_window(id, true); } } KeyboardInput { diff --git a/crates/kas-widgets/src/combobox.rs b/crates/kas-widgets/src/combobox.rs index 4e2ccbb57..ab942fc28 100644 --- a/crates/kas-widgets/src/combobox.rs +++ b/crates/kas-widgets/src/combobox.rs @@ -273,7 +273,7 @@ impl ComboBox { Response::Focus(x) => Response::Focus(x), Response::Update | Response::Select => { if let Some(id) = self.popup_id { - mgr.close_window(id); + mgr.close_window(id, true); } if let Some(index) = self.popup.inner.find_child_index(id) { if index != self.active { @@ -290,7 +290,7 @@ impl ComboBox { Response::Msg((index, ())) => { *mgr |= self.set_active(index); if let Some(id) = self.popup_id { - mgr.close_window(id); + mgr.close_window(id, true); } if let Some(ref f) = self.on_select { Response::update_or_msg((f)(mgr, index)) @@ -319,7 +319,7 @@ impl event::Handler for ComboBox { match event { Event::Activate => { if let Some(id) = self.popup_id { - mgr.close_window(id); + mgr.close_window(id, true); } else { open_popup(self, mgr, true); } @@ -337,7 +337,7 @@ impl event::Handler for ComboBox { } } else { if let Some(id) = self.popup_id { - mgr.close_window(id); + mgr.close_window(id, false); } return Response::Unhandled; } @@ -373,7 +373,7 @@ impl event::Handler for ComboBox { } } if let Some(id) = self.popup_id { - mgr.close_window(id); + mgr.close_window(id, true); } } Event::PopupRemoved(id) => { diff --git a/crates/kas-widgets/src/menu/submenu.rs b/crates/kas-widgets/src/menu/submenu.rs index 076a00417..668fd1a5a 100644 --- a/crates/kas-widgets/src/menu/submenu.rs +++ b/crates/kas-widgets/src/menu/submenu.rs @@ -84,9 +84,9 @@ impl SubMenu { } } } - fn close_menu(&mut self, mgr: &mut Manager) { + fn close_menu(&mut self, mgr: &mut Manager, restore_focus: bool) { if let Some(id) = self.popup_id { - mgr.close_window(id); + mgr.close_window(id, restore_focus); } } @@ -98,7 +98,7 @@ impl SubMenu { mgr.next_nav_focus(self, rev, true); Response::None } else if dir == self.direction.as_direction().reversed() { - self.close_menu(mgr); + self.close_menu(mgr, true); Response::None } else { Response::Unhandled @@ -221,7 +221,7 @@ impl event::SendEvent for SubMenu { Response::None } r @ (Response::Update | Response::Msg(_)) => { - self.close_menu(mgr); + self.close_menu(mgr, true); r } } @@ -266,7 +266,7 @@ impl Menu for SubMenu { for i in 0..self.list.len() { self.list[i].set_menu_path(mgr, None, set_focus); } - self.close_menu(mgr); + self.close_menu(mgr, set_focus); } } }