From 3a4b37a8ff33768496c5b6ae2b7a7aac4f7f3757 Mon Sep 17 00:00:00 2001
From: Jonathan Johnson <jon@khonsulabs.com>
Date: Thu, 28 Nov 2024 07:31:16 -0800
Subject: [PATCH] Fixed shift-tab to direct parent

Closes #224
---
 CHANGELOG.md   |  3 +++
 src/context.rs | 47 ++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 17a246aee..720633a47 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -186,6 +186,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Widgets of 0-size are now still rendered, even though all the drawing
   operations will be clipped away. This allows the `ComponentProbe` to still
   actively probe when placed in a location and given no space.
+- Focus order when reversing now correctly evaluates direct parent widgets.
+  Prior to this change, placing a button inside of a button would not allow
+  shift-tab to focus the outer button when focus was on the inner button.
 
 ### Added
 
diff --git a/src/context.rs b/src/context.rs
index 56368b1f8..0c3b6ece8 100644
--- a/src/context.rs
+++ b/src/context.rs
@@ -340,10 +340,14 @@ impl<'context> EventContext<'context> {
     }
 
     fn next_focus_after(&mut self, mut focus: MountedWidget, advance: bool) -> Option<WidgetId> {
-        // First, look within the current focus for any focusable children.
+        // First, look within the current focus for any focusable children if we
+        // are advancing the focus. If we are reversing, we will wait to
+        // consider children as part of the reverse scan.
         let stop_at = focus.id();
-        if let Some(focus) = self.next_focus_within(&focus, None, stop_at, advance) {
-            return Some(focus);
+        if advance {
+            if let Some(focus) = self.next_focus_within(&focus, None, stop_at, advance) {
+                return Some(focus);
+            }
         }
 
         // Now, look for the next widget in each hierarchy
@@ -354,6 +358,16 @@ impl<'context> EventContext<'context> {
             let Some(parent) = focus.parent() else {
                 break focus;
             };
+            // If we're reversing focus, we need to consider the parent itself
+            // as a focus target.
+            let accept_focus = !advance
+                && parent
+                    .lock()
+                    .as_widget()
+                    .accept_focus(&mut self.for_other(&parent));
+            if accept_focus {
+                return Some(parent.id());
+            }
             focus = parent;
         };
 
@@ -415,13 +429,18 @@ impl<'context> EventContext<'context> {
             children.next();
         }
 
+        // Check each child if it can accept focus. When advancing, this is done
+        // before evaluating the children's children, but when reversing this is
+        // done after evaluating the children's children.
         for child in children {
-            let mut child_context = self.for_other(&child);
-            let accept_focus = child.lock().as_widget().accept_focus(&mut child_context);
-            drop(child_context);
+            let accept_focus = advance
+                && child
+                    .lock()
+                    .as_widget()
+                    .accept_focus(&mut self.for_other(&child));
             if accept_focus {
                 return Some(child.id());
-            } else if stop_at == child.id() {
+            } else if stop_at == child.id() && advance {
                 // We cycled completely, and the original widget didn't accept
                 // focus.
                 return None;
@@ -429,6 +448,20 @@ impl<'context> EventContext<'context> {
                 return Some(next_focus.id());
             } else if let Some(focus) = self.next_focus_within(&child, None, stop_at, advance) {
                 return Some(focus);
+            } else if !advance {
+                // When we are reversing, we needed to evaluate this child's
+                // children first. After checking its children, we need to also
+                // check that this isn't the stop_at widget before we try
+                // focusing this child.
+                if stop_at == child.id() {
+                    return None;
+                } else if child
+                    .lock()
+                    .as_widget()
+                    .accept_focus(&mut self.for_other(&child))
+                {
+                    return Some(child.id());
+                }
             }
         }