Fixes #2485 ++ - Wizard v2 architecture modernization with Padding-based layout (#4510)

* Initial plan

* Fix Wizard v2 architecture issues - ScrollBar API, event handlers, key bindings

Co-authored-by: tig <585482+tig@users.noreply.github.com>

* Implement issue #4155 - Put nav buttons in bottom Padding, Help in right Padding

Co-authored-by: tig <585482+tig@users.noreply.github.com>

* Address code review feedback - Extract helper method, improve null checks

Co-authored-by: tig <585482+tig@users.noreply.github.com>

* Fix disposal issue - Ensure _helpTextView is always disposed

Co-authored-by: tig <585482+tig@users.noreply.github.com>

* Refactor & improvements. WIP

* Tweaking layout

* Wizard tweaks

* Added View.GetSubViews that optinoally gets subviews of adornments

* Refactor Wizard API: modern events, layout, and design

- Replaced custom event args with standard .NET event args (CancelEventArgs, ValueChangingEventArgs, etc.)
- Removed Finished event; use Accepting for wizard completion
- Updated Cancelled, MovingBack, MovingNext to use CancelEventArgs
- Refactored UICatalog scenarios and tests to new event model
- Improved WizardStep sizing and wizard auto-resizing to content
- Enhanced IDesignable for Wizard and WizardStep with richer design-time UI
- Simplified help text padding logic in WizardStep
- Removed obsolete code and modernized code style throughout
- Improves API consistency, usability, and .NET idiomatic usage

* Fixes #4515 - Navigating into and out of Adornments does not work

* WIP. QUite broken.

* All fixed?

* Tweaks.

* Exclude Margin subviews from drawing; add shadow tests

Update Margin adornment to skip drawing subviews that are themselves Margin views, preventing unsupported nested Margin rendering. Add unit tests to verify that opaque-shadowed buttons in Margin are not drawn, while Border and Padding still support shadow rendering. Update test class to use output helper and assert driver output.

* Final code cleanup and test improvements.

* Update Margin.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update View.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update View.Hierarchy.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update View.Hierarchy.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Refactor: code style, formatting, and minor logic cleanup

- Standardized spacing and formatting for method signatures and object initializations.
- Converted simple methods and properties to expression-bodied members for conciseness.
- Replaced named arguments with positional arguments for consistency.
- Improved XML documentation formatting for readability.
- Simplified logic in event handlers (e.g., Wizard Back button).
- Removed redundant checks where properties are guaranteed to exist.
- Fixed minor bugs related to padding, height calculation, and event handling.
- Adopted consistent use of `var` for local variables.
- Corrected namespace declarations.
- Refactored methods returning constants to use expression-bodied syntax.
- General code cleanup for clarity and maintainability; no breaking changes.

* api docs

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: tig <585482+tig@users.noreply.github.com>
Co-authored-by: Tig <tig@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
Copilot
2025-12-21 07:42:04 -07:00
committed by GitHub
parent af0efb3c64
commit 4145b984ba
29 changed files with 3507 additions and 1380 deletions

View File

@@ -6,16 +6,14 @@ public class SubViewTests
[Fact]
public void SuperViewChanged_Raised_On_Add ()
{
var super = new View { };
var super = new View ();
var sub = new View ();
int superRaisedCount = 0;
int subRaisedCount = 0;
var superRaisedCount = 0;
var subRaisedCount = 0;
super.SuperViewChanged += (s, e) => { superRaisedCount++; };
super.SuperViewChanged += (s, e) =>
{
superRaisedCount++;
};
sub.SuperViewChanged += (s, e) =>
{
if (sub.SuperView is { })
@@ -34,23 +32,21 @@ public class SubViewTests
[Fact]
public void SuperViewChanged_Raised_On_Remove ()
{
var super = new View { };
var super = new View ();
var sub = new View ();
int superRaisedCount = 0;
int subRaisedCount = 0;
var superRaisedCount = 0;
var subRaisedCount = 0;
super.SuperViewChanged += (s, e) => { superRaisedCount++; };
super.SuperViewChanged += (s, e) =>
{
superRaisedCount++;
};
sub.SuperViewChanged += (s, e) =>
{
if (sub.SuperView is null)
{
subRaisedCount++;
}
};
{
if (sub.SuperView is null)
{
subRaisedCount++;
}
};
super.Add (sub);
Assert.True (super.SubViews.Count == 1);
@@ -95,6 +91,13 @@ public class SubViewTests
Assert.Equal (new (1, 1), view.GetContentSize ());
}
[Fact]
public void Add_Margin_Throws ()
{
View view = new ();
Assert.Throws<InvalidOperationException> (() => view.Margin!.Add (new View ()));
}
[Fact]
public void Remove_Does_Not_Impact_ContentSize ()
{
@@ -392,18 +395,12 @@ public class SubViewTests
var view = new View ();
var superView = new View ();
int superViewChangedCount = 0;
int superViewChangingCount = 0;
var superViewChangedCount = 0;
var superViewChangingCount = 0;
view.SuperViewChanged += (s, e) =>
{
superViewChangedCount++;
};
view.SuperViewChanged += (s, e) => { superViewChangedCount++; };
view.SuperViewChanging += (s, e) =>
{
superViewChangingCount++;
};
view.SuperViewChanging += (s, e) => { superViewChangingCount++; };
// Act
superView.Add (view);
@@ -411,7 +408,6 @@ public class SubViewTests
// Assert
Assert.Equal (1, superViewChangingCount);
Assert.Equal (1, superViewChangedCount);
}
[Fact]
@@ -450,7 +446,6 @@ public class SubViewTests
top2.Dispose ();
}
[Fact]
public void Initialized_Event_Comparing_With_Added_Event ()
{
@@ -479,10 +474,10 @@ public class SubViewTests
int tc = 0, wc = 0, v1c = 0, v2c = 0, sv1c = 0;
winAddedToTop.SubViewAdded += (s, e) =>
{
Assert.Equal (e.SuperView!.Frame.Width, winAddedToTop.Frame.Width);
Assert.Equal (e.SuperView.Frame.Height, winAddedToTop.Frame.Height);
};
{
Assert.Equal (e.SuperView!.Frame.Width, winAddedToTop.Frame.Width);
Assert.Equal (e.SuperView.Frame.Height, winAddedToTop.Frame.Height);
};
v1AddedToWin.SubViewAdded += (s, e) =>
{
@@ -503,69 +498,70 @@ public class SubViewTests
};
top.Initialized += (s, e) =>
{
tc++;
Assert.Equal (1, tc);
Assert.Equal (1, wc);
Assert.Equal (1, v1c);
Assert.Equal (1, v2c);
Assert.Equal (1, sv1c);
{
tc++;
Assert.Equal (1, tc);
Assert.Equal (1, wc);
Assert.Equal (1, v1c);
Assert.Equal (1, v2c);
Assert.Equal (1, sv1c);
Assert.True (top.CanFocus);
Assert.True (winAddedToTop.CanFocus);
Assert.False (v1AddedToWin.CanFocus);
Assert.False (v2AddedToWin.CanFocus);
Assert.False (svAddedTov1.CanFocus);
Assert.True (top.CanFocus);
Assert.True (winAddedToTop.CanFocus);
Assert.False (v1AddedToWin.CanFocus);
Assert.False (v2AddedToWin.CanFocus);
Assert.False (svAddedTov1.CanFocus);
top.Layout ();
};
top.Layout ();
};
winAddedToTop.Initialized += (s, e) =>
{
wc++;
Assert.Equal (top.Viewport.Width, winAddedToTop.Frame.Width);
Assert.Equal (top.Viewport.Height, winAddedToTop.Frame.Height);
};
{
wc++;
Assert.Equal (top.Viewport.Width, winAddedToTop.Frame.Width);
Assert.Equal (top.Viewport.Height, winAddedToTop.Frame.Height);
};
v1AddedToWin.Initialized += (s, e) =>
{
v1c++;
{
v1c++;
// Top.Frame: 0, 0, 80, 25; Top.Viewport: 0, 0, 80, 25
// BUGBUG: This is wrong, it should be 78, 23. This test has always been broken.
// in no way should the v1AddedToWin.Frame be the same as the Top.Frame/Viewport
// as it is a subview of winAddedToTop, which has a border!
//Assert.Equal (top.Viewport.Width, v1AddedToWin.Frame.Width);
//Assert.Equal (top.Viewport.Height, v1AddedToWin.Frame.Height);
};
// Top.Frame: 0, 0, 80, 25; Top.Viewport: 0, 0, 80, 25
// BUGBUG: This is wrong, it should be 78, 23. This test has always been broken.
// in no way should the v1AddedToWin.Frame be the same as the Top.Frame/Viewport
// as it is a subview of winAddedToTop, which has a border!
//Assert.Equal (top.Viewport.Width, v1AddedToWin.Frame.Width);
//Assert.Equal (top.Viewport.Height, v1AddedToWin.Frame.Height);
};
v2AddedToWin.Initialized += (s, e) =>
{
v2c++;
{
v2c++;
// Top.Frame: 0, 0, 80, 25; Top.Viewport: 0, 0, 80, 25
// BUGBUG: This is wrong, it should be 78, 23. This test has always been broken.
// in no way should the v2AddedToWin.Frame be the same as the Top.Frame/Viewport
// as it is a subview of winAddedToTop, which has a border!
//Assert.Equal (top.Viewport.Width, v2AddedToWin.Frame.Width);
//Assert.Equal (top.Viewport.Height, v2AddedToWin.Frame.Height);
};
// Top.Frame: 0, 0, 80, 25; Top.Viewport: 0, 0, 80, 25
// BUGBUG: This is wrong, it should be 78, 23. This test has always been broken.
// in no way should the v2AddedToWin.Frame be the same as the Top.Frame/Viewport
// as it is a subview of winAddedToTop, which has a border!
//Assert.Equal (top.Viewport.Width, v2AddedToWin.Frame.Width);
//Assert.Equal (top.Viewport.Height, v2AddedToWin.Frame.Height);
};
svAddedTov1.Initialized += (s, e) =>
{
sv1c++;
{
sv1c++;
// Top.Frame: 0, 0, 80, 25; Top.Viewport: 0, 0, 80, 25
// BUGBUG: This is wrong, it should be 78, 23. This test has always been broken.
// in no way should the svAddedTov1.Frame be the same as the Top.Frame/Viewport
// because sv1AddedTov1 is a subview of v1AddedToWin, which is a subview of
// winAddedToTop, which has a border!
//Assert.Equal (top.Viewport.Width, svAddedTov1.Frame.Width);
//Assert.Equal (top.Viewport.Height, svAddedTov1.Frame.Height);
Assert.False (svAddedTov1.CanFocus);
//Assert.Throws<InvalidOperationException> (() => svAddedTov1.CanFocus = true);
Assert.False (svAddedTov1.CanFocus);
};
// Top.Frame: 0, 0, 80, 25; Top.Viewport: 0, 0, 80, 25
// BUGBUG: This is wrong, it should be 78, 23. This test has always been broken.
// in no way should the svAddedTov1.Frame be the same as the Top.Frame/Viewport
// because sv1AddedTov1 is a subview of v1AddedToWin, which is a subview of
// winAddedToTop, which has a border!
//Assert.Equal (top.Viewport.Width, svAddedTov1.Frame.Width);
//Assert.Equal (top.Viewport.Height, svAddedTov1.Frame.Height);
Assert.False (svAddedTov1.CanFocus);
//Assert.Throws<InvalidOperationException> (() => svAddedTov1.CanFocus = true);
Assert.False (svAddedTov1.CanFocus);
};
v1AddedToWin.Add (svAddedTov1);
winAddedToTop.Add (v1AddedToWin, v2AddedToWin);
@@ -639,7 +635,7 @@ public class SubViewTests
superView.Add (subView1, subView2, subView3);
// Act
var removedViews = superView.RemoveAll ();
IReadOnlyCollection<View> removedViews = superView.RemoveAll ();
// Assert
Assert.Empty (superView.SubViews);
@@ -662,7 +658,7 @@ public class SubViewTests
superView.Add (subView1, subView2, subView3, subView4);
// Act
var removedViews = superView.RemoveAll<Button> ();
IReadOnlyCollection<Button> removedViews = superView.RemoveAll<Button> ();
// Assert
Assert.Equal (3, superView.SubViews.Count);
@@ -683,7 +679,7 @@ public class SubViewTests
superView.Add (subView1, subView2, subView3);
// Act
var removedViews = superView.RemoveAll<Button> ();
IReadOnlyCollection<Button> removedViews = superView.RemoveAll<Button> ();
// Assert
Assert.Equal (2, superView.SubViews.Count);
@@ -700,7 +696,7 @@ public class SubViewTests
var superView = new View ();
var subView = new View ();
var events = new List<string> ();
List<string> events = new ();
subView.SuperViewChanging += (s, e) => { events.Add ("SuperViewChanging"); };
@@ -722,7 +718,7 @@ public class SubViewTests
var superView = new View ();
var subView = new View ();
View? currentValueInEvent = new View (); // Set to non-null to ensure it gets updated
var currentValueInEvent = new View (); // Set to non-null to ensure it gets updated
View? newValueInEvent = null;
subView.SuperViewChanging += (s, e) =>
@@ -749,7 +745,7 @@ public class SubViewTests
superView.Add (subView);
View? currentValueInEvent = null;
View? newValueInEvent = new View (); // Set to non-null to ensure it gets updated
var newValueInEvent = new View (); // Set to non-null to ensure it gets updated
subView.SuperViewChanging += (s, e) =>
{
@@ -770,7 +766,7 @@ public class SubViewTests
{
// Arrange
using IApplication app = Application.Create ();
var runnable = new Runnable<bool> ();
Runnable<bool> runnable = new ();
var subView = new View ();
runnable.Add (subView);
@@ -781,11 +777,11 @@ public class SubViewTests
subView.SuperViewChanging += (s, e) =>
{
Assert.NotNull (s);
// At this point, SuperView is still set, so App should be accessible
appInEvent = (s as View)?.App;
};
Assert.NotNull (runnable.App);
// Act
@@ -804,7 +800,7 @@ public class SubViewTests
{
// Arrange
var superView = new View ();
var events = new List<string> ();
List<string> events = new ();
var subView = new TestViewWithSuperViewEvents (events);
@@ -854,6 +850,7 @@ public class SubViewTests
protected override bool OnSuperViewChanging (ValueChangingEventArgs<View?> args)
{
_events.Add ("OnSuperViewChanging");
return base.OnSuperViewChanging (args);
}
@@ -907,10 +904,7 @@ public class SubViewTests
var subView = new TestViewThatCancelsChange ();
var eventRaised = false;
subView.SuperViewChanging += (s, e) =>
{
eventRaised = true;
};
subView.SuperViewChanging += (s, e) => { eventRaised = true; };
// Act
superView.Add (subView);
@@ -983,4 +977,327 @@ public class SubViewTests
return true; // Always cancel the change
}
}
#region GetSubViews Tests
[Fact]
public void GetSubViews_Returns_Empty_Collection_When_No_SubViews ()
{
// Arrange
View view = new ();
// Act
IReadOnlyCollection<View> result = view.GetSubViews ();
// Assert
Assert.NotNull (result);
Assert.Empty (result);
}
[Fact]
public void GetSubViews_Returns_Direct_SubViews_By_Default ()
{
// Arrange
View superView = new ();
View subView1 = new () { Id = "subView1" };
View subView2 = new () { Id = "subView2" };
View subView3 = new () { Id = "subView3" };
superView.Add (subView1, subView2, subView3);
// Act
IReadOnlyCollection<View> result = superView.GetSubViews ();
// Assert
Assert.NotNull (result);
Assert.Equal (3, result.Count);
Assert.Contains (subView1, result);
Assert.Contains (subView2, result);
Assert.Contains (subView3, result);
}
[Fact]
public void GetSubViews_Does_Not_Include_Adornment_SubViews_By_Default ()
{
// Arrange
View superView = new ();
View subView = new () { Id = "subView" };
superView.Add (subView);
superView.BeginInit ();
superView.EndInit ();
// Add a subview to the Border (e.g., ShadowView)
View borderSubView = new () { Id = "borderSubView" };
superView.Border!.Add (borderSubView);
// Act
IReadOnlyCollection<View> result = superView.GetSubViews ();
// Assert
Assert.Single (result);
Assert.Contains (subView, result);
Assert.DoesNotContain (borderSubView, result);
}
[Fact]
public void GetSubViews_Includes_Border_SubViews_When_IncludeAdornments_Is_True ()
{
// Arrange
View superView = new ();
View subView = new () { Id = "subView" };
superView.Add (subView);
superView.BeginInit ();
superView.EndInit ();
// Add a subview to the Border
View borderSubView = new () { Id = "borderSubView" };
// Thickness matters
superView.Border!.Thickness = new (1);
superView.Border!.Add (borderSubView);
// Act
IReadOnlyCollection<View> result = superView.GetSubViews (includeBorder: true);
// Assert
Assert.Equal (2, result.Count);
Assert.Contains (subView, result);
Assert.Contains (borderSubView, result);
}
[Fact]
public void GetSubViews_Includes_Padding_SubViews_When_IncludeAdornments_Is_True ()
{
// Arrange
View superView = new ();
View subView = new () { Id = "subView" };
superView.Add (subView);
superView.BeginInit ();
superView.EndInit ();
// Add a subview to the Padding
View paddingSubView = new () { Id = "paddingSubView" };
// Thickness matters
superView.Padding!.Thickness = new (1);
superView.Padding!.Add (paddingSubView);
// Act
IReadOnlyCollection<View> result = superView.GetSubViews (includePadding: true);
// Assert
Assert.Equal (2, result.Count);
Assert.Contains (subView, result);
Assert.Contains (paddingSubView, result);
}
[Fact]
public void GetSubViews_Includes_All_Adornment_SubViews_When_IncludeAdornments_Is_True ()
{
// Arrange
View superView = new ();
View subView1 = new () { Id = "subView1" };
View subView2 = new () { Id = "subView2" };
superView.Add (subView1, subView2);
superView.BeginInit ();
superView.EndInit ();
// Add subviews to each adornment
View borderSubView = new () { Id = "borderSubView" };
View paddingSubView = new () { Id = "paddingSubView" };
// Thickness matters
//superView.Margin!.Thickness = new (1);
//superView.Margin!.Add (marginSubView);
superView.Border!.Thickness = new (1);
superView.Border!.Add (borderSubView);
superView.Padding!.Thickness = new (1);
superView.Padding!.Add (paddingSubView);
// Act
IReadOnlyCollection<View> result = superView.GetSubViews (true, true, true);
// Assert
Assert.Equal (4, result.Count);
Assert.Contains (subView1, result);
Assert.Contains (subView2, result);
Assert.Contains (borderSubView, result);
Assert.Contains (paddingSubView, result);
}
[Fact]
public void GetSubViews_Returns_Correct_Order ()
{
// Arrange
View superView = new ();
View subView1 = new () { Id = "subView1" };
View subView2 = new () { Id = "subView2" };
superView.Add (subView1, subView2);
superView.BeginInit ();
superView.EndInit ();
View borderSubView = new () { Id = "borderSubView" };
View paddingSubView = new () { Id = "paddingSubView" };
// Thickness matters
superView.Border!.Thickness = new (1);
superView.Border!.Add (borderSubView);
superView.Padding!.Thickness = new (1);
superView.Padding!.Add (paddingSubView);
// Act
IReadOnlyCollection<View> result = superView.GetSubViews (true, true, true);
List<View> resultList = result.ToList ();
// Assert - Order should be: direct SubViews, Border, Padding
Assert.Equal (4, resultList.Count);
Assert.Equal (subView1, resultList [0]);
Assert.Equal (subView2, resultList [1]);
Assert.Equal (borderSubView, resultList [2]);
Assert.Equal (paddingSubView, resultList [3]);
}
[Fact]
public void GetSubViews_Returns_Snapshot_Safe_For_Modification ()
{
// Arrange
View superView = new ();
View subView1 = new () { Id = "subView1" };
View subView2 = new () { Id = "subView2" };
superView.Add (subView1, subView2);
// Act
IReadOnlyCollection<View> result = superView.GetSubViews ();
// Modify the SuperView's SubViews
View subView3 = new () { Id = "subView3" };
superView.Add (subView3);
// Assert - The snapshot should not include subView3
Assert.Equal (2, result.Count);
Assert.Contains (subView1, result);
Assert.Contains (subView2, result);
Assert.DoesNotContain (subView3, result);
}
[Fact]
public void GetSubViews_Multiple_SubViews_In_Each_Adornment ()
{
// Arrange
View superView = new ();
View subView = new () { Id = "subView" };
superView.Add (subView);
superView.BeginInit ();
superView.EndInit ();
// Add multiple subviews to each adornment
View borderSubView1 = new () { Id = "borderSubView1" };
View borderSubView2 = new () { Id = "borderSubView2" };
View paddingSubView1 = new () { Id = "paddingSubView1" };
View paddingSubView2 = new () { Id = "paddingSubView2" };
// Thickness matters
superView.Border!.Thickness = new (1);
superView.Border!.Add (borderSubView1, borderSubView2);
// Thickness matters
superView.Padding!.Thickness = new (1);
superView.Padding!.Add (paddingSubView1, paddingSubView2);
// Act
IReadOnlyCollection<View> result = superView.GetSubViews (true, true, true);
// Assert
Assert.Equal (5, result.Count);
Assert.Contains (subView, result);
Assert.Contains (borderSubView1, result);
Assert.Contains (borderSubView2, result);
Assert.Contains (paddingSubView1, result);
Assert.Contains (paddingSubView2, result);
}
[Fact]
public void GetSubViews_Works_With_Adornment_Itself ()
{
// Arrange - Test that an Adornment (which is a View) can also have GetSubViews called
View view = new ();
view.BeginInit ();
view.EndInit ();
View paddingSubView = new () { Id = "paddingSubView" };
view.Padding!.Add (paddingSubView);
// Act - Call GetSubViews on the Margin itself
IReadOnlyCollection<View> result = view.Padding.GetSubViews ();
// Assert
Assert.Single (result);
Assert.Contains (paddingSubView, result);
}
[Fact]
public void GetSubViews_Handles_Null_Adornments_Gracefully ()
{
// Arrange - Create an Adornment view which doesn't have its own adornments
View view = new ();
view.BeginInit ();
view.EndInit ();
// Border is an Adornment and doesn't have Margin, Border, Padding
View borderSubView = new () { Id = "borderSubView" };
view.Border!.Add (borderSubView);
// Act - GetSubViews on Border (an Adornment) with includeAdornments
IReadOnlyCollection<View> result = view.Border.GetSubViews (true);
// Assert - Should only return direct subviews, not crash
Assert.Single (result);
Assert.Contains (borderSubView, result);
}
[Fact]
public void GetSubViews_Returns_IReadOnlyCollection ()
{
// Arrange
View superView = new ();
View subView = new () { Id = "subView" };
superView.Add (subView);
// Act
IReadOnlyCollection<View> result = superView.GetSubViews ();
// Assert
Assert.IsAssignableFrom<IReadOnlyCollection<View>> (result);
// Verify Count property is available and single item
Assert.Single (result);
}
[Fact]
public void GetSubViews_Empty_Adornments_Do_Not_Add_Nulls ()
{
// Arrange
View superView = new ();
View subView = new () { Id = "subView" };
superView.Add (subView);
superView.BeginInit ();
superView.EndInit ();
// Don't add any subviews to adornments
// Act
IReadOnlyCollection<View> result = superView.GetSubViews (true);
// Assert - Should only have the direct subview, no nulls
Assert.Single (result);
Assert.Contains (subView, result);
Assert.All (result, Assert.NotNull);
}
}
#endregion GetSubViews Tests