Skip to content

Commit

Permalink
Merge branch 'main' into fix-seek-throttle
Browse files Browse the repository at this point in the history
  • Loading branch information
mister-ben authored Jul 30, 2024
2 parents dfc7ee0 + c4007db commit 17eb87e
Show file tree
Hide file tree
Showing 8 changed files with 1,651 additions and 1,611 deletions.
25 changes: 25 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,28 @@
<a name="8.17.3"></a>
## [8.17.3](https://github.com/videojs/video.js/compare/v8.17.2...v8.17.3) (2024-07-30)

### Bug Fixes

* Listen to taps on track controls ([#8809](https://github.com/videojs/video.js/issues/8809)) ([86d29cd](https://github.com/videojs/video.js/commit/86d29cd)), closes [#8808](https://github.com/videojs/video.js/issues/8808) [1000#0](https://github.com/1000/issues/0)
* Refactor evented to make mincompatable with Chrome 53 ([#8810](https://github.com/videojs/video.js/issues/8810)) ([a7c9f26](https://github.com/videojs/video.js/commit/a7c9f26)), closes [/github.com/videojs/video.js/blob/e78bcc7b2d829fce01451cf105b918d8feec4548/src/js/mixins/evented.js#L165-L195](https://github.com//github.com/videojs/video.js/blob/e78bcc7b2d829fce01451cf105b918d8feec4548/src/js/mixins/evented.js/issues/L165-L195) [#8783](https://github.com/videojs/video.js/issues/8783) [1000#0](https://github.com/1000/issues/0)
* **spatial-navigation:** focus lost in error modal ([#8817](https://github.com/videojs/video.js/issues/8817)) ([daf40bd](https://github.com/videojs/video.js/commit/daf40bd)), closes [1000#0](https://github.com/1000/issues/0)
* **spatial-navigation:** keep navigation going when player has an error ([#8805](https://github.com/videojs/video.js/issues/8805)) ([76e99b7](https://github.com/videojs/video.js/commit/76e99b7)), closes [1000#0](https://github.com/1000/issues/0)
* **spatial-navigation:** refocus available also to the close button of the error modal ([#8819](https://github.com/videojs/video.js/issues/8819)) ([45570d9](https://github.com/videojs/video.js/commit/45570d9)), closes [1000#0](https://github.com/1000/issues/0)

<a name="8.17.2"></a>
## [8.17.2](https://github.com/videojs/video.js/compare/v8.17.1...v8.17.2) (2024-07-22)

### Chores

* update vhs version 3.13.2 ([#8812](https://github.com/videojs/video.js/issues/8812)) ([49151ee](https://github.com/videojs/video.js/commit/49151ee)), closes [1000#0](https://github.com/1000/issues/0)

<a name="8.17.1"></a>
## [8.17.1](https://github.com/videojs/video.js/compare/v8.17.0...v8.17.1) (2024-07-15)

### Bug Fixes

* ensure transient button event listeners are removed on dispose ([#8796](https://github.com/videojs/video.js/issues/8796)) ([0a836e1](https://github.com/videojs/video.js/commit/0a836e1)), closes [#8795](https://github.com/videojs/video.js/issues/8795) [1000#0](https://github.com/1000/issues/0)

<a name="8.17.0"></a>
# [8.17.0](https://github.com/videojs/video.js/compare/v8.16.1...v8.17.0) (2024-07-10)

Expand Down
3,002 changes: 1,400 additions & 1,602 deletions package-lock.json

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "video.js",
"description": "An HTML5 video player that supports HLS and DASH with a common API and skin.",
"version": "8.17.0",
"version": "8.17.3",
"main": "./dist/video.cjs.js",
"module": "./dist/video.es.js",
"style": "./dist/video-js.css",
Expand Down Expand Up @@ -86,7 +86,7 @@
},
"dependencies": {
"@babel/runtime": "^7.12.5",
"@videojs/http-streaming": "3.13.1",
"@videojs/http-streaming": "3.13.2",
"@videojs/vhs-utils": "^4.0.0",
"@videojs/xhr": "2.7.0",
"aes-decrypter": "^4.0.1",
Expand Down Expand Up @@ -161,7 +161,7 @@
"shx": "^0.3.2",
"sinon": "^11.1.1",
"typescript": "^5.5.2",
"uglify-js": "^3.6.0",
"uglify-js": "^3.19.0",
"unified": "^7.0.2",
"videojs-generate-karma-config": "^8.1.0",
"videojs-languages": "^2.0.0",
Expand Down
7 changes: 6 additions & 1 deletion src/js/mixins/evented.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,12 @@ const normalizeListenArgs = (self, args, fnName) => {

[type, listener] = args;
} else {
[target, type, listener] = args;
// This was `[target, type, listener] = args;` but this block needs more than
// one statement to produce minified output compatible with Chrome 53.
// See https://github.com/videojs/video.js/pull/8810
target = args[0];
type = args[1];
listener = args[2];
}

validateTarget(target, self, fnName);
Expand Down
8 changes: 8 additions & 0 deletions src/js/modal-dialog.js
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,14 @@ class ModalDialog extends Component {
if (closeButton) {
parentEl.appendChild(closeButton.el_);
}

/**
* Fired after `ModalDialog` is re-filled with content & close button is appended.
*
* @event ModalDialog#aftermodalfill
* @type {Event}
*/
this.trigger('aftermodalfill');
}

/**
Expand Down
89 changes: 86 additions & 3 deletions src/js/spatial-navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,25 @@ class SpatialNavigation extends EventTarget {
this.player_.on('focusin', this.handlePlayerFocus_.bind(this));
this.player_.on('focusout', this.handlePlayerBlur_.bind(this));
this.isListening_ = true;
this.player_.errorDisplay.on('aftermodalfill', () => {
this.updateFocusableComponents();

if (this.focusableComponents.length) {
// The modal has focusable components:

if (this.focusableComponents.length > 1) {
// The modal has close button + some additional buttons.
// Focusing first additional button:

this.focusableComponents[1].focus();
} else {
// The modal has only close button,
// Focusing it:

this.focusableComponents[0].focus();
}
}
});
}

/**
Expand Down Expand Up @@ -196,7 +215,7 @@ class SpatialNavigation extends EventTarget {
}

if (!(event.currentTarget.contains(event.relatedTarget)) && !isChildrenOfPlayer || !nextFocusedElement) {
if (currentComponent.name() === 'CloseButton') {
if (currentComponent && currentComponent.name() === 'CloseButton') {
this.refocusComponent();
} else {
this.pause();
Expand Down Expand Up @@ -267,9 +286,65 @@ class SpatialNavigation extends EventTarget {
focusableComponents.push(value);
}
}

// TODO - Refactor the following logic after refactor of videojs-errors elements to be components is done.
if (value.name_ === 'ErrorDisplay' && value.opened_) {
const buttonContainer = value.el_.querySelector('.vjs-errors-ok-button-container');

if (buttonContainer) {
const modalButtons = buttonContainer.querySelectorAll('button');

modalButtons.forEach((element, index) => {
// Add elements as objects to be handled by the spatial navigation
focusableComponents.push({
name: () => {
return 'ModalButton' + (index + 1);
},
el: () => element,
getPositions: () => {
const rect = element.getBoundingClientRect();

// Creating objects that mirror DOMRectReadOnly for boundingClientRect and center
const boundingClientRect = {
x: rect.x,
y: rect.y,
width: rect.width,
height: rect.height,
top: rect.top,
right: rect.right,
bottom: rect.bottom,
left: rect.left
};

// Calculating the center position
const center = {
x: rect.left + rect.width / 2,
y: rect.top + rect.height / 2,
width: 0,
height: 0,
top: rect.top + rect.height / 2,
right: rect.left + rect.width / 2,
bottom: rect.top + rect.height / 2,
left: rect.left + rect.width / 2
};

return {
boundingClientRect,
center
};
},
// Asume that the following are always focusable
getIsAvailableToBeFocused: () => true,
getIsFocusable: (el) => true,
focus: () => element.focus()
});
});
}
}
});

this.focusableComponents = focusableComponents;

return this.focusableComponents;
}

Expand Down Expand Up @@ -307,7 +382,11 @@ class SpatialNavigation extends EventTarget {
return null;
}

return searchForSuitableChild(component.el());
if (component.el()) {
return searchForSuitableChild(component.el());
}
return null;

}

/**
Expand Down Expand Up @@ -464,7 +543,7 @@ class SpatialNavigation extends EventTarget {
*/
refocusComponent() {
if (this.lastFocusedComponent_) {
// If use is not active, set it to active.
// If user is not active, set it to active.
if (!this.player_.userActive()) {
this.player_.userActive(true);
}
Expand Down Expand Up @@ -492,6 +571,10 @@ class SpatialNavigation extends EventTarget {
* @param {Component} component - The component to be focused.
*/
focus(component) {
if (typeof component !== 'object') {
return;
}

if (component.getIsAvailableToBeFocused(component.el())) {
component.focus();
} else if (this.findSuitableDOMChild(component)) {
Expand Down
4 changes: 2 additions & 2 deletions src/js/tracks/text-track-settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -332,12 +332,12 @@ class TextTrackSettings extends ModalDialog {
}

bindFunctionsToSelectsAndButtons() {
this.on(this.$('.vjs-done-button'), 'click', () => {
this.on(this.$('.vjs-done-button'), ['click', 'tap'], () => {
this.saveSettings();
this.close();
});

this.on(this.$('.vjs-default-button'), 'click', () => {
this.on(this.$('.vjs-default-button'), ['click', 'tap'], () => {
this.setDefaults();
this.updateDisplay();
});
Expand Down
121 changes: 121 additions & 0 deletions test/unit/spatial-navigation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import TestHelpers from './test-helpers.js';
import sinon from 'sinon';
import document from 'global/document';
import TextTrackSelect from '../../src/js/tracks/text-track-select';
import * as Dom from '../../src/js/utils/dom.js';

QUnit.module('SpatialNavigation', {
beforeEach() {
Expand Down Expand Up @@ -491,3 +492,123 @@ QUnit.test('should call `searchForTrackSelect()` if spatial navigation is enable

assert.ok(trackSelectSpy.calledOnce);
});

QUnit.test('error on player calls updateFocusableComponents', function(assert) {
const updateFocusableComponentsSpy = sinon.spy(this.spatialNav, 'updateFocusableComponents');

this.spatialNav.start();

this.player.error('Error 1');

assert.ok(updateFocusableComponentsSpy.calledOnce, 'on error event spatial navigation should call "updateFocusableComponents"');
});

QUnit.test('error on player focus the second focusable element of error modal', function(assert) {
this.spatialNav.start();

const firstComponent = {
name: () => 'firstComponent',
el: () => document.createElement('div'),
focus: sinon.spy(),
getIsAvailableToBeFocused: () => true
};

const secondComponent = {
name: () => 'secondComponent',
el: () => document.createElement('div'),
focus: sinon.spy(),
getIsAvailableToBeFocused: () => true
};

this.spatialNav.focusableComponents = [firstComponent, secondComponent];
this.spatialNav.getCurrentComponent = () => firstComponent;
this.spatialNav.updateFocusableComponents = () => [firstComponent, secondComponent];

this.player.error({
code: 1,
dismiss: true
});

assert.ok(secondComponent.focus.calledOnce, 'Focus should move to the second component');
assert.notOk(firstComponent.focus.called, 'Focus should not remain on the first component');
});

QUnit.test('on error, modalButtons should get the buttons if those are available', function(assert) {
this.spatialNav.start();

const buttonContainer = Dom.createEl('div', {}, {class: 'vjs-errors-ok-button-container'});

const testEl1 = Dom.createEl('button', {}, {class: 'c1'});

// Add first element to error modal
buttonContainer.appendChild(testEl1);

const testEl2 = Dom.createEl('button', {}, {class: 'c1'});

// Add second element to error modal
buttonContainer.appendChild(testEl2);
this.player.errorDisplay.el().appendChild(buttonContainer);

this.player.error({
code: 1,
dismiss: true
});

this.spatialNav.getCurrentComponent = () => this.spatialNav.focusableComponents[0];

const getPositionsEl1Spy = sinon.spy(this.spatialNav.focusableComponents[0], 'getPositions');
const getPositionsEl2Spy = sinon.spy(this.spatialNav.focusableComponents[1], 'getPositions');

this.spatialNav.move('left');

assert.strictEqual(this.spatialNav.focusableComponents.length, 2, 'button elements are now part of the array of focusableComponents');
assert.ok(getPositionsEl1Spy.calledOnce, 'getPositions method called on button');
assert.ok(getPositionsEl2Spy.calledOnce, 'getPositions method called on button');
assert.strictEqual(this.spatialNav.focusableComponents[0].name(), 'ModalButton1', 'testEl1 name should be ModalButton1');
assert.strictEqual(this.spatialNav.focusableComponents[1].name(), 'ModalButton2', 'testEl2 name should be ModalButton2');

getPositionsEl1Spy.restore();
getPositionsEl2Spy.restore();
});

QUnit.test('on error, modalButtons added functions should work properly', function(assert) {
this.spatialNav.start();

const buttonContainer = Dom.createEl('div', {}, {class: 'vjs-errors-ok-button-container'});

const testEl1 = Dom.createEl('button', {}, {class: 'c1'});

// Add first element to error modal
buttonContainer.appendChild(testEl1);

this.player.errorDisplay.el().appendChild(buttonContainer);

this.player.error({ code: 1, dismiss: true });

assert.strictEqual(this.spatialNav.focusableComponents[0].el() instanceof Element, true, 'el function from modal buttons should return a DOM element'); // eslint-disable-line no-undef
assert.strictEqual(this.spatialNav.focusableComponents[0].getIsFocusable(), true, 'getIsFocusable function from modal buttons is always true');
assert.strictEqual(this.spatialNav.focusableComponents[0].getIsAvailableToBeFocused(), true, 'getIsAvailableToBeFocused function from modal buttons is always true');
assert.strictEqual(this.spatialNav.focusableComponents[0].getIsAvailableToBeFocused(), true, 'getIsAvailableToBeFocused function from modal buttons is always true');
assert.strictEqual(typeof this.spatialNav.focusableComponents[0].getPositions(), 'object', 'focusableComponents function from modal buttons should return an object');
});

QUnit.test('If component passes the required functions it should be added to focusableComponents', function(assert) {
this.spatialNav.start();

const firstComponent = {
name_: 'firstComponent',
name: () => this.name,
el_: document.createElement('div'),
el: () => this.el_,
focus: sinon.spy(),
getIsAvailableToBeFocused: () => true,
getIsFocusable: () => true
};

this.player.children_.push(firstComponent);
this.spatialNav.getCurrentComponent = () => firstComponent;
this.spatialNav.updateFocusableComponents();

assert.strictEqual(this.spatialNav.focusableComponents.length, 1, 'focusableComponents array should have 1 component');
assert.strictEqual(this.spatialNav.focusableComponents[0].name_, 'firstComponent', 'the name of the component in focusableComponents array should be "firstComponent"');
});

0 comments on commit 17eb87e

Please sign in to comment.