Subview clean up

Author: tig

Terminal.Gui/View/Adornment/Adornment.cs

@@ -82,21 +82,6 @@ public Thickness Thickness
     #endregion Thickness
 
     #region View Overrides
-
-    /// <summary>
-    ///     Adornments cannot be used as sub-views (see <see cref="Parent"/>); setting this property will throw
-    ///     <see cref="InvalidOperationException"/>.
-    /// </summary>
-    /// <remarks>
-    ///     While there are no real use cases for an Adornment being a subview, it is not explicitly dis-allowed to support
-    ///     testing. E.g. in AllViewsTester.
-    /// </remarks>
-    public override View? SuperView
-    {
-        get => base.SuperView!;
-        set => throw new InvalidOperationException (@"Adornments can not be Subviews or have SuperViews. Use Parent instead.");
-    }
-
     /// <summary>
     ///     Gets the rectangle that describes the area of the Adornment. The Location is always (0,0).
     ///     The size is the size of the <see cref="View.Frame"/>.

Terminal.Gui/View/Layout/DimAuto.cs

@@ -72,7 +72,7 @@ internal override int Calculate (int location, int superviewContentSize, View us
         {
             maxCalculatedSize = textSize;
 
-            if (us is { ContentSizeTracksViewport: false, Subviews.Count: 0 })
+            if (us is { ContentSizeTracksViewport: false, InternalSubviews.Count: 0 })
             {
                 // ContentSize was explicitly set. Use `us.ContentSize` to determine size.
                 maxCalculatedSize = dimension == Dimension.Width ? us.GetContentSize ().Width : us.GetContentSize ().Height;
@@ -81,7 +81,7 @@ internal override int Calculate (int location, int superviewContentSize, View us
             {
                 // TOOD: All the below is a naive implementation. It may be possible to optimize this.
 
-                List<View> includedSubviews = us.Subviews.ToList ();
+                List<View> includedSubviews = us.InternalSubviews.ToList ();
 
                 // If [x] it can cause `us.ContentSize` to change.
                 // If [ ] it doesn't need special processing for us to determine `us.ContentSize`.
@@ -197,11 +197,11 @@ or DimAbsolute
 
                 if (dimension == Dimension.Width)
                 {
-                    centeredSubViews = us.Subviews.Where (v => v.X.Has<PosCenter> (out _)).ToList ();
+                    centeredSubViews = us.InternalSubviews.Where (v => v.X.Has<PosCenter> (out _)).ToList ();
                 }
                 else
                 {
-                    centeredSubViews = us.Subviews.Where (v => v.Y.Has<PosCenter> (out _)).ToList ();
+                    centeredSubViews = us.InternalSubviews.Where (v => v.Y.Has<PosCenter> (out _)).ToList ();
                 }
 
                 viewsNeedingLayout.AddRange (centeredSubViews);

Terminal.Gui/View/View.Command.cs

@@ -91,7 +91,7 @@ private void SetupCommands ()
         if (!args.Cancel)
         {
             // If there's an IsDefault peer view in Subviews, try it
-            var isDefaultView = SuperView?.Subviews.FirstOrDefault (v => v is Button { IsDefault: true });
+            var isDefaultView = SuperView?.InternalSubviews.FirstOrDefault (v => v is Button { IsDefault: true });
 
             if (isDefaultView != this && isDefaultView is Button { IsDefault: true } button)
             {

Terminal.Gui/View/View.Hierarchy.cs

@@ -15,26 +15,27 @@ public partial class View // SuperView/SubView hierarchy management (SuperView,
     // to make the same mistakes our users make when they poke at the Subviews.
     internal IList<View> InternalSubviews => _subviews ?? _empty;
 
-    /// <summary>This returns a list of the subviews contained by this view.</summary>
-    /// <value>The subviews.</value>
+    /// <summary>Gets the list of Subviews.</summary>
+    /// <remarks>
+    ///     Use <see cref="Add(Terminal.Gui.View?)"/> and <see cref="Remove(Terminal.Gui.View?)"/> to add or remove subviews.
+    /// </remarks>
     public IList<View> Subviews => _subviews?.AsReadOnly () ?? _empty;
 
     private View? _superView;
 
-    /// <summary>Returns the container for this view, or null if this view has not been added to a container.</summary>
-    /// <value>The super view.</value>
-    public virtual View? SuperView
+    /// <summary>Gets this Views SuperView (the View's container), or <see langword="null"/> if this view has not been added as a Subview.</summary>
+    public View? SuperView
     {
         get => _superView!;
-        set => throw new NotImplementedException ();
+        set => throw new InvalidOperationException (@"SuperView cannot be set.");
     }
 
     #region AddRemove
 
     /// <summary>Indicates whether the view was added to <see cref="SuperView"/>.</summary>
     public bool IsAdded { get; private set; }
 
-    /// <summary>Adds a subview (child) to this view.</summary>
+    /// <summary>Adds a Subview (child) to this view.</summary>
     /// <remarks>
     ///     <para>
     ///         The Views that have been added to this view can be retrieved via the <see cref="Subviews"/> property. See also
@@ -94,7 +95,7 @@ public virtual View? SuperView
         return view;
     }
 
-    /// <summary>Adds the specified views (children) to the view.</summary>
+    /// <summary>Adds the specified Subview (children) to the view.</summary>
     /// <param name="views">Array of one or more views (can be optional parameter).</param>
     /// <remarks>
     ///     <para>
@@ -119,28 +120,29 @@ public void Add (params View []? views)
         }
     }
 
-    /// <summary>Event fired when this view is added to another.</summary>
+    // TODO: Make these events follow the standard pattern
+    /// <summary>Raised when a Subview has been added to this View.</summary>
     public event EventHandler<SuperViewChangedEventArgs>? Added;
 
-    /// <summary>Method invoked when a subview is being added to this view.</summary>
+    /// <summary>Method invoked when a Subview has been added to this view.</summary>
     /// <param name="e">Event where <see cref="ViewEventArgs.View"/> is the subview being added.</param>
-    public virtual void OnAdded (SuperViewChangedEventArgs e)
+    protected virtual void OnAdded (SuperViewChangedEventArgs e)
     {
         View view = e.SubView;
         view.IsAdded = true;
         view.Added?.Invoke (this, e);
     }
 
-    /// <summary>Method invoked when a subview is being removed from this view.</summary>
+    /// <summary>Method invoked when a Subview is being removed from this view.</summary>
     /// <param name="e">Event args describing the subview being removed.</param>
-    public virtual void OnRemoved (SuperViewChangedEventArgs e)
+    protected virtual void OnRemoved (SuperViewChangedEventArgs e)
     {
         View view = e.SubView;
         view.IsAdded = false;
         view.Removed?.Invoke (this, e);
     }
 
-    /// <summary>Removes a subview added via <see cref="Add(View)"/> or <see cref="Add(View[])"/> from this View.</summary>
+    /// <summary>Removes a Subview added via <see cref="Add(View)"/> or <see cref="Add(View[])"/> from this View.</summary>
     /// <remarks>
     ///     <para>
     ///         Normally Subviews will be disposed when this View is disposed. Removing a Subview causes ownership of the
@@ -208,7 +210,7 @@ public virtual void OnRemoved (SuperViewChangedEventArgs e)
     }
 
     /// <summary>
-    ///     Removes all subviews (children) added via <see cref="Add(View)"/> or <see cref="Add(View[])"/> from this View.
+    ///     Removes all Subview (children) added via <see cref="Add(View)"/> or <see cref="Add(View[])"/> from this View.
     /// </summary>
     /// <remarks>
     ///     <para>
@@ -231,15 +233,15 @@ public virtual void RemoveAll ()
         }
     }
 
-    /// <summary>Event fired when this view is removed from another.</summary>
+    /// <summary>Raised when a Subview has been removed from this View.</summary>
     public event EventHandler<SuperViewChangedEventArgs>? Removed;
 
     #endregion AddRemove
 
-    // TODO: Mark as internal. Or nuke.
+    // TODO: This drives a weird coupling of Application.Top and View. It's not clear why this is needed.
     /// <summary>Get the top superview of a given <see cref="View"/>.</summary>
     /// <returns>The superview view.</returns>
-    public View? GetTopSuperView (View? view = null, View? superview = null)
+    internal View? GetTopSuperView (View? view = null, View? superview = null)
     {
         View? top = superview ?? Application.Top;
 
@@ -275,7 +277,7 @@ public static bool IsInHierarchy (View? start, View? view, bool includeAdornment
             return true;
         }
 
-        foreach (View subView in start.Subviews)
+        foreach (View subView in start.InternalSubviews)
         {
             if (view == subView)
             {

Terminal.Gui/View/View.Keyboard.cs

@@ -562,12 +562,12 @@ private static bool InvokeCommandsBoundToKeyOnAdornment (Adornment adornment, Ke
             return true;
         }
 
-        if (adornment?.Subviews is null)
+        if (adornment?.InternalSubviews is null)
         {
             return false;
         }
 
-        foreach (View subview in adornment.Subviews)
+        foreach (View subview in adornment.InternalSubviews)
         {
             bool? subViewHandled = subview.InvokeCommands (key);
 
@@ -604,7 +604,7 @@ internal bool InvokeCommandsBoundToHotKey (Key hotKey, ref bool? handled)
         }
 
         // Now, process any HotKey bindings in the subviews
-        foreach (View subview in Subviews)
+        foreach (View subview in InternalSubviews)
         {
             if (subview == Focused)
             {