From 285c92f82b060c2959051207a230d0d8346b680e Mon Sep 17 00:00:00 2001 From: Jonathan Johnson Date: Thu, 28 Dec 2023 15:47:07 -0800 Subject: [PATCH] Root tab order fix, Spacebar widget activiation Closes #99 Disclose now accepts focus and responds to spacebar as a result of this. --- CHANGELOG.md | 12 +++ examples/disclose.rs | 2 +- src/context.rs | 28 +++++-- src/tree.rs | 9 ++- src/widgets/button.rs | 33 +-------- src/widgets/disclose.rs | 38 ++++++---- src/window.rs | 157 ++++++++++++++++++++++++---------------- 7 files changed, 162 insertions(+), 117 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 222145d..fa03145 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Fixed + +- The root widget is now included in the search for widgets to accept focus. +- Widgets that have been laid out with a 0px width or height no longer have + their `redraw` functions called nor can they receive focus. + ### Added - `figures` is now directly re-exported at this crate's root. Kludgine still @@ -17,6 +23,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `Collapse` widget to show/hide the content when the disclosure button is clicked. This widget also supports an optional label that is shown above the content and is also clickable. +- [#99][99]: When an unhandled spacebar event is received by the window, the + focused widget will be activated and deactived by the events. This previously + was a `Button`-specific behavior that has been refactored into an automatic + behavior for all widgets. + +[99]: https://github.com/khonsulabs/cushy/issues/99 ## v0.2.0 diff --git a/examples/disclose.rs b/examples/disclose.rs index 42b9af3..3f0c1ab 100644 --- a/examples/disclose.rs +++ b/examples/disclose.rs @@ -6,7 +6,7 @@ fn main() -> cushy::Result { Disclose::new( "This is some inner content" .align_left() - .and(Disclose::new("This is even further inside")) + .and(Disclose::new("This is even further inside".contain())) .into_rows(), ) .labelled_by("This demonstrates the Disclose widget") diff --git a/src/context.rs b/src/context.rs index 5e21ea6..43c254d 100644 --- a/src/context.rs +++ b/src/context.rs @@ -357,7 +357,19 @@ impl<'context, 'window> EventContext<'context, 'window> { // We've exhausted a forward scan, we can now start searching the final // parent, which is the root. - self.next_focus_within(&root, None, stop_at, advance) + let mut child_context = self.for_other(&root); + let accept_focus = root.lock().as_widget().accept_focus(&mut child_context); + drop(child_context); + if accept_focus { + Some(root.id()) + } else if stop_at == root.id() { + // We cycled completely. + None + } else if let Some(next_focus) = self.widget().explicit_focus_target(advance) { + Some(next_focus.id()) + } else { + self.next_focus_within(&root, None, stop_at, advance) + } } fn next_focus_sibling( @@ -379,6 +391,11 @@ impl<'context, 'window> EventContext<'context, 'window> { stop_at: WidgetId, advance: bool, ) -> Option { + let last_layout = self.current_node.last_layout()?; + if last_layout.size.width <= 0 || last_layout.size.height <= 0 { + return None; + } + let mut visual_order = self.get(&LayoutOrder); if !advance { visual_order = visual_order.rev(); @@ -397,16 +414,15 @@ impl<'context, 'window> EventContext<'context, 'window> { } for child in children { - // Ensure we haven't cycled completely. - if stop_at == child.id() { - break; - } - let mut child_context = self.for_other(&child); let accept_focus = child.lock().as_widget().accept_focus(&mut child_context); drop(child_context); if accept_focus { return Some(child.id()); + } else if stop_at == child.id() { + // We cycled completely, and the original widget didn't accept + // focus. + return None; } else if let Some(next_focus) = self.widget().explicit_focus_target(advance) { return Some(next_focus.id()); } else if let Some(focus) = self.next_focus_within(&child, None, stop_at, advance) { diff --git a/src/tree.rs b/src/tree.rs index d3dd45e..425c660 100644 --- a/src/tree.rs +++ b/src/tree.rs @@ -174,9 +174,12 @@ impl Tree { let mut index = 0; while index < unordered.len() { - let Some(layout) = &data.nodes[unordered[index]].layout else { - unordered.remove(index); - continue; + let layout = match &data.nodes[unordered[index]].layout { + Some(layout) if layout.size.width > 0 && layout.size.height > 0 => layout, + _ => { + unordered.remove(index); + continue; + } }; let top = layout.origin.y; let bottom = top + layout.size.height; diff --git a/src/widgets/button.rs b/src/widgets/button.rs index 7214b04..76d4574 100644 --- a/src/widgets/button.rs +++ b/src/widgets/button.rs @@ -3,7 +3,7 @@ use std::time::Duration; use figures::units::{Lp, Px, UPx}; use figures::{IntoSigned, Point, Rect, Round, ScreenScale, Size}; -use kludgine::app::winit::event::{DeviceId, ElementState, KeyEvent, MouseButton}; +use kludgine::app::winit::event::{DeviceId, MouseButton}; use kludgine::app::winit::window::CursorIcon; use kludgine::shapes::{Shape, StrokeOptions}; use kludgine::Color; @@ -19,9 +19,8 @@ use crate::styles::components::{ HighlightColor, IntrinsicPadding, OpaqueWidgetColor, OutlineColor, SurfaceColor, TextColor, }; use crate::styles::{ColorExt, Styles}; -use crate::utils::ModifiersExt; use crate::value::{Dynamic, IntoValue, Value}; -use crate::widget::{Callback, EventHandling, MakeWidget, Widget, WidgetRef, HANDLED, IGNORED}; +use crate::widget::{Callback, EventHandling, MakeWidget, Widget, WidgetRef, HANDLED}; use crate::FitMeasuredSize; /// A clickable button. @@ -484,34 +483,6 @@ impl Widget for Button { size + double_padding } - fn keyboard_input( - &mut self, - _device_id: DeviceId, - input: KeyEvent, - _is_synthetic: bool, - context: &mut EventContext<'_, '_>, - ) -> EventHandling { - if input.text.as_deref() == Some(" ") && !context.modifiers().possible_shortcut() { - let changed = match input.state { - ElementState::Pressed => { - let changed = context.activate(); - if !changed { - // The widget was already active. This is now a repeated keypress - self.invoke_on_click(context); - } - changed - } - ElementState::Released => context.deactivate(), - }; - if changed { - context.set_needs_redraw(); - } - HANDLED - } else { - IGNORED - } - } - fn unhover(&mut self, context: &mut EventContext<'_, '_>) { self.update_colors(context, false); } diff --git a/src/widgets/disclose.rs b/src/widgets/disclose.rs index f5e94c4..86c6060 100644 --- a/src/widgets/disclose.rs +++ b/src/widgets/disclose.rs @@ -11,7 +11,7 @@ use kludgine::{Color, DrawableExt}; use super::button::{ButtonActiveBackground, ButtonBackground, ButtonHoverBackground}; use crate::animation::{AnimationHandle, AnimationTarget, Spawn}; use crate::context::LayoutContext; -use crate::styles::components::{HighlightColor, IntrinsicPadding, OutlineColor, TextSize}; +use crate::styles::components::{HighlightColor, IntrinsicPadding, LineHeight, OutlineColor}; use crate::styles::Dimension; use crate::value::{Dynamic, IntoDynamic, IntoValue, Value}; use crate::widget::{ @@ -61,9 +61,7 @@ impl MakeWidgetWithTag for Disclose { fn make_with_tag(self, tag: WidgetTag) -> WidgetInstance { let collapsed = self.collapsed.into_dynamic(); - DiscloseIndicator::new(collapsed.clone(), self.label, self.contents) - .make_with_tag(tag) - .make_widget() + DiscloseIndicator::new(collapsed.clone(), self.label, self.contents).make_with_tag(tag) } } @@ -135,6 +133,8 @@ impl DiscloseIndicator { }; let stroke_color = if self.hovering_indicator { context.get(&OutlineColor) + } else if context.focused(true) { + context.get(&HighlightColor) } else { context.get(&OutlineColor).with_alpha(0) }; @@ -142,6 +142,7 @@ impl DiscloseIndicator { if self.target_colors.is_none() { self.target_colors = Some(target_colors); self.color.set(current_color); + self.stroke_color.set(stroke_color); } else if self.target_colors != Some(target_colors) { self.target_colors = Some(target_colors); self.color_animation = ( @@ -167,9 +168,11 @@ impl Widget for DiscloseIndicator { .get(&IndicatorSize) .into_px(context.gfx.scale()) .round(); - let stroke = StrokeOptions::px_wide(Lp::points(2).into_px(context.gfx.scale()).round()); + let stroke_options = + StrokeOptions::px_wide(Lp::points(1).into_px(context.gfx.scale()).round()) + .colored(stroke_color); - let radius = ((size - stroke.line_width) / 2).round(); + let radius = ((size - stroke_options.line_width) / 2).round(); let pt1 = Point::new(radius, Px::ZERO).rotate_by(Angle::degrees(0)); let pt2 = Point::new(radius, Px::ZERO).rotate_by(Angle::degrees(120)); let pt3 = Point::new(radius, Px::ZERO).rotate_by(Angle::degrees(240)); @@ -194,12 +197,6 @@ impl Widget for DiscloseIndicator { .gfx .draw_shape(path.fill(color).translate_by(center).rotate_by(angle)); - let stroke_options = if context.focused(true) { - stroke.colored(context.get(&HighlightColor)) - } else { - StrokeOptions::px_wide(Lp::points(1).into_px(context.gfx.scale()).round()) - .colored(stroke_color) - }; context.gfx.draw_shape( path.stroke(stroke_options) .translate_by(center) @@ -266,6 +263,18 @@ impl Widget for DiscloseIndicator { ) } + fn accept_focus(&mut self, _context: &mut crate::context::EventContext<'_, '_>) -> bool { + true + } + + fn focus(&mut self, context: &mut crate::context::EventContext<'_, '_>) { + context.set_needs_redraw(); + } + + fn blur(&mut self, context: &mut crate::context::EventContext<'_, '_>) { + context.set_needs_redraw(); + } + fn hit_test( &mut self, location: Point, @@ -292,7 +301,7 @@ impl Widget for DiscloseIndicator { let hovering = self.hit_test(location, context); if self.hovering_indicator != hovering { context.set_needs_redraw(); - self.hovering_indicator = true; + self.hovering_indicator = hovering; } hovering.then_some(CursorIcon::Pointer) @@ -315,6 +324,7 @@ impl Widget for DiscloseIndicator { if self.hit_test(location, context) { self.mouse_buttons_pressed += 1; self.activate(context); + context.focus(); HANDLED } else { IGNORED @@ -345,6 +355,6 @@ impl Widget for DiscloseIndicator { define_components! { Disclose { /// The size to render a [`Disclose`] indicator. - IndicatorSize(Dimension, "size", @TextSize) + IndicatorSize(Dimension, "size", @LineHeight) } } diff --git a/src/window.rs b/src/window.rs index b2bb983..a62e7fb 100644 --- a/src/window.rs +++ b/src/window.rs @@ -666,6 +666,100 @@ where ); fonts } + + fn handle_window_keyboard_input( + &mut self, + window: &mut RunningWindow<'_>, + kludgine: &mut Kludgine, + input: KeyEvent, + ) { + match input.logical_key { + Key::Character(ch) if ch == "w" && window.modifiers().primary() => { + if input.state.is_pressed() + && Self::request_close(&mut self.should_close, &mut self.behavior, window) + { + window.set_needs_redraw(); + } + } + Key::Named(NamedKey::Space) if !window.modifiers().possible_shortcut() => { + let target = self.tree.focused_widget().unwrap_or(self.root.node_id); + let target = self.tree.widget_from_node(target).expect("missing widget"); + let mut target = EventContext::new( + WidgetContext::new( + target, + &self.current_theme, + window, + self.theme_mode.get(), + &mut self.cursor, + ), + kludgine, + ); + + match input.state { + ElementState::Pressed => { + if target.active() { + target.deactivate(); + target.apply_pending_state(); + } + target.activate(); + } + ElementState::Released => { + target.deactivate(); + } + } + } + + Key::Named(NamedKey::Tab) if !window.modifiers().possible_shortcut() => { + if input.state.is_pressed() { + let reverse = window.modifiers().state().shift_key(); + + let target = self.tree.focused_widget().unwrap_or(self.root.node_id); + let target = self.tree.widget_from_node(target).expect("missing widget"); + let mut target = EventContext::new( + WidgetContext::new( + target, + &self.current_theme, + window, + self.theme_mode.get(), + &mut self.cursor, + ), + kludgine, + ); + + if reverse { + target.return_focus(); + } else { + target.advance_focus(); + } + } + } + Key::Named(NamedKey::Enter) => { + self.keyboard_activate_widget( + input.state.is_pressed(), + self.tree.default_widget(), + window, + kludgine, + ); + } + Key::Named(NamedKey::Escape) => { + self.keyboard_activate_widget( + input.state.is_pressed(), + self.tree.escape_widget(), + window, + kludgine, + ); + } + _ => { + tracing::event!( + Level::DEBUG, + logical = ?input.logical_key, + physical = ?input.physical_key, + state = ?input.state, + "Ignored Keyboard Input", + ); + } + } + } } #[derive(Clone, Copy, Eq, PartialEq, Debug)] @@ -1011,68 +1105,7 @@ where drop(target); if !handled { - match input.logical_key { - Key::Character(ch) if ch == "w" && window.modifiers().primary() => { - if input.state.is_pressed() - && Self::request_close( - &mut self.should_close, - &mut self.behavior, - &mut window, - ) - { - window.set_needs_redraw(); - } - } - Key::Named(NamedKey::Tab) if !window.modifiers().possible_shortcut() => { - if input.state.is_pressed() { - let reverse = window.modifiers().state().shift_key(); - - let target = self.tree.focused_widget().unwrap_or(self.root.node_id); - let target = self.tree.widget_from_node(target).expect("missing widget"); - let mut target = EventContext::new( - WidgetContext::new( - target, - &self.current_theme, - &mut window, - self.theme_mode.get(), - &mut self.cursor, - ), - kludgine, - ); - - if reverse { - target.return_focus(); - } else { - target.advance_focus(); - } - } - } - Key::Named(NamedKey::Enter) => { - self.keyboard_activate_widget( - input.state.is_pressed(), - self.tree.default_widget(), - &mut window, - kludgine, - ); - } - Key::Named(NamedKey::Escape) => { - self.keyboard_activate_widget( - input.state.is_pressed(), - self.tree.escape_widget(), - &mut window, - kludgine, - ); - } - _ => { - tracing::event!( - Level::DEBUG, - logical = ?input.logical_key, - physical = ?input.physical_key, - state = ?input.state, - "Ignored Keyboard Input", - ); - } - } + self.handle_window_keyboard_input(&mut window, kludgine, input); } }