Skip to content

Commit

Permalink
fix(spatial-navigation): keep navigation going when player has an err…
Browse files Browse the repository at this point in the history
…or (#8805)

## Description
The bug:
 Focus is lost when playback error is displayed.

This small PR will update the spatial-navigation logic so when the error
modal is shown the spatial-navigation will try to focus the components
present in the error modal, in most cases this will be the vjs close
button.

## Specific Changes proposed
Keep navigation working when player shows the error modal by focusing a
component in that modal.

## Requirements Checklist
- [x] Feature implemented / Bug fixed
- [ ] If necessary, more likely in a feature request than a bug fix
- [ ] Change has been verified in an actual browser (Chrome, Firefox,
IE)
  - [ ] Unit Tests updated or fixed
  - [ ] Docs/guides updated
- [ ] Example created ([starter template on
JSBin](https://codepen.io/gkatsev/pen/GwZegv?editors=1000#0))
- [ ] Has no DOM changes which impact accessiblilty or trigger warnings
(e.g. Chrome issues tab)
  - [ ] Has no changes to JSDoc which cause `npm run docs:api` to error
- [ ] Reviewed by Two Core Contributors
  • Loading branch information
CarlosVillasenor committed Jul 22, 2024
1 parent 86d29cd commit 76e99b7
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 3 deletions.
17 changes: 14 additions & 3 deletions src/js/spatial-navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ class SpatialNavigation extends EventTarget {
this.player_.on('modalclose', () => {
this.refocusComponent();
});
this.player_.on('error', () => {
this.focus(this.updateFocusableComponents()[0]);
});
this.player_.on('focusin', this.handlePlayerFocus_.bind(this));
this.player_.on('focusout', this.handlePlayerBlur_.bind(this));
this.isListening_ = true;
Expand Down Expand Up @@ -196,7 +199,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 @@ -307,7 +310,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 +471,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 +499,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
11 changes: 11 additions & 0 deletions test/unit/spatial-navigation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ QUnit.test('start method initializes event listeners', function(assert) {
assert.ok(onSpy.calledWith('loadedmetadata'), 'loadedmetadata event listener added');
assert.ok(onSpy.calledWith('modalKeydown'), 'modalKeydown event listener added');
assert.ok(onSpy.calledWith('modalclose'), 'modalclose event listener added');
assert.ok(onSpy.calledWith('error'), 'error event listener added');

// Additionally, check if isListening_ flag is set
assert.ok(this.spatialNav.isListening_, 'isListening_ flag is set');
Expand Down Expand Up @@ -491,3 +492,13 @@ 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"');
});

0 comments on commit 76e99b7

Please sign in to comment.