Repository: zng-ui/zng (language: rust)

Issue:
TabNav::Cycle with single child FocusScopeOnFocus::LastFocused does not appear to work
<!--
Please, make sure:

- The issue happens in the latest crate release or newer (master branch).
- The issue happens after `cargo update`.
-->

#### I tried this code:

```rust
Window! {
    zng::focus::tab_nav = zng::focus::TabNav::Cycle;
    child = Wrap! {
        zng::focus::focus_scope = true;
        zng::focus::focus_scope_behavior = zng::focus::FocusScopeOnFocus::LastFocused;
        children = ui_vec![
            Button! { child = Text!("Focusable 1") },
            Button! { child = Text!("Focusable 2") },
        ];
    };
}
```

#### Following these steps:

1 - Run.
2 - Press Tab twice.

#### I saw this happen:

Focus stays in "Focusable 2".

#### I expected this to happen:

Focus to cycle.

#### Notes

This is technically not a bug, the focus does leave the child scope and cycles back to it instantly, but in practice the effect is surprising, specially when the child scope is a `Scroll!` as it defines the focus_scope_behavior by default.


Discussion / hints:

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 42c7c5e03..1e9a177ab 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -43,6 +43,13 @@ fn version_0_5() -> impl UiNode {
 * **Breaking** Rename `VarCapabilities` to `VarCapability`.
 * Fix `zng::view_process::default::run_same_process` not propagating app panics.
 * Add `WindowCloseRequestedArgs::headed/headless`.
+* **Breaking** Fix tab navigation footgun when a focus scope with `FocusScopeOnFocus::LastFocused` is the single child of
+  another scope with `TabNav::Cycle` (#162).
+    - Breaking change has minimal impact:
+        - Added input in `WidgetFocusInfo::on_focus_scope_move`.
+        - Removed `FocusChangedCause::is_prev_request`.
+* Add `FocusChangedCause::request_target` helper method.
+* Add `WidgetPath::parent_id` helper method.
 
 # 0.4.0
 
diff --git a/crates/zng-app/src/widget/info/path.rs b/crates/zng-app/src/widget/info/path.rs
index 5f3542f65..47fe35c6b 100644
--- a/crates/zng-app/src/widget/info/path.rs
+++ b/crates/zng-app/src/widget/info/path.rs
@@ -69,6 +69,11 @@ impl WidgetPath {
         self.path[self.path.len() - 1]
     }
 
+    /// The widget parent, if it is not the root widget.
+    pub fn parent_id(&self) -> Option<WidgetId> {
+        self.ancestors().iter().copied().next_back()
+    }
+
     /// [`ancestors`](WidgetPath::ancestors) and [`widget_id`](WidgetPath::widget_id), root first.
     pub fn widgets_path(&self) -> &[WidgetId] {
         &self.path[..]
diff --git a/crates/zng-ext-input/src/focus.rs b/crates/zng-ext-input/src/focus.rs
index 959a0868f..08d4e0831 100644
--- a/crates/zng-ext-input/src/focus.rs
+++ b/crates/zng-ext-input/src/focus.rs
@@ -261,11 +261,11 @@ pub enum FocusChangedCause {
     Recovery,
 }
 impl FocusChangedCause {
-    /// If the change cause by a request to the previous widget, by tab index.
-    pub fn is_prev_request(self) -> bool {
+    /// Get focus request target.
+    pub fn request_target(self) -> Option<FocusTarget> {
         match self {
-            FocusChangedCause::Request(r) => matches!(r.target, FocusTarget::Prev),
-            _ => false,
+            Self::Request(r) => Some(r.target),
+            _ => None,
         }
     }
 }
@@ -501,12 +501,19 @@ impl FocusManager {
                 focus.is_highlighting_var.set(true);
             }
 
-            let reverse = args.cause.is_prev_request();
+            // reentering single child of parent scope that cycles
+            let is_tab_cycle_reentry = matches!(args.cause.request_target(), Some(FocusTarget::Prev | FocusTarget::Next))
+                && match (&args.prev_focus, &args.new_focus) {
+                    (Some(p), Some(n)) => Some(n.widget_id()) == p.parent_id(),
+                    _ => false,
+                };
+
+            let reverse = matches!(args.cause.request_target(), Some(FocusTarget::Prev));
             let prev_focus = args.prev_focus.clone();
             FOCUS_CHANGED_EVENT.notify(args);
 
             // may have focused scope.
-            while let Some(after_args) = focus.move_after_focus(reverse) {
+            while let Some(after_args) = focus.move_after_focus(is_tab_cycle_reentry, reverse) {
                 FOCUS_CHANGED_EVENT.notify(after_args);
             }
 
@@ -1332,14 +1339,15 @@ impl FocusService {
     }
 
     #[must_use]
-    fn move_after_focus(&mut self, reverse: bool) -> Option<FocusChangedArgs> {
+    fn move_after_focus(&mut self, is_tab_cycle_reentry: bool, reverse: bool) -> Option<FocusChangedArgs> {
         if let Some(focused) = &self.focused {
             if let Some(info) = WINDOWS.focused_info() {
                 if let Some(widget) = FocusInfoTree::new(info, self.focus_disabled_widgets.get(), self.focus_hidden_widgets.get())
                     .get(focused.path.widget_id())
                 {
                     if widget.is_scope() {
-                        if let Some(widget) = widget.on_focus_scope_move(|id| self.return_focused.get(&id).map(|p| p.as_path()), reverse) {
+                        let last_focused = |id| self.return_focused.get(&id).map(|p| p.as_path());
+                        if let Some(widget) = widget.on_focus_scope_move(last_focused, is_tab_cycle_reentry, reverse) {
                             self.enabled_nav = widget.enabled_nav_with_frame();
                             return self.move_focus(
                                 Some(FocusedInfo::new(widget)),
diff --git a/crates/zng-ext-input/src/focus/focus_info.rs b/crates/zng-ext-input/src/focus/focus_info.rs
index a54405714..a645bd232 100644
--- a/crates/zng-ext-input/src/focus/focus_info.rs
+++ b/crates/zng-ext-input/src/focus/focus_info.rs
@@ -844,7 +844,8 @@ impl WidgetFocusInfo {
     /// # Input
     ///
     /// * `last_focused`: A function that returns the last focused widget within a focus scope identified by `WidgetId`.
-    /// * `reverse`: If the focus is *reversing* into `self`.
+    /// * `is_tab_cycle_reentry`: If the focus returned to `self` immediately after leaving because the parent scope is `TabNav::Cycle`.
+    /// * `reverse`: If the focus *reversed* into `self`.
     ///
     /// # Returns
     ///
@@ -854,6 +855,7 @@ impl WidgetFocusInfo {
     pub fn on_focus_scope_move<'f>(
         &self,
         last_focused: impl FnOnce(WidgetId) -> Option<&'f WidgetPath>,
+        is_tab_cycle_reentry: bool,
         reverse: bool,
     ) -> Option<WidgetFocusInfo> {
         match self.focus_info() {
@@ -866,23 +868,25 @@ impl WidgetFocusInfo {
                             self.first_tab_descendant()
                         }
                     }
-                    FocusScopeOnFocus::LastFocused | FocusScopeOnFocus::LastFocusedIgnoreBounds => last_focused(self.info.id())
-                        .and_then(|path| self.info.tree().get(path.widget_id()))
-                        .and_then(|w| w.into_focusable(self.focus_disabled_widgets(), self.focus_hidden_widgets()))
-                        .and_then(|f| {
-                            if f.info.is_descendant(&self.info) {
-                                Some(f) // valid last focused
-                            } else {
-                                None
-                            }
-                        })
-                        .or_else(|| {
-                            if reverse {
-                                self.last_tab_descendant()
-                            } else {
-                                self.first_tab_descendant()
-                            }
-                        }), // fallback
+                    FocusScopeOnFocus::LastFocused | FocusScopeOnFocus::LastFocusedIgnoreBounds => {
+                        if is_tab_cycle_reentry { None } else { last_focused(self.info.id()) }
+                            .and_then(|path| self.info.tree().get(path.widget_id()))
+                            .and_then(|w| w.into_focusable(self.focus_disabled_widgets(), self.focus_hidden_widgets()))
+                            .and_then(|f| {
+                                if f.info.is_descendant(&self.info) {
+                                    Some(f) // valid last focused
+                                } else {
+                                    None
+                                }
+                            })
+                            .or_else(|| {
+                                if reverse {
+                                    self.last_tab_descendant()
+                                } else {
+                                    self.first_tab_descendant()
+                                }
+                            })
+                    } // fallback
                     FocusScopeOnFocus::Widget => None,
                 };
 
@@ -1739,6 +1743,9 @@ pub enum FocusScopeOnFocus {
     /// Focus the descendant that was last focused before focus moved out of the scope. If the
     /// scope cannot return focus, behaves like [`FirstDescendant`].
     ///
+    /// If the scope is the only child of a parent that is `TabNav::Cycle` and the focus just exited and
+    /// returned in a cycle action, behaves like [`FirstDescendant`].
+    ///
     /// Behaves like [`Widget`] if the first(or last) descendant inner-bounds is not fully contained
     /// by the scope inner-bounds.
     ///


Test patch: