fix: Fix Escape handling in menus (videojs#8916)

mister-ben · web-flow · commit d0cf139384ad · 2024-11-14T09:21:07.000+01:00
Fixes that Escape being pressed isn't closing menus due to incorrect
`event.key` name.
diff --git a/src/js/menu/menu-button.js b/src/js/menu/menu-button.js
@@ -297,7 +297,7 @@ class MenuButton extends Component {
   handleKeyDown(event) {
 
     // Escape or Tab unpress the 'button'
-    if (event.key === 'Esc' || event.key === 'Tab') {
+    if (event.key === 'Escape' || event.key === 'Tab') {
       if (this.buttonPressed_) {
         this.unpressButton();
       }
@@ -328,7 +328,7 @@ class MenuButton extends Component {
    */
   handleMenuKeyUp(event) {
     // Escape hides popup menu
-    if (event.key === 'Esc' || event.key === 'Tab') {
+    if (event.key === 'Escape' || event.key === 'Tab') {
       this.removeClass('vjs-hover');
     }
   }
@@ -356,7 +356,7 @@ class MenuButton extends Component {
    */
   handleSubmenuKeyDown(event) {
     // Escape or Tab unpress the 'button'
-    if (event.key === 'Esc' || event.key === 'Tab') {
+    if (event.key === 'Escape' || event.key === 'Tab') {
       if (this.buttonPressed_) {
         this.unpressButton();
       }
diff --git a/test/unit/menu.test.js b/test/unit/menu.test.js
@@ -235,7 +235,7 @@ QUnit.test('should remove old event listeners when the menu item adds to the new
 
     assert.ok(clickListenerSpy.calledOnce, 'click event listener should be called');
     assert.strictEqual(clickListenerSpy.getCall(0).args[0].target, menuItem.el(), 'event target should be the `menuItem`');
-    assert.ok(unpressButtonSpy.calledOnce, '`menuButton`.`unpressButtion` has been called');
+    assert.ok(unpressButtonSpy.calledOnce, '`menuButton`.`unpressButton` has been called');
     assert.ok(focusSpy.calledOnce, '`menuButton`.`focus` has been called');
 
     unpressButtonSpy.restore();
@@ -265,3 +265,21 @@ QUnit.test('should remove old event listeners when the menu item adds to the new
   oldMenu.dispose();
   menuButton.dispose();
 });
+
+QUnit.test('Escape should close menu', function(assert) {
+  const player = TestHelpers.makePlayer();
+  const menuButton = new MenuButton(player, {});
+  const unpressButtonSpy = sinon.spy(menuButton, 'unpressButton');
+
+  menuButton.createItems = () => [new MenuItem(player, {})];
+  menuButton.update();
+  menuButton.handleClick(new window.PointerEvent('click'));
+  menuButton.menu.children()[0].el_.dispatchEvent(new window.KeyboardEvent('keydown', {
+    key: 'Escape',
+    bubbles: true,
+    cancelable: true
+  }));
+
+  assert.ok(unpressButtonSpy.calledOnce, '`menuButton`.`unpressButton` has been called');
+
+});

Original file line number	Diff line number	Diff line change
`@@ -297,7 +297,7 @@ class MenuButton extends Component {`
`297`	`297`	`handleKeyDown(event) {`
`298`	`298`
`299`	`299`	`// Escape or Tab unpress the 'button'`
`300`		`- if (event.key === 'Esc' \|\| event.key === 'Tab') {`
	`300`	`+ if (event.key === 'Escape' \|\| event.key === 'Tab') {`
`301`	`301`	`if (this.buttonPressed_) {`
`302`	`302`	`this.unpressButton();`
`303`	`303`	`}`
`@@ -328,7 +328,7 @@ class MenuButton extends Component {`
`328`	`328`	`*/`
`329`	`329`	`handleMenuKeyUp(event) {`
`330`	`330`	`// Escape hides popup menu`
`331`		`- if (event.key === 'Esc' \|\| event.key === 'Tab') {`
	`331`	`+ if (event.key === 'Escape' \|\| event.key === 'Tab') {`
`332`	`332`	`this.removeClass('vjs-hover');`
`333`	`333`	`}`
`334`	`334`	`}`
`@@ -356,7 +356,7 @@ class MenuButton extends Component {`
`356`	`356`	`*/`
`357`	`357`	`handleSubmenuKeyDown(event) {`
`358`	`358`	`// Escape or Tab unpress the 'button'`
`359`		`- if (event.key === 'Esc' \|\| event.key === 'Tab') {`
	`359`	`+ if (event.key === 'Escape' \|\| event.key === 'Tab') {`
`360`	`360`	`if (this.buttonPressed_) {`
`361`	`361`	`this.unpressButton();`
`362`	`362`	`}`