Skip to content

Commit 3f8c665

Browse files
tigtig
authored
Fixes #4074 - Popover eats Key.Space (#4075)
* touching publish.yml * Added unit tests. Fixed * Actually fixed bug. * Addres Bdisp feedback * Addres Bdisp feedback2
1 parent ee8f9a8 commit 3f8c665

File tree

5 files changed

+200
-16
lines changed

5 files changed

+200
-16
lines changed

‎Terminal.Gui/View/View.Command.cs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ private void SetupCommands ()
3030

3131
SetFocus ();
3232

33+
// Always return true on hotkey, even if SetFocus fails because
34+
// hotkeys are always handled by the View (unless RaiseHandlingHotKey cancels).
3335
return true;
3436
});
3537

@@ -45,9 +47,9 @@ private void SetupCommands ()
4547

4648
if (CanFocus)
4749
{
48-
SetFocus ();
49-
50-
return true;
50+
// For Select, if the view is focusable and SetFocus succeeds, by defition,
51+
// the event is handled. So return what SetFocus returns.
52+
return SetFocus ();
5153
}
5254

5355
return false;

‎Terminal.Gui/Views/Label.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public Label ()
2525
Width = Dim.Auto (DimAutoStyle.Text);
2626

2727
// On HoKey, pass it to the next view
28-
AddCommand (Command.HotKey, InvokeHotKeyOnNext);
28+
AddCommand (Command.HotKey, InvokeHotKeyOnNextPeer);
2929

3030
TitleChanged += Label_TitleChanged;
3131
MouseClick += Label_MouseClick;
@@ -59,7 +59,7 @@ public override Rune HotKeySpecifier
5959
set => TextFormatter.HotKeySpecifier = base.HotKeySpecifier = value;
6060
}
6161

62-
private bool? InvokeHotKeyOnNext (ICommandContext commandContext)
62+
private bool? InvokeHotKeyOnNextPeer (ICommandContext commandContext)
6363
{
6464
if (RaiseHandlingHotKey () == true)
6565
{
@@ -70,16 +70,19 @@ public override Rune HotKeySpecifier
7070
{
7171
SetFocus ();
7272

73+
// Always return true on hotkey, even if SetFocus fails because
74+
// hotkeys are always handled by the View (unless RaiseHandlingHotKey cancels).
75+
// This is the same behavior as the base (View).
7376
return true;
7477
}
7578

7679
if (HotKey.IsValid)
7780
{
81+
// If the Label has a hotkey, we need to find the next view in the subview list
7882
int me = SuperView?.SubViews.IndexOf (this) ?? -1;
7983

8084
if (me != -1 && me < SuperView?.SubViews.Count - 1)
8185
{
82-
8386
return SuperView?.SubViews.ElementAt (me + 1).InvokeCommand (Command.HotKey) == true;
8487
}
8588
}

‎Tests/IntegrationTests/FluentTests/MenuBarv2Tests.cs

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -506,4 +506,79 @@ public void MenuBarItem_Without_QuitKey_Open_QuitKey_Does_Not_Quit_MenuBar_Super
506506
.WriteOutLogs (_out)
507507
.Stop ();
508508
}
509+
510+
[Theory]
511+
[ClassData (typeof (V2TestDrivers))]
512+
public void MenuBar_Not_Active_DoesNotEat_Space (V2TestDriver d)
513+
{
514+
int spaceKeyDownCount = 0;
515+
View testView = new View ()
516+
{
517+
CanFocus = true,
518+
Id = "testView",
519+
};
520+
521+
testView.KeyDown += (sender, key) =>
522+
{
523+
if (key == Key.Space)
524+
{
525+
spaceKeyDownCount++;
526+
}
527+
};
528+
529+
using GuiTestContext c = With.A<Window> (50, 20, d)
530+
.Then (
531+
() =>
532+
{
533+
var menuBar = new MenuBarv2 ();
534+
Toplevel top = Application.Top!;
535+
menuBar.EnableForDesign (ref top);
536+
Application.Top!.Add (menuBar);
537+
})
538+
.Add (testView)
539+
.WaitIteration ()
540+
.Focus (testView)
541+
.RaiseKeyDownEvent (Key.Space)
542+
.Then (() => Assert.Equal (1, spaceKeyDownCount))
543+
.WriteOutLogs (_out)
544+
.Stop ();
545+
}
546+
547+
[Theory]
548+
[ClassData (typeof (V2TestDrivers))]
549+
public void MenuBar_Not_Active_DoesNotEat_Enter (V2TestDriver d)
550+
{
551+
int enterKeyDownCount = 0;
552+
View testView = new View ()
553+
{
554+
CanFocus = true,
555+
Id = "testView",
556+
};
557+
558+
testView.KeyDown += (sender, key) =>
559+
{
560+
if (key == Key.Enter)
561+
{
562+
enterKeyDownCount++;
563+
}
564+
};
565+
566+
using GuiTestContext c = With.A<Window> (50, 20, d)
567+
.Then (
568+
() =>
569+
{
570+
var menuBar = new MenuBarv2 ();
571+
Toplevel top = Application.Top!;
572+
menuBar.EnableForDesign (ref top);
573+
Application.Top!.Add (menuBar);
574+
})
575+
.Add (testView)
576+
.WaitIteration ()
577+
.Focus (testView)
578+
.RaiseKeyDownEvent (Key.Enter)
579+
.Then (() => Assert.Equal (1, enterKeyDownCount))
580+
.WriteOutLogs (_out)
581+
.Stop ();
582+
}
583+
509584
}

‎Tests/IntegrationTests/FluentTests/PopverMenuTests.cs

Lines changed: 112 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,6 @@ public void EnableForDesign_CreatesMenuItems (V2TestDriver d)
4747
[ClassData (typeof (V2TestDrivers))]
4848
public void Activate_Sets_Application_Navigation_Correctly (V2TestDriver d)
4949
{
50-
MenuBarv2? menuBar = null;
51-
5250
using GuiTestContext c = With.A<Window> (50, 20, d)
5351
.Then (
5452
() =>
@@ -93,8 +91,6 @@ public void Activate_Sets_Application_Navigation_Correctly (V2TestDriver d)
9391
[ClassData (typeof (V2TestDrivers))]
9492
public void QuitKey_Hides (V2TestDriver d)
9593
{
96-
MenuBarv2? menuBar = null;
97-
9894
using GuiTestContext c = With.A<Window> (50, 20, d)
9995
.Then (
10096
() =>
@@ -145,8 +141,6 @@ public void QuitKey_Hides (V2TestDriver d)
145141
[ClassData (typeof (V2TestDrivers))]
146142
public void QuitKey_Restores_Focus_Correctly (V2TestDriver d)
147143
{
148-
MenuBarv2? menuBar = null;
149-
150144
using GuiTestContext c = With.A<Window> (50, 20, d)
151145
.Then (
152146
() =>
@@ -195,8 +189,6 @@ public void QuitKey_Restores_Focus_Correctly (V2TestDriver d)
195189
[ClassData (typeof (V2TestDrivers))]
196190
public void MenuBarItem_With_QuitKey_Open_QuitKey_Does_Not_Quit_App (V2TestDriver d)
197191
{
198-
MenuBarv2? menuBar = null;
199-
200192
using GuiTestContext c = With.A<Window> (50, 20, d)
201193
.Then (
202194
() =>
@@ -240,4 +232,116 @@ public void MenuBarItem_With_QuitKey_Open_QuitKey_Does_Not_Quit_App (V2TestDrive
240232
.WriteOutLogs (_out)
241233
.Stop ();
242234
}
235+
236+
237+
[Theory]
238+
[ClassData (typeof (V2TestDrivers))]
239+
public void Not_Active_DoesNotEat_Space (V2TestDriver d)
240+
{
241+
int spaceKeyDownCount = 0;
242+
View testView = new View ()
243+
{
244+
CanFocus = true,
245+
Id = "testView",
246+
};
247+
248+
testView.KeyDown += (sender, key) =>
249+
{
250+
if (key == Key.Space)
251+
{
252+
spaceKeyDownCount++;
253+
}
254+
};
255+
256+
using GuiTestContext c = With.A<Window> (50, 20, d)
257+
.Then (
258+
() =>
259+
{
260+
var popoverMenu = new PopoverMenu();
261+
Toplevel top = Application.Top!;
262+
popoverMenu.EnableForDesign (ref top);
263+
Application.Popover!.Register (popoverMenu);
264+
})
265+
.Add (testView)
266+
.WaitIteration ()
267+
.Focus (testView)
268+
.RaiseKeyDownEvent (Key.Space)
269+
.Then (() => Assert.Equal (1, spaceKeyDownCount))
270+
.WriteOutLogs (_out)
271+
.Stop ();
272+
}
273+
274+
[Theory]
275+
[ClassData (typeof (V2TestDrivers))]
276+
public void Not_Active_DoesNotEat_Enter (V2TestDriver d)
277+
{
278+
int enterKeyDownCount = 0;
279+
View testView = new View ()
280+
{
281+
CanFocus = true,
282+
Id = "testView",
283+
};
284+
285+
testView.KeyDown += (sender, key) =>
286+
{
287+
if (key == Key.Enter)
288+
{
289+
enterKeyDownCount++;
290+
}
291+
};
292+
293+
using GuiTestContext c = With.A<Window> (50, 20, d)
294+
.Then (
295+
() =>
296+
{
297+
var popoverMenu = new PopoverMenu ();
298+
Toplevel top = Application.Top!;
299+
popoverMenu.EnableForDesign (ref top);
300+
Application.Popover!.Register (popoverMenu);
301+
})
302+
.Add (testView)
303+
.WaitIteration ()
304+
.Focus (testView)
305+
.RaiseKeyDownEvent (Key.Enter)
306+
.Then (() => Assert.Equal (1, enterKeyDownCount))
307+
.WriteOutLogs (_out)
308+
.Stop ();
309+
}
310+
311+
[Theory]
312+
[ClassData (typeof (V2TestDrivers))]
313+
public void Not_Active_DoesNotEat_QuitKey (V2TestDriver d)
314+
{
315+
int quitKeyDownCount = 0;
316+
View testView = new View ()
317+
{
318+
CanFocus = true,
319+
Id = "testView",
320+
};
321+
322+
testView.KeyDown += (sender, key) =>
323+
{
324+
if (key == Application.QuitKey)
325+
{
326+
quitKeyDownCount++;
327+
}
328+
};
329+
330+
using GuiTestContext c = With.A<Window> (50, 20, d)
331+
.Then (
332+
() =>
333+
{
334+
var popoverMenu = new PopoverMenu ();
335+
Toplevel top = Application.Top!;
336+
popoverMenu.EnableForDesign (ref top);
337+
Application.Popover!.Register (popoverMenu);
338+
})
339+
.Add (testView)
340+
.WaitIteration ()
341+
.Focus (testView)
342+
.RaiseKeyDownEvent (Application.QuitKey)
343+
.Then (() => Assert.Equal (1, quitKeyDownCount))
344+
.WriteOutLogs (_out)
345+
.Stop ();
346+
}
243347
}

‎Tests/UnitTests/View/ViewCommandTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ public void Button_CanFocus_False_Raises_Accepted_Correctly ()
100100
};
101101

102102
int btnAcceptedCount = 0;
103-
bool btnCancelAccepting = false;
103+
bool btnCancelAccepting = true;
104104
var btn = new Button ()
105105
{
106106
Title = "Button",
@@ -148,7 +148,7 @@ public void Button_CanFocus_False_Raises_Accepted_Correctly ()
148148
});
149149

150150
Assert.Equal (1, btnAcceptedCount);
151-
Assert.Equal (2, wAcceptedCount);
151+
Assert.Equal (0, wAcceptedCount);
152152

153153
Application.ResetState (true);
154154
}

0 commit comments

Comments
 (0)