Fixed #3786 - Focus

Author: tig

Terminal.Gui/Application/ApplicationNavigation.cs

@@ -77,6 +77,7 @@ internal void SetFocused (View? value)
         {
             return;
         }
+        Debug.Assert (value is null or { CanFocus: true, HasFocus: true });
 
         _focused = value;
 

Terminal.Gui/View/View.Hierarchy.cs

@@ -169,6 +169,13 @@ public virtual void OnRemoved (SuperViewChangedEventArgs e)
         Debug.Assert (!view.HasFocus);
 
         _subviews.Remove (view);
+
+        // Clean up focus stuff
+        _previouslyFocused = null;
+        if (view._superView is { } && view._superView._previouslyFocused == this)
+        {
+            view._superView._previouslyFocused = null;
+        }
         view._superView = null;
 
         SetNeedsLayout ();

Terminal.Gui/View/View.Navigation.cs

@@ -180,7 +180,8 @@ public bool CanFocus
             if (!_canFocus && HasFocus)
             {
                 // If CanFocus is set to false and this view has focus, make it leave focus
-                HasFocus = false;
+                // Set traverssingdown so we don't go back up the hierachy...
+                SetHasFocusFalse (null, traversingDown: false);
             }
 
             if (_canFocus && !HasFocus && Visible && SuperView is { Focused: null })
@@ -296,7 +297,12 @@ internal bool RestoreFocus ()
 
         if (Focused is null && _previouslyFocused is { } && indicies.Contains (_previouslyFocused))
         {
-            return _previouslyFocused.SetFocus ();
+            if (_previouslyFocused.SetFocus ())
+            {
+                return true;
+            }
+
+            _previouslyFocused = null;
         }
 
         return false;
@@ -412,14 +418,14 @@ public bool SetFocus ()
     ///     other methods that
     ///     set or remove focus from a view.
     /// </summary>
-    /// <param name="previousFocusedView">
-    ///     The previously focused view. If <see langword="null"/> there is no previously focused
+    /// <param name="currentFocusedView">
+    ///     The currently focused view. If <see langword="null"/> there is no previously focused
     ///     view.
     /// </param>
     /// <param name="traversingUp"></param>
     /// <returns><see langword="true"/> if <see cref="HasFocus"/> was changed to <see langword="true"/>.</returns>
     /// <exception cref="InvalidOperationException"></exception>
-    private (bool focusSet, bool cancelled) SetHasFocusTrue (View? previousFocusedView, bool traversingUp = false)
+    private (bool focusSet, bool cancelled) SetHasFocusTrue (View? currentFocusedView, bool traversingUp = false)
     {
         Debug.Assert (SuperView is null || ApplicationNavigation.IsInHierarchy (SuperView, this));
 
@@ -429,6 +435,11 @@ public bool SetFocus ()
             return (false, false);
         }
 
+        if (currentFocusedView is { HasFocus: false })
+        {
+            throw new ArgumentException ("SetHasFocusTrue: currentFocusedView must HasFocus.");
+        }
+
         var thisAsAdornment = this as Adornment;
         View? superViewOrParent = thisAsAdornment?.Parent ?? SuperView;
 
@@ -451,7 +462,7 @@ public bool SetFocus ()
 
         bool previousValue = HasFocus;
 
-        bool cancelled = NotifyFocusChanging (false, true, previousFocusedView, this);
+        bool cancelled = NotifyFocusChanging (false, true, currentFocusedView, this);
 
         if (cancelled)
         {
@@ -462,7 +473,7 @@ public bool SetFocus ()
         // Any of them may cancel gaining focus. In which case we need to back out.
         if (superViewOrParent is { HasFocus: false } sv)
         {
-            (bool focusSet, bool svCancelled) = sv.SetHasFocusTrue (previousFocusedView, true);
+            (bool focusSet, bool svCancelled) = sv.SetHasFocusTrue (currentFocusedView, true);
 
             if (!focusSet)
             {
@@ -488,31 +499,33 @@ public bool SetFocus ()
 
         if (!traversingUp)
         {
-            // Restore focus to the previously focused subview 
+            // Restore focus to the previously focused subview, if any
             if (!RestoreFocus ())
             {
-                // Couldn't restore focus, so use Advance to navigate to the next focusable subview
-                if (!AdvanceFocus (NavigationDirection.Forward, null))
-                {
-                    // Couldn't advance, so we're the most focused view in the application
-                    Application.Navigation?.SetFocused (this);
-                }
+                Debug.Assert (_previouslyFocused is null);
+                // Couldn't restore focus, so use Advance to navigate to the next focusable subview, if any
+                AdvanceFocus (NavigationDirection.Forward, null);
             }
         }
 
-        if (previousFocusedView is { HasFocus: true } && GetFocusChain (NavigationDirection.Forward, TabStop).Contains (previousFocusedView))
+        // Now make sure the old focused view loses focus
+        if (currentFocusedView is { HasFocus: true } && GetFocusChain (NavigationDirection.Forward, TabStop).Contains (currentFocusedView))
         {
-            previousFocusedView.SetHasFocusFalse (this);
+            currentFocusedView.SetHasFocusFalse (this);
         }
 
-        _previouslyFocused = null;
+        if (_previouslyFocused is { })
+        {
+            _previouslyFocused = null;
+        }
 
         if (Arrangement.HasFlag (ViewArrangement.Overlapped))
         {
             SuperView?.MoveSubviewToEnd (this);
         }
 
-        NotifyFocusChanged (HasFocus, previousFocusedView, this);
+        // Focus work is done. Notify.
+        NotifyFocusChanged (HasFocus, currentFocusedView, this);
 
         SetNeedsDisplay ();
 
@@ -527,6 +540,9 @@ public bool SetFocus ()
 
     private bool NotifyFocusChanging (bool currentHasFocus, bool newHasFocus, View? currentFocused, View? newFocused)
     {
+        Debug.Assert (currentFocused is null || currentFocused is { HasFocus: true });
+        Debug.Assert (newFocused is null || newFocused is { CanFocus: true });
+
         // Call the virtual method
         if (OnHasFocusChanging (currentHasFocus, newHasFocus, currentFocused, newFocused))
         {
@@ -543,6 +559,13 @@ private bool NotifyFocusChanging (bool currentHasFocus, bool newHasFocus, View?
             return true;
         }
 
+        View? appFocused = Application.Navigation?.GetFocused ();
+
+        if (appFocused == currentFocused)
+        {
+            Application.Navigation?.SetFocused (null);
+        }
+
         return false;
     }
 
@@ -586,8 +609,8 @@ private bool NotifyFocusChanging (bool currentHasFocus, bool newHasFocus, View?
     ///     focused.
     /// </param>
     /// <param name="traversingDown">
-    ///     Set to true to indicate method is being called recurively, traversing down the focus
-    ///     chain.
+    ///     Set to true to traverse down the focus
+    ///     chain only. If false, the method will attempt to AdvanceFocus on the superview or restorefocus on Application.Navigation.GetFocused().
     /// </param>
     /// <exception cref="InvalidOperationException"></exception>
     private void SetHasFocusFalse (View? newFocusedView, bool traversingDown = false)
@@ -598,53 +621,60 @@ private void SetHasFocusFalse (View? newFocusedView, bool traversingDown = false
             throw new InvalidOperationException ("SetHasFocusFalse should not be called if the view does not have focus.");
         }
 
+        if (newFocusedView is { HasFocus: false })
+        {
+            throw new InvalidOperationException ("SetHasFocusFalse new focused view does not have focus.");
+        }
+
         var thisAsAdornment = this as Adornment;
         View? superViewOrParent = thisAsAdornment?.Parent ?? SuperView;
 
         // If newFocusedVew is null, we need to find the view that should get focus, and SetFocus on it.
         if (!traversingDown && newFocusedView is null)
         {
-            if (superViewOrParent?._previouslyFocused is { })
+            if (superViewOrParent?._previouslyFocused is { CanFocus: true })
             {
-                if (superViewOrParent._previouslyFocused != this)
+                if (superViewOrParent._previouslyFocused != this && superViewOrParent._previouslyFocused.SetFocus ())
                 {
-                    superViewOrParent?._previouslyFocused?.SetFocus ();
-
                     // The above will cause SetHasFocusFalse, so we can return
+                    Debug.Assert (!_hasFocus);
                     return;
                 }
             }
 
-            if (superViewOrParent is { })
+            if (superViewOrParent is { CanFocus: true })
             {
                 if (superViewOrParent.AdvanceFocus (NavigationDirection.Forward, TabStop))
                 {
                     // The above will cause SetHasFocusFalse, so we can return
+                    Debug.Assert (!_hasFocus);
                     return;
                 }
 
-                newFocusedView = superViewOrParent;
+                if (superViewOrParent is { HasFocus: true, CanFocus: true })
+                {
+                    newFocusedView = superViewOrParent;
+                }
             }
 
-            if (Application.Navigation is { } && Application.Top is { })
+            if (Application.Navigation is { } && Application.Navigation.GetFocused () is { CanFocus: true })
             {
                 // Temporarily ensure this view can't get focus
                 bool prevCanFocus = _canFocus;
                 _canFocus = false;
-                bool restoredFocus = Application.Top!.RestoreFocus ();
+                bool restoredFocus = Application.Navigation.GetFocused ()!.RestoreFocus ();
                 _canFocus = prevCanFocus;
 
                 if (restoredFocus)
                 {
                     // The above caused SetHasFocusFalse, so we can return
+                    Debug.Assert (!_hasFocus);
                     return;
                 }
             }
-
             // No other focusable view to be found. Just "leave" us...
         }
 
-
         // Before we can leave focus, we need to make sure that all views down the subview-hierarchy have left focus.
         View? mostFocused = MostFocused;
 
@@ -665,15 +695,7 @@ private void SetHasFocusFalse (View? newFocusedView, bool traversingDown = false
                 bottom = bottom.SuperView;
             }
 
-            if (bottom == this && bottom.SuperView is Adornment a)
-            {
-                //a.SetHasFocusFalse (newFocusedView, true);
-
-                Debug.Assert (_hasFocus);
-            }
-
             Debug.Assert (_hasFocus);
-
         }
 
         if (superViewOrParent is { })
@@ -684,24 +706,16 @@ private void SetHasFocusFalse (View? newFocusedView, bool traversingDown = false
         bool previousValue = HasFocus;
 
         // Note, can't be cancelled.
-        NotifyFocusChanging (HasFocus, !HasFocus, newFocusedView, this);
+        NotifyFocusChanging (HasFocus, !HasFocus, this, newFocusedView);
+
+        Debug.Assert (_hasFocus);
 
         // Get whatever peer has focus, if any so we can update our superview's _previouslyMostFocused
         View? focusedPeer = superViewOrParent?.Focused;
 
         // Set HasFocus false
         _hasFocus = false;
 
-        if (Application.Navigation is { })
-        {
-            View? appFocused = Application.Navigation.GetFocused ();
-
-            if (appFocused is { } || appFocused == this)
-            {
-                Application.Navigation.SetFocused (newFocusedView ?? superViewOrParent);
-            }
-        }
-
         NotifyFocusChanged (HasFocus, this, newFocusedView);
 
         if (_hasFocus)
@@ -721,6 +735,11 @@ private void SetHasFocusFalse (View? newFocusedView, bool traversingDown = false
 
     private void NotifyFocusChanged (bool newHasFocus, View? previousFocusedView, View? focusedVew)
     {
+        if (newHasFocus && focusedVew?.Focused is null)
+        {
+            Application.Navigation?.SetFocused (focusedVew);
+        }
+
         // Call the virtual method
         OnHasFocusChanged (newHasFocus, previousFocusedView, focusedVew);