Skip to content

Commit

Permalink
fix: Fix Escape handling in menus (#8916)
Browse files Browse the repository at this point in the history
Fixes that Escape being pressed isn't closing menus due to incorrect
`event.key` name.
  • Loading branch information
mister-ben authored Nov 14, 2024
1 parent ecef37c commit d0cf139
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 4 deletions.
6 changes: 3 additions & 3 deletions src/js/menu/menu-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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');
}
}
Expand Down Expand Up @@ -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();
}
Expand Down
20 changes: 19 additions & 1 deletion test/unit/menu.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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');

});

0 comments on commit d0cf139

Please sign in to comment.