Skip to content

Commit

Permalink
v2 Fixes gui-cs#2810. Pressing Alt key on a Toplevel with only a Menu…
Browse files Browse the repository at this point in the history
…Bar throws System.InvalidOperationException.
  • Loading branch information
BDisp committed Aug 15, 2023
1 parent 59b5b40 commit 5a11b88
Show file tree
Hide file tree
Showing 7 changed files with 182 additions and 33 deletions.
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
119 changes: 115 additions & 4 deletions UnitTests/Views/ToplevelTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -962,10 +962,104 @@ public void OnEnter_OnLeave_Triggered_On_Application_Begin_End ()
Application.End (rs);

Assert.True (isEnter);
Assert.False (isLeave);
}

[Fact, AutoInitShutdown]
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]
public void PositionCursor_SetCursorVisibility_To_Invisible_If_Focused_Is_Null ()
{
var tf = new TextField ("test") { Width = 5 };
Expand Down Expand Up @@ -1494,5 +1588,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);
}
}
}

0 comments on commit 5a11b88

Please sign in to comment.