Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v2 Fixes #2810. Pressing Alt key on a Toplevel with only a MenuBar throws System.InvalidOperationException. #2812

3 changes: 2 additions & 1 deletion Terminal.Gui/Application.cs
Original file line number Diff line number Diff line change
Expand Up @@ -832,7 +832,8 @@ public static void End (RunState runState)
OverlappedTop.OnAllChildClosed ();
} else {
SetCurrentOverlappedAsTop ();
Current.OnEnter (Current);
runState.Toplevel.OnLeave (Current);
Current.OnEnter (runState.Toplevel);
}
Refresh ();
}
Expand Down
2 changes: 1 addition & 1 deletion Terminal.Gui/View/ViewSubViews.cs
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ public override bool CanFocus {
SuperView?.EnsureFocus ();
if (SuperView != null && SuperView.Focused == null) {
SuperView.FocusNext ();
if (SuperView.Focused == null) {
if (SuperView.Focused == null && Application.Current != null) {
Application.Current.FocusNext ();
}
Application.BringOverlappedTopToFront ();
Expand Down
1 change: 1 addition & 0 deletions Terminal.Gui/Views/Toplevel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,7 @@ void FocusNearestView (IEnumerable<View> views, Direction direction)
///<inheritdoc/>
public override void Add (View view)
{
CanFocus = true;
AddMenuStatusBar (view);
base.Add (view);
}
Expand Down
24 changes: 0 additions & 24 deletions Terminal.Gui/Views/Window.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,29 +56,5 @@ void SetInitialProperties ()
ColorScheme = Colors.Base; // TODO: make this a theme property
BorderStyle = DefaultBorderStyle;
}

// TODO: Are these overrides really needed?
/// <inheritdoc/>
public override void Add (View view)
{
base.Add (view);
if (view.CanFocus) {
CanFocus = true;
}
AddMenuStatusBar (view);
}

/// <inheritdoc/>
public override void Remove (View view)
{
if (view == null) {
return;
}

SetNeedsDisplay ();
base.Remove (view);
RemoveMenuStatusBar (view);

}
}
}
49 changes: 46 additions & 3 deletions UnitTests/View/NavigationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -764,11 +764,25 @@ public void CanFocus_Sets_To_False_On_Single_View_Focus_View_On_Another_Toplevel
Assert.True (view1.CanFocus);
Assert.True (view1.HasFocus);
Assert.True (view2.CanFocus);
Assert.False (view2.HasFocus);
Assert.False (view2.HasFocus); // Only one of the most focused toplevels view can have focus

Assert.True (Application.Top.ProcessKey (new KeyEvent (Key.Tab, new KeyModifiers ())));
Assert.True (view1.CanFocus);
Assert.False (view1.HasFocus); // Only one of the most focused toplevels view can have focus
Assert.True (view2.CanFocus);
Assert.True (view2.HasFocus);

Assert.True (Application.Top.ProcessKey (new KeyEvent (Key.Tab, new KeyModifiers ())));
Assert.True (view1.CanFocus);
Assert.True (view1.HasFocus);
Assert.True (view2.CanFocus);
Assert.False (view2.HasFocus); // Only one of the most focused toplevels view can have focus

view1.CanFocus = false;
Assert.False (view1.CanFocus);
Assert.False (view1.HasFocus);
Assert.True (view2.CanFocus);
Assert.True (view2.HasFocus);
Assert.Equal (win2, Application.Current.Focused);
Assert.Equal (view2, Application.Current.MostFocused);
}
Expand All @@ -790,11 +804,26 @@ public void CanFocus_Sets_To_False_With_Two_Views_Focus_Another_View_On_The_Same
Assert.True (view1.CanFocus);
Assert.True (view1.HasFocus);
Assert.True (view2.CanFocus);
Assert.False (view2.HasFocus);
Assert.False (view2.HasFocus); // Only one of the most focused toplevels view can have focus

Assert.True (Application.Top.ProcessKey (new KeyEvent (Key.Tab | Key.CtrlMask, new KeyModifiers ())));
Assert.True (Application.Top.ProcessKey (new KeyEvent (Key.Tab | Key.CtrlMask, new KeyModifiers ())));
Assert.True (view1.CanFocus);
Assert.False (view1.HasFocus); // Only one of the most focused toplevels view can have focus
Assert.True (view2.CanFocus);
Assert.True (view2.HasFocus);

Assert.True (Application.Top.ProcessKey (new KeyEvent (Key.Tab | Key.CtrlMask, new KeyModifiers ())));
Assert.True (view1.CanFocus);
Assert.True (view1.HasFocus);
Assert.True (view2.CanFocus);
Assert.False (view2.HasFocus); // Only one of the most focused toplevels view can have focus

view1.CanFocus = false;
Assert.False (view1.CanFocus);
Assert.False (view1.HasFocus);
Assert.True (view2.CanFocus);
Assert.False (view2.HasFocus);
Assert.Equal (win1, Application.Current.Focused);
Assert.Equal (view12, Application.Current.MostFocused);
}
Expand All @@ -815,13 +844,27 @@ public void CanFocus_Sets_To_False_On_Toplevel_Focus_View_On_Another_Toplevel ()
Assert.True (view1.CanFocus);
Assert.True (view1.HasFocus);
Assert.True (view2.CanFocus);
Assert.False (view2.HasFocus);
Assert.False (view2.HasFocus); // Only one of the most focused toplevels view can have focus

Assert.True (Application.Top.ProcessKey (new KeyEvent (Key.Tab | Key.CtrlMask, new KeyModifiers ())));
Assert.True (view1.CanFocus);
Assert.False (view1.HasFocus); // Only one of the most focused toplevels view can have focus
Assert.True (view2.CanFocus);
Assert.True (view2.HasFocus);

Assert.True (Application.Top.ProcessKey (new KeyEvent (Key.Tab | Key.CtrlMask, new KeyModifiers ())));
Assert.True (view1.CanFocus);
Assert.True (view1.HasFocus);
Assert.True (view2.CanFocus);
Assert.False (view2.HasFocus); // Only one of the most focused toplevels view can have focus

win1.CanFocus = false;
Assert.False (view1.CanFocus);
Assert.False (view1.HasFocus);
Assert.False (win1.CanFocus);
Assert.False (win1.HasFocus);
Assert.True (view2.CanFocus);
Assert.True (view2.HasFocus);
Assert.Equal (win2, Application.Current.Focused);
Assert.Equal (view2, Application.Current.MostFocused);
}
Expand Down
110 changes: 109 additions & 1 deletion UnitTests/Views/ToplevelTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -962,7 +962,98 @@ public void OnEnter_OnLeave_Triggered_On_Application_Begin_End ()
Application.End (rs);

Assert.True (isEnter);
Assert.False (isLeave);
Assert.False (isLeave); // Leave event cannot be trigger because it v.Enter was performed and v is focused
Assert.True (v.HasFocus);
}

[Fact, AutoInitShutdown]
public void OnEnter_OnLeave_Triggered_On_Application_Begin_End_With_More_Toplevels ()
{
var iterations = 0;
var steps = new int [5];
var isEnterTop = false;
var isLeaveTop = false;
var vt = new View ();
var top = Application.Top;
var diag = new Dialog ();

vt.Enter += (s, e) => {
iterations++;
isEnterTop = true;
if (iterations == 1) {
steps [0] = iterations;
Assert.Null (e.View);
} else {
steps [4] = iterations;
Assert.Equal (diag, e.View);
}
};
vt.Leave += (s, e) => {
iterations++;
steps [1] = iterations;
isLeaveTop = true;
Assert.Equal (diag, e.View);
};
top.Add (vt);

Assert.False (vt.CanFocus);
var exception = Record.Exception (() => top.OnEnter (top));
Assert.Null (exception);
exception = Record.Exception (() => top.OnLeave (top));
Assert.Null (exception);

vt.CanFocus = true;
Application.Begin (top);

Assert.True (isEnterTop);
Assert.False (isLeaveTop);

isEnterTop = false;
var isEnterDiag = false;
var isLeaveDiag = false;
var vd = new View ();
vd.Enter += (s, e) => {
iterations++;
steps [2] = iterations;
isEnterDiag = true;
Assert.Null (e.View);
};
vd.Leave += (s, e) => {
iterations++;
steps [3] = iterations;
isLeaveDiag = true;
Assert.Equal (top, e.View);
};
diag.Add (vd);

Assert.False (vd.CanFocus);
exception = Record.Exception (() => diag.OnEnter (diag));
Assert.Null (exception);
exception = Record.Exception (() => diag.OnLeave (diag));
Assert.Null (exception);

vd.CanFocus = true;
var rs = Application.Begin (diag);

Assert.True (isEnterDiag);
Assert.False (isLeaveDiag);
Assert.False (isEnterTop);
Assert.True (isLeaveTop);

isEnterDiag = false;
isLeaveTop = false;
Application.End (rs);

Assert.False (isEnterDiag);
Assert.True (isLeaveDiag);
Assert.True (isEnterTop);
Assert.False (isLeaveTop); // Leave event cannot be trigger because it v.Enter was performed and v is focused
Assert.True (vt.HasFocus);
Assert.Equal (1, steps [0]);
Assert.Equal (2, steps [1]);
Assert.Equal (3, steps [2]);
Assert.Equal (4, steps [3]);
Assert.Equal (5, steps [^1]);
}

[Fact, AutoInitShutdown]
Expand Down Expand Up @@ -1494,5 +1585,22 @@ void Current_DrawContentComplete (object sender, DrawEventArgs e)

Application.End (rs);
}

[Fact, AutoInitShutdown]
public void Activating_MenuBar_By_Alt_Key_Does_Not_Throw ()
{
var menu = new MenuBar (new MenuBarItem [] {
new MenuBarItem ("Child", new MenuItem [] {
new MenuItem ("_Create Child", "", null)
})
});
var topChild = new Toplevel ();
topChild.Add (menu);
Application.Top.Add (topChild);
Application.Begin (Application.Top);

var exception = Record.Exception (() => topChild.ProcessHotKey (new KeyEvent (Key.AltMask, new KeyModifiers { Alt = true })));
Assert.Null (exception);
}
}
}
17 changes: 17 additions & 0 deletions UnitTests/Views/WindowTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -193,5 +193,22 @@ public void OnCanFocusChanged_Only_Must_ContentView_Forces_SetFocus_After_IsInit
Assert.False (win2.HasFocus);
Assert.False (view2.HasFocus);
}

[Fact, AutoInitShutdown]
public void Activating_MenuBar_By_Alt_Key_Does_Not_Throw ()
{
var menu = new MenuBar (new MenuBarItem [] {
new MenuBarItem ("Child", new MenuItem [] {
new MenuItem ("_Create Child", "", null)
})
});
var win = new Window ();
win.Add (menu);
Application.Top.Add (win);
Application.Begin (Application.Top);

var exception = Record.Exception (() => win.ProcessHotKey (new KeyEvent (Key.AltMask, new KeyModifiers { Alt = true })));
Assert.Null (exception);
}
}
}
Loading