From 3ec2ac7f99704434d2fdc61642247ef0aa4e77b3 Mon Sep 17 00:00:00 2001 From: mister-ben Date: Wed, 18 May 2022 16:59:17 +0200 Subject: [PATCH] feat: Player can be replaced with original el after dispose() (#7722) --- src/js/component.js | 11 ++++++++-- src/js/player.js | 4 ++-- src/js/video.js | 6 +++++ test/unit/component.test.js | 12 ++++++++++ test/unit/player.test.js | 14 ++++++++++++ test/unit/video.test.js | 44 +++++++++++++++++++++++++++++++++++++ 6 files changed, 87 insertions(+), 4 deletions(-) diff --git a/src/js/component.js b/src/js/component.js index 728a51c77..ae03fe683 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -137,8 +137,11 @@ class Component { * Dispose of the `Component` and all child components. * * @fires Component#dispose + * + * @param {Object} options + * @param {Element} options.originalEl element with which to replace player element */ - dispose() { + dispose(options = {}) { // Bail out if the component has already been disposed. if (this.isDisposed_) { @@ -182,7 +185,11 @@ class Component { if (this.el_) { // Remove element from DOM if (this.el_.parentNode) { - this.el_.parentNode.removeChild(this.el_); + if (options.restoreEl) { + this.el_.parentNode.replaceChild(options.restoreEl, this.el_); + } else { + this.el_.parentNode.removeChild(this.el_); + } } this.el_ = null; diff --git a/src/js/player.js b/src/js/player.js index f2c6f8beb..4f782fdc3 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -666,8 +666,8 @@ class Player extends Component { } }); - // the actual .el_ is removed here - super.dispose(); + // the actual .el_ is removed here, or replaced if + super.dispose({restoreEl: this.options_.restoreEl}); } /** diff --git a/src/js/video.js b/src/js/video.js index 87c58cf8d..a2772cbf2 100644 --- a/src/js/video.js +++ b/src/js/video.js @@ -162,6 +162,12 @@ function videojs(id, options, ready) { options = options || {}; + // Store a copy of the el before modification, if it is to be restored in destroy() + // If div ingest, store the parent div + if (options.restoreEl === true) { + options.restoreEl = (el.parentNode && el.parentNode.hasAttribute('data-vjs-player') ? el.parentNode : el).cloneNode(true); + } + hooks('beforesetup').forEach((hookFunction) => { const opts = hookFunction(el, mergeOptions(options)); diff --git a/test/unit/component.test.js b/test/unit/component.test.js index 793ea7344..a7c3dd1b1 100644 --- a/test/unit/component.test.js +++ b/test/unit/component.test.js @@ -1431,3 +1431,15 @@ QUnit.test('ready queue should not run after dispose', function(assert) { assert.notOk(callback, 'ready callback not run'); }); + +QUnit.test('a component\'s el can be replaced on dispose', function(assert) { + const comp = this.player.addChild('Component', {}, {}, 2); + const prevIndex = Array.from(this.player.el_.childNodes).indexOf(comp.el_); + const replacementEl = document.createElement('div'); + + comp.dispose({restoreEl: replacementEl}); + + assert.strictEqual(replacementEl.parentNode, this.player.el_, 'replacement was inserted'); + assert.strictEqual(Array.from(this.player.el_.childNodes).indexOf(replacementEl), prevIndex, 'replacement was inserted at same position'); + +}); diff --git a/test/unit/player.test.js b/test/unit/player.test.js index 0f081cb25..6468f5eff 100644 --- a/test/unit/player.test.js +++ b/test/unit/player.test.js @@ -105,6 +105,20 @@ QUnit.test('dispose should not throw if playerEl is missing', function(assert) { assert.notOk(error, 'Function did not throw an error on dispose'); }); +QUnit.test('dispose should replace playerEl with restoreEl', function(assert) { + const videoTag = TestHelpers.makeTag(); + const fixture = document.getElementById('qunit-fixture'); + const replacement = document.createElement('div'); + + fixture.appendChild(videoTag); + + const player = new Player(videoTag, {restoreEl: replacement}); + + player.dispose(); + + assert.ok(replacement.parentNode, fixture, 'Replacement node present after dispose'); +}); + // technically, all uses of videojs.options should be replaced with // Player.prototype.options_ in this file and a equivalent test using // videojs.options should be made in video.test.js. Keeping this here diff --git a/test/unit/video.test.js b/test/unit/video.test.js index 259742aca..602f39db6 100644 --- a/test/unit/video.test.js +++ b/test/unit/video.test.js @@ -579,3 +579,47 @@ QUnit.test('adds video-js class name with the video-js embed', function(assert) assert.ok(player.hasClass('video-js'), 'video-js class was added to the first embed'); assert.ok(player2.hasClass('video-js'), 'video-js class was preserved to the second embed'); }); + +QUnit.test('stores placeholder el and restores on dispose', function(assert) { + const fixture = document.getElementById('qunit-fixture'); + + const embeds = [ + { + type: 'video el', + src: '', + initSelector: 'test1', + testSelector: '#test1' + }, + { + type: 'video-js el', + src: '', + initSelector: 'test2', + testSelector: '#test2' + }, + { + type: 'div ingest', + src: '
', + initSelector: 'test3', + testSelector: 'div[data-vjs-player]' + } + ]; + + embeds.forEach(embed => { + const comparisonEl = document.createRange().createContextualFragment(embed.src).children[0]; + + fixture.innerHTML += embed.src; + + const player = videojs(embed.initSelector, {restoreEl: true}); + + assert.ok(comparisonEl.isEqualNode(player.options_.restoreEl), `${embed.type}: restoreEl option replaced by an element`); + assert.notOk(document.querySelector(embed.testSelector).isSameNode(player.options_.restoreEl), `${embed.type}: restoreEl is not the original element`); + assert.notOk(comparisonEl.isSameNode(player.options_.restoreEl), `${embed.type}: restoreEl is not the control element`); + + player.dispose(); + + const expectedEl = document.querySelector(embed.testSelector); + + assert.ok(comparisonEl.isEqualNode(expectedEl), `${embed.type}: element was restored`); + + }); +});