Skip to content

Commit

Permalink
Fixes gui-cs#2558. MenuBar positions wrong in some situations.
Browse files Browse the repository at this point in the history
  • Loading branch information
@BDisp
BDisp committed Apr 17, 2023
1 parent 5cefd5b commit 408f1ca
Show file tree
Hide file tree
Showing 4 changed files with 148 additions and 65 deletions.
101 changes: 54 additions & 47 deletions Terminal.Gui/Application.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public static partial class Application {

// For Unit testing - ignores UseSystemConsole
internal static bool _forceFakeConsole;

private static bool? _enableConsoleScrolling;
/// <summary>
/// The current <see cref="ConsoleDriver.EnableConsoleScrolling"/> used in the terminal.
Expand Down Expand Up @@ -119,7 +119,7 @@ private static List<CultureInfo> GetSupportedCultures ()
File.Exists (Path.Combine (assemblyLocation, cultureInfo.Name, resourceFilename))
).ToList ();
}

#region Initialization (Init/Shutdown)

/// <summary>
Expand Down Expand Up @@ -357,7 +357,7 @@ public RunState (Toplevel view)
/// For debug (see DEBUG_IDISPOSABLE define) purposes; the runstate instances that have been created
/// </summary>
public static List<RunState> Instances = new List<RunState> ();

/// <summary>
/// Creates a new RunState object.
/// </summary>
Expand Down Expand Up @@ -404,7 +404,7 @@ protected virtual void Dispose (bool disposing)
/// Building block API: Prepares the provided <see cref="Toplevel"/> for execution.
/// </summary>
/// <returns>The <see cref="RunState"/> handle that needs to be passed to the <see cref="End(RunState)"/> method upon completion.</returns>
/// <param name="Toplevel">The <see cref="Toplevel"/> to prepare execution for.</param>
/// <param name="toplevel">The <see cref="Toplevel"/> to prepare execution for.</param>
/// <remarks>
/// This method prepares the provided <see cref="Toplevel"/> for running with the focus,
/// it adds this to the list of <see cref="Toplevel"/>s, sets up the <see cref="MainLoop"/> to process the
Expand All @@ -413,93 +413,100 @@ protected virtual void Dispose (bool disposing)
/// the <see cref="RunLoop"/> method, and then the <see cref="End(RunState)"/> method upon termination which will
/// undo these changes.
/// </remarks>
public static RunState Begin (Toplevel Toplevel)
public static RunState Begin (Toplevel toplevel)
{
if (Toplevel == null) {
throw new ArgumentNullException (nameof (Toplevel));
} else if (Toplevel.IsOverlappedContainer && OverlappedTop != Toplevel && OverlappedTop != null) {
if (toplevel == null) {
throw new ArgumentNullException (nameof (toplevel));
} else if (toplevel.IsOverlappedContainer && OverlappedTop != toplevel && OverlappedTop != null) {
throw new InvalidOperationException ("Only one Overlapped Container is allowed.");
}

var rs = new RunState (Toplevel);
var rs = new RunState (toplevel);

// View implements ISupportInitializeNotification which is derived from ISupportInitialize
if (!Toplevel.IsInitialized) {
Toplevel.BeginInit ();
Toplevel.EndInit ();
if (Top != null && !_toplevels.Contains (Top) && toplevel != Top) {
if (toplevel.IsOverlappedContainer) {
Top.Dispose ();
Top = null;
Top = toplevel;
}
Top.BeginInit ();
Top.EndInit ();
}
if (!toplevel.IsInitialized) {
toplevel.BeginInit ();
toplevel.EndInit ();
}

lock (_toplevels) {
// If Top was already initialized with Init, and Begin has never been called
// Top was not added to the Toplevels Stack. It will thus never get disposed.
// Clean it up here:
if (Top != null && Toplevel != Top && !_toplevels.Contains (Top)) {
Top.Dispose ();
Top = null;
} else if (Top != null && Toplevel != Top && _toplevels.Contains (Top)) {
Top.OnLeave (Toplevel);
// Top was not added to the Toplevels Stack and is needed to add it.
if (Top != null && !_toplevels.Contains (Top)) {
Top.Id = "1";
_toplevels.Push (Top);
}
// If toplevel parameter is different of Top then invoke the Leave event.
if (Top != null && toplevel != Top && _toplevels.Contains (Top)) {
Top.OnLeave (toplevel);
}
if (string.IsNullOrEmpty (Toplevel.Id.ToString ())) {
if (string.IsNullOrEmpty (toplevel.Id.ToString ())) {
var count = 1;
var id = (_toplevels.Count + count).ToString ();
while (_toplevels.Count > 0 && _toplevels.FirstOrDefault (x => x.Id.ToString () == id) != null) {
count++;
id = (_toplevels.Count + count).ToString ();
}
Toplevel.Id = (_toplevels.Count + count).ToString ();
toplevel.Id = (_toplevels.Count + count).ToString ();

_toplevels.Push (Toplevel);
_toplevels.Push (toplevel);
} else {
var dup = _toplevels.FirstOrDefault (x => x.Id.ToString () == Toplevel.Id);
var dup = _toplevels.FirstOrDefault (x => x.Id.ToString () == toplevel.Id);
if (dup == null) {
_toplevels.Push (Toplevel);
_toplevels.Push (toplevel);
}
}

if (_toplevels.FindDuplicates (new ToplevelEqualityComparer ()).Count > 0) {
throw new ArgumentException ("There are duplicates Toplevels Id's");
}
}
if (Top == null || Toplevel.IsOverlappedContainer) {
Top = Toplevel;
}

var refreshDriver = true;
if (OverlappedTop == null || Toplevel.IsOverlappedContainer || (Current?.Modal == false && Toplevel.Modal)
|| (Current?.Modal == false && !Toplevel.Modal) || (Current?.Modal == true && Toplevel.Modal)) {
if (OverlappedTop == null || toplevel.IsOverlappedContainer || (Current?.Modal == false && toplevel.Modal)
|| (Current?.Modal == false && !toplevel.Modal) || (Current?.Modal == true && toplevel.Modal)) {

if (Toplevel.Visible) {
Current = Toplevel;
if (toplevel.Visible) {
Current = toplevel;
SetCurrentOverlappedAsTop ();
} else {
refreshDriver = false;
}
} else if ((OverlappedTop != null && Toplevel != OverlappedTop && Current?.Modal == true && !_toplevels.Peek ().Modal)
|| (OverlappedTop != null && Toplevel != OverlappedTop && Current?.Running == false)) {
} else if ((OverlappedTop != null && toplevel != OverlappedTop && Current?.Modal == true && !_toplevels.Peek ().Modal)
|| (OverlappedTop != null && toplevel != OverlappedTop && Current?.Running == false)) {
refreshDriver = false;
MoveCurrent (Toplevel);
MoveCurrent (toplevel);
} else {
refreshDriver = false;
MoveCurrent (Current);
}

Driver.PrepareToRun (MainLoop, ProcessKeyEvent, ProcessKeyDownEvent, ProcessKeyUpEvent, ProcessMouseEvent);
if (Toplevel.LayoutStyle == LayoutStyle.Computed) {
Toplevel.SetRelativeLayout (new Rect (0, 0, Driver.Cols, Driver.Rows));
if (toplevel.LayoutStyle == LayoutStyle.Computed) {
toplevel.SetRelativeLayout (new Rect (0, 0, Driver.Cols, Driver.Rows));
}
Toplevel.LayoutSubviews ();
Toplevel.PositionToplevels ();
Toplevel.FocusFirst ();
toplevel.LayoutSubviews ();
toplevel.PositionToplevels ();
toplevel.FocusFirst ();
if (refreshDriver) {
OverlappedTop?.OnChildLoaded (Toplevel);
Toplevel.OnLoaded ();
Toplevel.SetNeedsDisplay ();
Toplevel.Redraw (Toplevel.Bounds);
Toplevel.PositionCursor ();
OverlappedTop?.OnChildLoaded (toplevel);
toplevel.OnLoaded ();
toplevel.SetNeedsDisplay ();
toplevel.Redraw (toplevel.Bounds);
toplevel.PositionCursor ();
Driver.Refresh ();
}

NotifyNewRunState?.Invoke (Toplevel, new RunStateEventArgs (rs));
NotifyNewRunState?.Invoke (toplevel, new RunStateEventArgs (rs));
return rs;
}

Expand Down Expand Up @@ -618,7 +625,7 @@ public static void Run (Toplevel view, Func<Exception, bool> errorHandler = null
}
#endif
}
}
}

/// <summary>
/// Triggers a refresh of the entire display.
Expand Down Expand Up @@ -892,7 +899,7 @@ static void OnNotifyStopRunState (Toplevel top)
NotifyStopRunState?.Invoke (top, new ToplevelEventArgs (top));
}
}

/// <summary>
/// Building block API: completes the execution of a <see cref="Toplevel"/> that was started with <see cref="Begin(Toplevel)"/> .
/// </summary>
Expand Down
65 changes: 54 additions & 11 deletions UnitTests/Application/ApplicationTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ public TestToplevel ()
}

[Fact]
public void Init_Begin_End_Cleans_Up ()
public void Init_Begin_End_Cleans_Up_New_Toplevel ()
{
Init ();

Expand Down Expand Up @@ -161,6 +161,49 @@ public void Init_Begin_End_Cleans_Up ()
Application.NotifyNewRunState -= NewRunStateFn;
Application.End (runstate);

Assert.NotNull (Application.Current);
Assert.NotNull (Application.Top);
Assert.NotNull (Application.MainLoop);
Assert.NotNull (Application.Driver);

Shutdown ();

Assert.Null (Application.Current);
Assert.Null (Application.Top);
Assert.Null (Application.MainLoop);
Assert.Null (Application.Driver);
}

[Fact]
public void Init_Begin_End_Cleans_Up_Application_Top ()
{
Init ();

// Begin will cause Run() to be called, which will call Begin(). Thus will block the tests
// if we don't stop
Application.Iteration = () => {
Application.RequestStop ();
};

Application.RunState runstate = null;
EventHandler<RunStateEventArgs> NewRunStateFn = (s, e) => {
Assert.NotNull (e.State);
runstate = e.State;
};
Application.NotifyNewRunState += NewRunStateFn;

Toplevel topLevel = Application.Top;
var rs = Application.Begin (topLevel);
Assert.NotNull (rs);
Assert.NotNull (runstate);
Assert.Equal (rs, runstate);

Assert.Equal (topLevel, Application.Top);
Assert.Equal (topLevel, Application.Current);

Application.NotifyNewRunState -= NewRunStateFn;
Application.End (runstate);

Assert.Null (Application.Current);
Assert.NotNull (Application.Top);
Assert.NotNull (Application.MainLoop);
Expand Down Expand Up @@ -420,9 +463,9 @@ public void Run_Loaded_Ready_Unlodaded_Events ()
Init ();
var top = Application.Top;
var count = 0;
top.Loaded += (s,e) => count++;
top.Ready += (s,e) => count++;
top.Unloaded += (s,e) => count++;
top.Loaded += (s, e) => count++;
top.Ready += (s, e) => count++;
top.Unloaded += (s, e) => count++;
Application.Iteration = () => Application.RequestStop ();
Application.Run ();
Application.Shutdown ();
Expand Down Expand Up @@ -461,7 +504,7 @@ public void Shutdown_Resets_SyncContext ()
#endregion

[Fact, AutoInitShutdown]
public void Begin_Sets_Application_Top_To_Console_Size()
public void Begin_Sets_Application_Top_To_Console_Size ()
{
Assert.Equal (new Rect (0, 0, 80, 25), Application.Top.Frame);

Expand All @@ -476,7 +519,7 @@ public void Begin_Sets_Application_Top_To_Console_Size()
[AutoInitShutdown]
public void SetCurrentAsTop_Run_A_Not_Modal_Toplevel_Make_It_The_Current_Application_Top ()
{
var t1 = new Toplevel ();
var t1 = Application.Top;
var t2 = new Toplevel ();
var t3 = new Toplevel ();
var d = new Dialog ();
Expand All @@ -485,15 +528,15 @@ public void SetCurrentAsTop_Run_A_Not_Modal_Toplevel_Make_It_The_Current_Applica
// t1, t2, t3, d, t4
var iterations = 5;

t1.Ready += (s,e) => {
t1.Ready += (s, e) => {
Assert.Equal (t1, Application.Top);
Application.Run (t2);
};
t2.Ready += (s,e) => {
t2.Ready += (s, e) => {
Assert.Equal (t2, Application.Top);
Application.Run (t3);
};
t3.Ready += (s,e) => {
t3.Ready += (s, e) => {
Assert.Equal (t3, Application.Top);
Application.Run (d);
};
Expand All @@ -509,7 +552,7 @@ public void SetCurrentAsTop_Run_A_Not_Modal_Toplevel_Make_It_The_Current_Applica
t2.RequestStop ();
};
// Now this will close the OverlappedContainer when all OverlappedChildren was closed
t2.Closed += (s,_) => {
t2.Closed += (s, _) => {
t1.RequestStop ();
};
Application.Iteration += () => {
Expand Down Expand Up @@ -741,7 +784,7 @@ public void QuitKey_Getter_Setter ()
var top = Application.Top;
var isQuiting = false;

top.Closing += (s,e) => {
top.Closing += (s, e) => {
isQuiting = true;
e.Cancel = true;
};
Expand Down
36 changes: 34 additions & 2 deletions UnitTests/Application/RunStateTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,12 @@ void Shutdown ()
Application.Shutdown ();
#if DEBUG_IDISPOSABLE
// Validate there are no outstanding RunState-based instances left
foreach (var inst in Application.RunState.Instances) Assert.True (inst.WasDisposed);
foreach (var inst in Application.RunState.Instances) Assert.True (inst.WasDisposed);
#endif
}

[Fact]
public void Begin_End_Cleans_Up_RunState ()
public void Begin_End_Cleans_Up_RunState_With_New_Toplevel ()
{
// Setup Mock driver
Init ();
Expand All @@ -92,6 +92,38 @@ public void Begin_End_Cleans_Up_RunState ()
Assert.Equal (top, Application.Current);
Application.End (rs);

Assert.NotNull (Application.Current);
Assert.NotNull (Application.Top);
Assert.NotNull (Application.MainLoop);
Assert.NotNull (Application.Driver);

Shutdown ();

#if DEBUG_IDISPOSABLE
Assert.True (rs.WasDisposed);
#endif

Assert.Null (Application.Current);
Assert.Null (Application.Top);
Assert.Null (Application.MainLoop);
Assert.Null (Application.Driver);
}

[Fact]
public void Begin_End_Cleans_Up_RunState_With_Application_Top ()
{
// Setup Mock driver
Init ();

// Test null Toplevel
Assert.Throws<ArgumentNullException> (() => Application.Begin (null));

var top = Application.Top;
var rs = Application.Begin (top);
Assert.NotNull (rs);
Assert.Equal (top, Application.Current);
Application.End (rs);

Assert.Null (Application.Current);
Assert.NotNull (Application.Top);
Assert.NotNull (Application.MainLoop);
Expand Down
Loading

0 comments on commit 408f1ca

Please sign in to comment.