BUGFIX: tab index from defered command

Author: togglebyte

anathema-backend/src/lib.rs

@@ -60,8 +60,7 @@ impl<'rt, 'bp, T: Backend> WidgetCycle<'rt, 'bp, T> {
         //   - Layout -
         // -----------------------------------------------------------------------------
         if needs_layout {
-            let filter = LayoutFilter::all();
-            self.layout(ctx, filter)?;
+            self.layout(ctx, LayoutFilter)?;
         }
 
         // -----------------------------------------------------------------------------

anathema-backend/src/testing/events.rs

@@ -0,0 +1,47 @@
+use std::collections::VecDeque;
+
+use anathema_widgets::components::events::{ComponentEvent, KeyEvent};
+
+pub struct EventsMut<'a> {
+    event_queue: &'a mut VecDeque<Option<ComponentEvent>>,
+}
+
+impl EventsMut<'_> {
+    pub fn next(self) -> Self {
+        self.event_queue.push_back(None);
+        self
+    }
+
+    pub fn stop(self) -> Self {
+        self.event_queue.push_back(Some(ComponentEvent::Stop));
+        self
+    }
+
+    pub fn press(self, event: KeyEvent) -> Self {
+        self.event_queue.push_back(Some(ComponentEvent::Key(event)));
+        self
+    }
+}
+
+#[derive(Debug)]
+pub struct Events {
+    event_queue: VecDeque<Option<ComponentEvent>>,
+}
+
+impl Events {
+    pub(super) fn new() -> Self {
+        Self {
+            event_queue: VecDeque::new(),
+        }
+    }
+
+    pub(super) fn mut_ref(&mut self) -> EventsMut<'_> {
+        EventsMut {
+            event_queue: &mut self.event_queue,
+        }
+    }
+
+    pub(super) fn pop(&mut self) -> Option<ComponentEvent> {
+        self.event_queue.pop_front().flatten()
+    }
+}

anathema-backend/src/testing/mod.rs

@@ -1,4 +1,3 @@
-use std::collections::VecDeque;
 use std::time::Duration;
 
 use anathema_geometry::Size;
@@ -10,13 +9,16 @@ use surface::TestSurface;
 
 use crate::Backend;
 
+mod events;
 mod surface;
 
-pub struct GlyphSomething<'a> {
+#[derive(Debug)]
+/// This is used for testing
+pub struct GlyphRef<'a> {
     inner: Option<&'a Glyph>,
 }
 
-impl<'a> GlyphSomething<'a> {
+impl<'a> GlyphRef<'a> {
     pub fn is_char(&self, rhs: char) -> bool {
         let Some(glyph) = self.inner else { return false };
         match glyph {
@@ -29,32 +31,40 @@ impl<'a> GlyphSomething<'a> {
 #[derive(Debug)]
 pub struct TestBackend {
     surface: TestSurface,
-    event_queue: VecDeque<Option<ComponentEvent>>,
+    events: events::Events,
 }
 
 impl TestBackend {
     pub fn new(size: impl Into<Size>) -> Self {
         let size = size.into();
         Self {
             surface: TestSurface::new(size),
-            // NOTE:
-            // we have to start by return None for the first event,
-            // as the first frame tick is there to populate the widget tree.
-            //
-            // This is required for tab indexing
-            event_queue: VecDeque::from([None]),
+            events: events::Events::new(),
         }
     }
 
-    pub fn add_event(&mut self, event: Option<ComponentEvent>) {
-        self.event_queue.push_back(event);
-    }
-
-    pub fn at(&self, x: usize, y: usize) -> GlyphSomething<'_> {
-        GlyphSomething {
+    pub fn at(&self, x: usize, y: usize) -> GlyphRef<'_> {
+        GlyphRef {
             inner: self.surface.get(x, y),
         }
     }
+
+    pub fn line(&self, index: usize) -> String {
+        let glyphs = self.surface.line(index);
+        glyphs
+            .iter()
+            .filter_map(|g| match g {
+                Glyph::Single(c, _) => Some(c),
+                Glyph::Cluster(_, _) => None,
+            })
+            .collect::<String>()
+            .trim()
+            .to_string()
+    }
+
+    pub fn events(&mut self) -> events::EventsMut<'_> {
+        self.events.mut_ref()
+    }
 }
 
 impl Backend for TestBackend {
@@ -63,7 +73,7 @@ impl Backend for TestBackend {
     }
 
     fn next_event(&mut self, _timeout: Duration) -> Option<ComponentEvent> {
-        self.event_queue.pop_front()?
+        self.events.pop()
     }
 
     fn resize(&mut self, new_size: Size, glyph_map: &mut GlyphMap) {
@@ -76,6 +86,7 @@ impl Backend for TestBackend {
         widgets: PaintChildren<'_, 'bp>,
         attribute_storage: &AttributeStorage<'bp>,
     ) {
+        self.surface.clear();
         paint(&mut self.surface, glyph_map, widgets, attribute_storage);
     }
 

anathema-backend/src/testing/surface.rs

@@ -7,29 +7,51 @@ use anathema_widgets::{GlyphMap, Style, WidgetRenderer};
 pub struct TestSurface {
     pub(super) size: Size,
     lines: Vec<Glyph>,
+    prev_lines: Vec<Glyph>,
 }
 
 impl TestSurface {
     pub fn new(size: Size) -> Self {
         let len = size.width * size.height;
         let lines = vec![Glyph::space(); len as usize];
-        Self { size, lines }
+        Self {
+            size,
+            lines,
+            prev_lines: vec![],
+        }
     }
 
     pub(crate) fn resize(&mut self, new_size: Size, _glyph_map: &mut GlyphMap) {
         self.size = new_size;
         todo!("truncate and pop lines outside of the new size")
     }
 
+    // Get from previous lines as the clear function has most likely
+    // been called by the time this is relevant
     pub(crate) fn get(&self, x: usize, y: usize) -> Option<&Glyph> {
         let index = y * self.size.width as usize + x;
-        self.lines.get(index)
+        self.prev_lines.get(index)
+    }
+
+    // Look at the old lines as the surface has been cleared
+    // by the time we can inspect the lines
+    pub(crate) fn line(&self, index: usize) -> &[Glyph] {
+        let from = index * self.size.width as usize;
+        let to = from + self.size.width as usize;
+        &self.prev_lines[from..to]
+    }
+
+    pub(crate) fn clear(&mut self) {
+        let len = self.size.width * self.size.height;
+        let mut lines = vec![Glyph::space(); len as usize];
+        std::mem::swap(&mut lines, &mut self.lines);
+        std::mem::swap(&mut lines, &mut self.prev_lines);
     }
 }
 
 impl WidgetRenderer for TestSurface {
     fn draw_glyph(&mut self, mut glyph: Glyph, local_pos: Pos) {
-        let index = self.size.height as i32 * local_pos.y + local_pos.x;
+        let index = self.size.width as i32 * local_pos.y + local_pos.x;
         let index = index as usize;
         std::mem::swap(&mut glyph, &mut self.lines[index]);
     }

anathema-backend/src/tui/mod.rs

@@ -143,7 +143,6 @@ impl Backend for TuiBackend {
         attribute_storage: &AttributeStorage<'bp>,
     ) {
         anathema_widgets::paint::paint(&mut self.screen, glyph_map, widgets, attribute_storage);
-        // TODO: decide if we need `paint` to return a Result or not
     }
 
     fn render(&mut self, glyph_map: &mut GlyphMap) {

anathema-runtime/src/runtime/mod.rs

@@ -125,8 +125,7 @@ impl<G: GlobalEventHandler> Runtime<G> {
             // yet have any widgets or components.
             frame.tick(backend)?;
 
-            let mut changed = false;
-            let mut tabindex = TabIndex::new(&mut frame.tabindex, frame.tree.view_mut(), &mut changed);
+            let mut tabindex = TabIndex::new(&mut frame.tabindex, frame.tree.view_mut());
             tabindex.next();
 
             if let Some(current) = frame.tabindex.as_ref() {
@@ -231,21 +230,19 @@ pub struct Frame<'rt, 'bp, G> {
     needs_layout: bool,
     stop: bool,
     global_event_handler: &'rt G,
-    tabindex: Option<Index>,
+    pub tabindex: Option<Index>,
 }
 
 impl<'rt, 'bp, G: GlobalEventHandler> Frame<'rt, 'bp, G> {
     pub fn handle_global_event(&mut self, event: ComponentEvent) -> Option<ComponentEvent> {
-        let mut changed = false;
-        let mut tabindex = TabIndex::new(&mut self.tabindex, self.tree.view_mut(), &mut changed);
+        let mut tabindex = TabIndex::new(&mut self.tabindex, self.tree.view_mut());
 
         let event = self
             .global_event_handler
             .handle(event, &mut tabindex, self.deferred_components);
 
-        let prev = tabindex.consume();
-
-        if changed {
+        if tabindex.changed {
+            let prev = tabindex.consume();
             if let Some(prev) = prev {
                 self.with_component(prev.widget_id, prev.state_id, |comp, children, ctx| {
                     comp.dyn_component.any_blur(children, ctx)
@@ -315,6 +312,7 @@ impl<'rt, 'bp, G: GlobalEventHandler> Frame<'rt, 'bp, G> {
         puffin::GlobalProfiler::lock().new_frame();
 
         let now = Instant::now();
+        self.cycle(backend)?;
         self.tick_components(self.dt.elapsed());
         let elapsed = self.handle_messages(now);
         self.poll_events(elapsed, now, backend);
@@ -324,7 +322,6 @@ impl<'rt, 'bp, G: GlobalEventHandler> Frame<'rt, 'bp, G> {
         // TODO: this secondary call is here to deal with changes causing changes
         //       which happens when values are removed or inserted and indices needs updating
         self.apply_changes()?;
-        self.cycle(backend)?;
 
         *self.dt = Instant::now();
         Ok(now.elapsed())
@@ -407,15 +404,6 @@ impl<'rt, 'bp, G: GlobalEventHandler> Frame<'rt, 'bp, G> {
         // Nb: Add drain_into to DeferredComponents
         let commands = self.deferred_components.drain().collect::<Vec<_>>();
         for mut cmd in commands {
-            // Blur the current component if the message is a `Focus` message
-            if let CommandKind::Focus = cmd.kind {
-                if let Some(current) = self.tabindex.take() {
-                    self.with_component(current.widget_id, current.state_id, |comp, children, ctx| {
-                        comp.dyn_component.any_blur(children, ctx)
-                    });
-                }
-            }
-
             for index in 0..self.layout_ctx.components.len() {
                 let Some((widget_id, state_id)) = self.layout_ctx.components.get(index) else { continue };
                 let Some(comp) = self.tree.get_ref(widget_id) else { continue };
@@ -431,18 +419,59 @@ impl<'rt, 'bp, G: GlobalEventHandler> Frame<'rt, 'bp, G> {
                     continue;
                 }
 
-                self.with_component(widget_id, state_id, |comp, children, ctx| match cmd.kind {
-                    CommandKind::Focus => {
-                        // Some(Index {
-                        //     path: panic!(),//children.offset.into(),
-                        //     index: comp.tabindex,
-                        //     widget_id,
-                        //     state_id,
-                        // });
-                        comp.dyn_component.any_focus(children, ctx);
+                // -----------------------------------------------------------------------------
+                //   - Set focus -
+                //   TODO: here is another candidate for refactoring to make it
+                //   less cludgy and verbose.
+                // -----------------------------------------------------------------------------
+                // Blur the current component if the message is a `Focus` message
+                if let CommandKind::Focus = cmd.kind {
+                    // If this component current has focus ignore this command
+                    if let Some(index) = self.tabindex.as_ref() {
+                        if index.widget_id == widget_id {
+                            continue;
+                        }
                     }
-                    CommandKind::SendMessage(msg) => comp.dyn_component.any_message(children, ctx, msg),
-                });
+
+                    // here we can find the component that should receive focus
+                    let new_index = self
+                        .with_component(widget_id, state_id, |comp, children, ctx| {
+                            if comp.dyn_component.any_accept_focus() {
+                                let index = Index {
+                                    path: children.parent_path().into(),
+                                    index: comp.tabindex,
+                                    widget_id,
+                                    state_id,
+                                };
+
+                                comp.dyn_component.any_focus(children, ctx);
+
+                                Some(index)
+                            } else {
+                                None
+                            }
+                        })
+                        .flatten();
+
+                    if let Some(index) = new_index {
+                        // If there is currently a component with focus that component
+                        // should only lose focus if the selected component accepts focus.
+                        if let Some(old) = self.tabindex.replace(index) {
+                            self.with_component(old.widget_id, old.state_id, |comp, children, ctx| {
+                                comp.dyn_component.any_blur(children, ctx)
+                            });
+                        }
+                    }
+                }
+
+                // -----------------------------------------------------------------------------
+                //   - Send message -
+                // -----------------------------------------------------------------------------
+                if let CommandKind::SendMessage(msg) = cmd.kind {
+                    self.with_component(widget_id, state_id, |comp, children, ctx| {
+                        comp.dyn_component.any_message(children, ctx, msg);
+                    });
+                }
                 break;
             }
         }