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

refactor: Replace keycode dependency with event.key #8735

Merged
merged 3 commits into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@
"@videojs/xhr": "2.6.0",
"aes-decrypter": "^4.0.1",
"global": "4.4.0",
"keycode": "2.2.0",
"m3u8-parser": "^7.1.0",
"mpd-parser": "^1.2.2",
"mux.js": "^7.0.1",
Expand Down
3 changes: 1 addition & 2 deletions src/js/button.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import ClickableComponent from './clickable-component.js';
import Component from './component';
import log from './utils/log.js';
import keycode from 'keycode';
import {createEl} from './utils/dom.js';

/**
Expand Down Expand Up @@ -118,7 +117,7 @@ class Button extends ClickableComponent {
// prevent the event from propagating through the DOM and triggering Player
// hotkeys. We do not preventDefault here because we _want_ the browser to
// handle it.
if (keycode.isEventKey(event, 'Space') || keycode.isEventKey(event, 'Enter')) {
if (event.key === ' ' || event.key === 'Enter') {
event.stopPropagation();
return;
}
Expand Down
3 changes: 1 addition & 2 deletions src/js/clickable-component.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import Component from './component';
import * as Dom from './utils/dom.js';
import log from './utils/log.js';
import keycode from 'keycode';

/** @import Player from './player' */

Expand Down Expand Up @@ -245,7 +244,7 @@ class ClickableComponent extends Component {
// Support Space or Enter key operation to fire a click event. Also,
// prevent the event from propagating through the DOM and triggering
// Player hotkeys.
if (keycode.isEventKey(event, 'Space') || keycode.isEventKey(event, 'Enter')) {
if (event.key === ' ' || event.key === 'Enter') {
event.preventDefault();
event.stopPropagation();
this.trigger('click');
Expand Down
3 changes: 1 addition & 2 deletions src/js/close-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
*/
import Button from './button';
import Component from './component';
import keycode from 'keycode';

/** @import Player from './player' */

Expand Down Expand Up @@ -80,7 +79,7 @@ class CloseButton extends Button {
*/
handleKeyDown(event) {
// Esc button will trigger `click` event
if (keycode.isEventKey(event, 'Esc')) {
if (event.key === 'Escape') {
event.preventDefault();
event.stopPropagation();
this.trigger('click');
Expand Down
3 changes: 1 addition & 2 deletions src/js/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import * as Fn from './utils/fn.js';
import * as Guid from './utils/guid.js';
import {toTitleCase, toLowerCase} from './utils/str.js';
import {merge} from './utils/obj.js';
import keycode from 'keycode';

/** @import Player from './player' */

Expand Down Expand Up @@ -1354,7 +1353,7 @@ class Component {

// We only stop propagation here because we want unhandled events to fall
// back to the browser. Exclude Tab for focus trapping, exclude also when spatialNavigation is enabled.
if (!keycode.isEventKey(event, 'Tab') && !(this.player_.options_.playerOptions.spatialNavigation && this.player_.options_.playerOptions.spatialNavigation.enabled)) {
if (event.key !== 'Tab' && !(this.player_.options_.playerOptions.spatialNavigation && this.player_.options_.playerOptions.spatialNavigation.enabled)) {
event.stopPropagation();
}
this.player_.handleKeyDown(event);
Expand Down
15 changes: 7 additions & 8 deletions src/js/control-bar/progress-control/seek-bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import * as Fn from '../../utils/fn.js';
import {formatTime} from '../../utils/time.js';
import {silencePromise} from '../../utils/promise';
import keycode from 'keycode';
import document from 'global/document';

/** @import Player from '../../player' */
Expand Down Expand Up @@ -438,37 +437,37 @@
handleKeyDown(event) {
const liveTracker = this.player_.liveTracker;

if (keycode.isEventKey(event, 'Space') || keycode.isEventKey(event, 'Enter')) {
if (event.key === ' ' || event.key === 'Enter') {

Check warning on line 440 in src/js/control-bar/progress-control/seek-bar.js

View check run for this annotation

Codecov / codecov/patch

src/js/control-bar/progress-control/seek-bar.js#L440

Added line #L440 was not covered by tests
event.preventDefault();
event.stopPropagation();
this.handleAction(event);
} else if (keycode.isEventKey(event, 'Home')) {
} else if (event.key === 'Home') {

Check warning on line 444 in src/js/control-bar/progress-control/seek-bar.js

View check run for this annotation

Codecov / codecov/patch

src/js/control-bar/progress-control/seek-bar.js#L444

Added line #L444 was not covered by tests
event.preventDefault();
event.stopPropagation();
this.userSeek_(0);
} else if (keycode.isEventKey(event, 'End')) {
} else if (event.key === 'End') {

Check warning on line 448 in src/js/control-bar/progress-control/seek-bar.js

View check run for this annotation

Codecov / codecov/patch

src/js/control-bar/progress-control/seek-bar.js#L448

Added line #L448 was not covered by tests
event.preventDefault();
event.stopPropagation();
if (liveTracker && liveTracker.isLive()) {
this.userSeek_(liveTracker.liveCurrentTime());
} else {
this.userSeek_(this.player_.duration());
}
} else if (/^[0-9]$/.test(keycode(event))) {
} else if (/^[0-9]$/.test(event.key)) {

Check warning on line 456 in src/js/control-bar/progress-control/seek-bar.js

View check run for this annotation

Codecov / codecov/patch

src/js/control-bar/progress-control/seek-bar.js#L456

Added line #L456 was not covered by tests
event.preventDefault();
event.stopPropagation();
const gotoFraction = (keycode.codes[keycode(event)] - keycode.codes['0']) * 10.0 / 100.0;
const gotoFraction = parseInt(event.key, 10) * 0.1;

Check warning on line 459 in src/js/control-bar/progress-control/seek-bar.js

View check run for this annotation

Codecov / codecov/patch

src/js/control-bar/progress-control/seek-bar.js#L459

Added line #L459 was not covered by tests

if (liveTracker && liveTracker.isLive()) {
this.userSeek_(liveTracker.seekableStart() + (liveTracker.liveWindow() * gotoFraction));
} else {
this.userSeek_(this.player_.duration() * gotoFraction);
}
} else if (keycode.isEventKey(event, 'PgDn')) {
} else if (event.key === 'PageDown') {

Check warning on line 466 in src/js/control-bar/progress-control/seek-bar.js

View check run for this annotation

Codecov / codecov/patch

src/js/control-bar/progress-control/seek-bar.js#L466

Added line #L466 was not covered by tests
event.preventDefault();
event.stopPropagation();
this.userSeek_(this.player_.currentTime() - (STEP_SECONDS * PAGE_KEY_MULTIPLIER));
} else if (keycode.isEventKey(event, 'PgUp')) {
} else if (event.key === 'PageUp') {

Check warning on line 470 in src/js/control-bar/progress-control/seek-bar.js

View check run for this annotation

Codecov / codecov/patch

src/js/control-bar/progress-control/seek-bar.js#L470

Added line #L470 was not covered by tests
event.preventDefault();
event.stopPropagation();
this.userSeek_(this.player_.currentTime() + (STEP_SECONDS * PAGE_KEY_MULTIPLIER));
Expand Down
5 changes: 2 additions & 3 deletions src/js/control-bar/volume-panel.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import Component from '../component.js';
import {isPlain} from '../utils/obj';
import * as Events from '../utils/events.js';
import keycode from 'keycode';
import document from 'global/document';

/** @import Player from './player' */
Expand Down Expand Up @@ -140,7 +139,7 @@
* @listens keyup
*/
handleVolumeControlKeyUp(event) {
if (keycode.isEventKey(event, 'Esc')) {
if (event.key === 'Escape') {

Check warning on line 142 in src/js/control-bar/volume-panel.js

View check run for this annotation

Codecov / codecov/patch

src/js/control-bar/volume-panel.js#L142

Added line #L142 was not covered by tests
this.muteToggle.focus();
}
}
Expand Down Expand Up @@ -185,7 +184,7 @@
* @listens keydown | keyup
*/
handleKeyPress(event) {
if (keycode.isEventKey(event, 'Esc')) {
if (event.key === 'Escape') {

Check warning on line 187 in src/js/control-bar/volume-panel.js

View check run for this annotation

Codecov / codecov/patch

src/js/control-bar/volume-panel.js#L187

Added line #L187 was not covered by tests
this.handleMouseOut();
}
}
Expand Down
13 changes: 6 additions & 7 deletions src/js/menu/menu-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import {toTitleCase} from '../utils/str.js';
import { IS_IOS } from '../utils/browser.js';
import document from 'global/document';
import keycode from 'keycode';

/** @import Player from '../player' */

Expand Down Expand Up @@ -298,19 +297,19 @@
handleKeyDown(event) {

// Escape or Tab unpress the 'button'
if (keycode.isEventKey(event, 'Esc') || keycode.isEventKey(event, 'Tab')) {
if (event.key === 'Esc' || event.key === 'Tab') {

Check warning on line 300 in src/js/menu/menu-button.js

View check run for this annotation

Codecov / codecov/patch

src/js/menu/menu-button.js#L300

Added line #L300 was not covered by tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like during review, we missed that these should be Escape rather than Esc, which keycode likely helped with. Someone noticed this issue in slack.

if (this.buttonPressed_) {
this.unpressButton();
}

// Don't preventDefault for Tab key - we still want to lose focus
if (!keycode.isEventKey(event, 'Tab')) {
if (!event.key === 'Tab') {

Check warning on line 306 in src/js/menu/menu-button.js

View check run for this annotation

Codecov / codecov/patch

src/js/menu/menu-button.js#L306

Added line #L306 was not covered by tests
event.preventDefault();
// Set focus back to the menu button's button
this.menuButton_.focus();
}
// Up Arrow or Down Arrow also 'press' the button to open the menu
} else if ((keycode.isEventKey(event, 'Up') || keycode.isEventKey(event, 'Down')) && !(this.player_.options_.playerOptions.spatialNavigation && this.player_.options_.playerOptions.spatialNavigation.enabled)) {
} else if ((event.key === 'Up') || event.key === 'Down' && !(this.player_.options_.playerOptions.spatialNavigation && this.player_.options_.playerOptions.spatialNavigation.enabled)) {

Check warning on line 312 in src/js/menu/menu-button.js

View check run for this annotation

Codecov / codecov/patch

src/js/menu/menu-button.js#L312

Added line #L312 was not covered by tests
if (!this.buttonPressed_) {
event.preventDefault();
this.pressButton();
Expand All @@ -329,7 +328,7 @@
*/
handleMenuKeyUp(event) {
// Escape hides popup menu
if (keycode.isEventKey(event, 'Esc') || keycode.isEventKey(event, 'Tab')) {
if (event.key === 'Esc' || event.key === 'Tab') {

Check warning on line 331 in src/js/menu/menu-button.js

View check run for this annotation

Codecov / codecov/patch

src/js/menu/menu-button.js#L331

Added line #L331 was not covered by tests
this.removeClass('vjs-hover');
}
}
Expand Down Expand Up @@ -357,12 +356,12 @@
*/
handleSubmenuKeyDown(event) {
// Escape or Tab unpress the 'button'
if (keycode.isEventKey(event, 'Esc') || keycode.isEventKey(event, 'Tab')) {
if (event.key === 'Esc' || event.key === 'Tab') {

Check warning on line 359 in src/js/menu/menu-button.js

View check run for this annotation

Codecov / codecov/patch

src/js/menu/menu-button.js#L359

Added line #L359 was not covered by tests
if (this.buttonPressed_) {
this.unpressButton();
}
// Don't preventDefault for Tab key - we still want to lose focus
if (!keycode.isEventKey(event, 'Tab')) {
if (!event.key === 'Tab') {

Check warning on line 364 in src/js/menu/menu-button.js

View check run for this annotation

Codecov / codecov/patch

src/js/menu/menu-button.js#L364

Added line #L364 was not covered by tests
event.preventDefault();
// Set focus back to the menu button's button
this.menuButton_.focus();
Expand Down
4 changes: 1 addition & 3 deletions src/js/menu/menu-item.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
*/
import ClickableComponent from '../clickable-component.js';
import Component from '../component.js';
import {MenuKeys} from './menu-keys.js';
import keycode from 'keycode';
import {createEl} from '../utils/dom.js';

/** @import Player from '../player' */
Expand Down Expand Up @@ -96,7 +94,7 @@
* @listens keydown
*/
handleKeyDown(event) {
if (!MenuKeys.some((key) => keycode.isEventKey(event, key))) {
if (['Tab', 'Escape', 'ArrowUp', 'ArrowLeft', 'ArrowRight', 'ArrowDown'].includes(event.key)) {

Check warning on line 97 in src/js/menu/menu-item.js

View check run for this annotation

Codecov / codecov/patch

src/js/menu/menu-item.js#L97

Added line #L97 was not covered by tests
gkatsev marked this conversation as resolved.
Show resolved Hide resolved
// Pass keydown handling up for unused keys
super.handleKeyDown(event);
}
Expand Down
20 changes: 0 additions & 20 deletions src/js/menu/menu-keys.js

This file was deleted.

5 changes: 2 additions & 3 deletions src/js/menu/menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import document from 'global/document';
import * as Dom from '../utils/dom.js';
import * as Events from '../utils/events.js';
import keycode from 'keycode';

/** @import Player from '../player' */

Expand Down Expand Up @@ -215,13 +214,13 @@
handleKeyDown(event) {

// Left and Down Arrows
if (keycode.isEventKey(event, 'Left') || keycode.isEventKey(event, 'Down')) {
if (event.key === 'ArrowLeft' || event.key === 'ArrowDown') {

Check warning on line 217 in src/js/menu/menu.js

View check run for this annotation

Codecov / codecov/patch

src/js/menu/menu.js#L217

Added line #L217 was not covered by tests
event.preventDefault();
event.stopPropagation();
this.stepForward();

// Up and Right Arrows
} else if (keycode.isEventKey(event, 'Right') || keycode.isEventKey(event, 'Up')) {
} else if (event.key === 'ArrowRight' || event.key === 'ArrowUp') {

Check warning on line 223 in src/js/menu/menu.js

View check run for this annotation

Codecov / codecov/patch

src/js/menu/menu.js#L223

Added line #L223 was not covered by tests
event.preventDefault();
event.stopPropagation();
this.stepBack();
Expand Down
5 changes: 2 additions & 3 deletions src/js/modal-dialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import * as Dom from './utils/dom';
import Component from './component';
import window from 'global/window';
import document from 'global/document';
import keycode from 'keycode';

/** @import Player from './player' */
/** @import { ContentDescriptor } from './utils/dom' */
Expand Down Expand Up @@ -470,14 +469,14 @@ class ModalDialog extends Component {
// Do not allow keydowns to reach out of the modal dialog.
event.stopPropagation();

if (keycode.isEventKey(event, 'Escape') && this.closeable()) {
if (event.key === 'Escape' && this.closeable()) {
event.preventDefault();
this.close();
return;
}

// exit early if it isn't a tab key
if (!keycode.isEventKey(event, 'Tab')) {
if (event.key !== 'Tab') {
return;
}

Expand Down
9 changes: 4 additions & 5 deletions src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import filterSource from './utils/filter-source';
import {getMimetype, findMimetype} from './utils/mimetypes';
import {hooks} from './utils/hooks';
import {isObject} from './utils/obj';
import keycode from 'keycode';
import icons from '../images/icons.svg';
import SpatialNavigation from './spatial-navigation.js';

Expand Down Expand Up @@ -3121,7 +3120,7 @@ class Player extends Component {
* Event to check for key press
*/
fullWindowOnEscKey(event) {
if (keycode.isEventKey(event, 'Esc')) {
if (event.key === 'Escape') {
if (this.isFullscreen() === true) {
if (!this.isFullWindow) {
this.exitFullscreen();
Expand Down Expand Up @@ -3371,9 +3370,9 @@ class Player extends Component {

// set fullscreenKey, muteKey, playPauseKey from `hotkeys`, use defaults if not set
const {
fullscreenKey = keydownEvent => keycode.isEventKey(keydownEvent, 'f'),
muteKey = keydownEvent => keycode.isEventKey(keydownEvent, 'm'),
playPauseKey = keydownEvent => (keycode.isEventKey(keydownEvent, 'k') || keycode.isEventKey(keydownEvent, 'Space'))
fullscreenKey = keydownEvent => (event.key.toLowerCase() === 'f'),
muteKey = keydownEvent => (event.key.toLowerCase() === 'm'),
playPauseKey = keydownEvent => (event.key.toLowerCase() === 'k' || event.key.toLowerCase() === ' ')
} = hotkeys;

if (fullscreenKey.call(this, event)) {
Expand Down
13 changes: 6 additions & 7 deletions src/js/slider/slider.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import * as Dom from '../utils/dom.js';
import {IS_CHROME} from '../utils/browser.js';
import {clamp} from '../utils/num.js';
import keycode from 'keycode';

/** @import Player from '../player' */

Expand Down Expand Up @@ -315,13 +314,13 @@
const horizontalSeek = spatialNavOptions && spatialNavOptions.horizontalSeek;

if (spatialNavEnabled) {
if ((horizontalSeek && keycode.isEventKey(event, 'Left')) ||
(!horizontalSeek && keycode.isEventKey(event, 'Down'))) {
if ((horizontalSeek && event.key === 'ArrowLeft') ||
(!horizontalSeek && event.key === 'ArrowDown')) {

Check warning on line 318 in src/js/slider/slider.js

View check run for this annotation

Codecov / codecov/patch

src/js/slider/slider.js#L317-L318

Added lines #L317 - L318 were not covered by tests
event.preventDefault();
event.stopPropagation();
this.stepBack();
} else if ((horizontalSeek && keycode.isEventKey(event, 'Right')) ||
(!horizontalSeek && keycode.isEventKey(event, 'Up'))) {
} else if ((horizontalSeek && event.key === 'ArrowRight') ||
(!horizontalSeek && event.key === 'ArrowUp')) {

Check warning on line 323 in src/js/slider/slider.js

View check run for this annotation

Codecov / codecov/patch

src/js/slider/slider.js#L322-L323

Added lines #L322 - L323 were not covered by tests
event.preventDefault();
event.stopPropagation();
this.stepForward();
Expand All @@ -330,13 +329,13 @@
}

// Left and Down Arrows
} else if (keycode.isEventKey(event, 'Left') || keycode.isEventKey(event, 'Down')) {
} else if (event.key === 'ArrowLeft' || event.key === 'ArrowDown') {

Check warning on line 332 in src/js/slider/slider.js

View check run for this annotation

Codecov / codecov/patch

src/js/slider/slider.js#L332

Added line #L332 was not covered by tests
event.preventDefault();
event.stopPropagation();
this.stepBack();

// Up and Right Arrows
} else if (keycode.isEventKey(event, 'Right') || keycode.isEventKey(event, 'Up')) {
} else if (event.key === 'ArrowUp' || event.key === 'ArrowRight') {

Check warning on line 338 in src/js/slider/slider.js

View check run for this annotation

Codecov / codecov/patch

src/js/slider/slider.js#L338

Added line #L338 was not covered by tests
event.preventDefault();
event.stopPropagation();
this.stepForward();
Expand Down
8 changes: 4 additions & 4 deletions src/js/spatial-navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
* @file spatial-navigation.js
*/
import EventTarget from './event-target';
import keycode from 'keycode';
import SpatialNavKeyCodes from './utils/spatial-navigation-key-codes';

/** @import Component from './component' */
Expand Down Expand Up @@ -82,14 +81,15 @@ class SpatialNavigation extends EventTarget {
// Determine if the event is a custom modalKeydown event
const actualEvent = event.originalEvent ? event.originalEvent : event;

if (keycode.isEventKey(actualEvent, 'left') || keycode.isEventKey(actualEvent, 'up') ||
keycode.isEventKey(actualEvent, 'right') || keycode.isEventKey(actualEvent, 'down')) {
if (['ArrowLeft', 'ArrowRight', 'ArrowUp', 'ArrowDown'].includes(actualEvent.key)) {
// Handle directional navigation
if (this.isPaused_) {
return;
}
actualEvent.preventDefault();
const direction = keycode(actualEvent);

// "ArrowLeft" => "left" etc
const direction = actualEvent.key.substring(5).toLowerCase();

this.move(direction);
} else if (SpatialNavKeyCodes.isEventKey(actualEvent, 'play') || SpatialNavKeyCodes.isEventKey(actualEvent, 'pause') ||
Expand Down
Loading