Root tab order fix, Spacebar widget activiation

Closes #99

Disclose now accepts focus and responds to spacebar as a result of this.

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
 

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")

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<WidgetId> {
+        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) {

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 {
+                let layout = match &data.nodes[unordered[index]].layout {
-                    unordered.remove(index);
+                    Some(layout) if layout.size.width > 0 && layout.size.height > 0 => layout,
-                    continue;
+                    _ => {
+                        unordered.remove(index);
+                        continue;
+                    }
                 };
                 let top = layout.origin.y;
                 let bottom = top + layout.size.height;

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);
     }

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)
+        DiscloseIndicator::new(collapsed.clone(), self.label, self.contents).make_with_tag(tag)
-            .make_with_tag(tag)
-            .make_widget()
     }
 }
 
@@ -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<Px>,
@@ -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)
     }
 }

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 {
+            self.handle_window_keyboard_input(&mut window, kludgine, input);
-                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",
-                    );
-                }
-            }
         }
     }