Skip to content

Commit

Permalink
fix(error-display): update display on consecutive errors (#8485)
Browse files Browse the repository at this point in the history
When consecutive errors occur, the `ErrorDisplay` component is not updated with the new error message.
This results in an inconsistent state between the `player.error` and `player.errorDisplay.contentEl().textContent`.

|                         | player.error() | player.errorDisplay.content() | player.errorDisplay.contentEl().textContent |
| ----------------------- | -------------- | ----------------------------- | ------------------------------------------- |
| player.error('Error 1') | Error 1 ✔️      | Error 1 ✔️                     | Error 1 ✔️                                   |
| player.error('Error 2') | Error 2 ✔️      | Error 2 ✔️                     | Error 1 ❌                                  |

An example of a use case where updating the error message is useful is :
- user tries to play media 1 but the media doestn't exist
- user tries to play media 2 but the media is not compatible

- call the `close` function before each call to the `open` function.
  - if errorDisplay is not **open**, the `close` function does nothing
  - if errorDisplay is **open**, the `close` function executes and triggers the close events, then the open function executes and triggers the open events, ensuring that the content is updated.
  • Loading branch information
amtins authored Nov 28, 2023
1 parent d535e16 commit 7831046
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 1 deletion.
5 changes: 4 additions & 1 deletion src/js/error-display.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ class ErrorDisplay extends ModalDialog {
*/
constructor(player, options) {
super(player, options);
this.on(player, 'error', (e) => this.open(e));
this.on(player, 'error', (e) => {
this.close();
this.open(e);
});
}

/**
Expand Down
63 changes: 63 additions & 0 deletions test/unit/error-display.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/* eslint-env qunit */
import TestHelpers from './test-helpers.js';

QUnit.module('ErrorDisplay');

QUnit.test('should update errorDisplay when several errors occur in succession', function(assert) {
const player = TestHelpers.makePlayer({ techOrder: ['html5'] });
const events = {
beforemodalopen: 0,
beforemodalclose: 0,
modalopen: 0,
modalclose: 0
};

player.errorDisplay.on(
['beforemodalopen', 'beforemodalclose', 'modalopen', 'modalclose'],
({ type }) => {
events[type] += 1;
}
);

// Dummy case for initial state
assert.strictEqual(player.error(), null, 'error is null');
assert.strictEqual(
player.errorDisplay.contentEl().textContent,
'',
'error display contentEl textContent is empty'
);
assert.strictEqual(events.beforemodalopen, 0, 'beforemodalopen was not called');
assert.strictEqual(events.modalopen, 0, 'modalopen was not called');
assert.strictEqual(events.beforemodalclose, 0, 'beforemodalclose was not called');
assert.strictEqual(events.modalclose, 0, 'modalclose was not called');

// Case for first error
player.error('Error 1');

assert.strictEqual(player.error().message, 'Error 1', 'error has message');
assert.strictEqual(
player.errorDisplay.contentEl().textContent,
'Error 1',
'error display contentEl textContent has content'
);
assert.strictEqual(events.beforemodalopen, 1, 'beforemodalopen was called once');
assert.strictEqual(events.modalopen, 1, 'modalopen was called once');
assert.strictEqual(events.beforemodalclose, 0, 'beforemodalclose was not called');
assert.strictEqual(events.modalclose, 0, 'modalclose was not called');

// Case for second error
player.error('Error 2');

assert.strictEqual(player.error().message, 'Error 2', 'error has new message');
assert.strictEqual(
player.errorDisplay.contentEl().textContent,
'Error 2',
'error display contentEl textContent has been updated with the new error message'
);
assert.strictEqual(events.beforemodalopen, 2, 'beforemodalopen has been called for the second time');
assert.strictEqual(events.modalopen, 2, 'modalopen has been called for the second time');
assert.strictEqual(events.beforemodalclose, 1, 'beforemodalclose was called once');
assert.strictEqual(events.modalclose, 1, 'modalclose was called once');

player.dispose();
});

0 comments on commit 7831046

Please sign in to comment.