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/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 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 8e6122685..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] @@ -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. @@ -388,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` 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 2218ad08d..dd9dddd85 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 @@ -266,8 +255,15 @@ 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). + /// + /// 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,18 +275,24 @@ 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 !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) } - // 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); } @@ -625,6 +627,7 @@ impl<'a> Manager<'a> { self.redraw(id); } self.state.nav_focus = None; + self.clear_char_focus(); trace!("Manager: nav_focus = None"); } diff --git a/crates/kas-core/src/event/manager/mgr_shell.rs b/crates/kas-core/src/event/manager/mgr_shell.rs index 0aead57fd..0f2b7a165 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)); @@ -313,14 +297,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 } } @@ -457,6 +444,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, true); + } + } KeyboardInput { input, is_synthetic, 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-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 345367b0e..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)) @@ -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. } } @@ -322,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); } @@ -340,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; } @@ -376,15 +373,7 @@ impl event::Handler for ComboBox { } } if let Some(id) = self.popup_id { - 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); - } + mgr.close_window(id, true); } } Event::PopupRemoved(id) => { 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(); 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/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/menubar.rs b/crates/kas-widgets/src/menu/menubar.rs index ab77a0e86..29ce3e431 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,7 +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 { - // 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); - let w = &mut self.bar[index]; - w.set_menu_path(mgr, Some(w.id())); + 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, @@ -230,7 +234,7 @@ impl 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 @@ -239,15 +243,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 95f7c3df8..668fd1a5a 100644 --- a/crates/kas-widgets/src/menu/submenu.rs +++ b/crates/kas-widgets/src/menu/submenu.rs @@ -72,19 +72,50 @@ 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) { + 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); + } + } + + 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, true); + 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 } } } @@ -102,9 +133,6 @@ impl WidgetConfig for SubMenu { fn key_nav(&self) -> bool { self.key_nav } - fn hover_highlight(&self) -> bool { - true - } } impl kas::Layout for SubMenu { @@ -155,25 +183,14 @@ impl> event::Handler for SubMenu { if self.popup_id.is_none() { - self.open_menu(mgr); - } - } - Event::NewPopup(id) => { - if self.popup_id.is_some() && !self.is_ancestor_of(id) { - self.close_menu(mgr); + self.open_menu(mgr, true); } } Event::PopupRemoved(id) => { debug_assert_eq!(Some(id), self.popup_id); self.popup_id = None; } - Event::Command(cmd, _) => 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), - _ => return Response::Unhandled, - }, + Event::Command(cmd, _) => return self.handle_dir_key(mgr, cmd), _ => return Response::Unhandled, } Response::None @@ -189,57 +206,23 @@ 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), 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, }, - Response::Update | Response::Select => { - self.set_menu_path(mgr, Some(id)); + Response::Select => { + self.set_menu_path(mgr, Some(id), true); Response::None } - Response::Msg(msg) => { - self.close_menu(mgr); - Response::Msg(msg) + r @ (Response::Update | Response::Msg(_)) => { + self.close_menu(mgr, true); + r } } } else { @@ -253,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() { @@ -263,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); } } } @@ -281,9 +264,9 @@ 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); + self.close_menu(mgr, set_focus); } } }