From 9b3d9415aa914f3647c449097c8d7f6bb0670ef1 Mon Sep 17 00:00:00 2001 From: Grzegorz Blaszczyk Date: Mon, 8 Aug 2022 17:14:06 +0200 Subject: [PATCH] feat: addClass and removeClass method supports adding/removing multiple classes (#7798) --- src/js/component.js | 16 +++---- src/js/control-bar/mute-toggle.js | 5 +- src/js/control-bar/play-toggle.js | 3 +- src/js/player.js | 9 ++-- src/js/tracks/text-track-display.js | 3 +- src/js/utils/dom.js | 73 +++++------------------------ test/unit/component.test.js | 35 ++++++++++++++ test/unit/utils/dom.test.js | 42 +++++++++++------ 8 files changed, 89 insertions(+), 97 deletions(-) diff --git a/src/js/component.js b/src/js/component.js index 8b3c951241..d48d179e80 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -815,21 +815,21 @@ class Component { /** * Add a CSS class name to the `Component`s element. * - * @param {string} classToAdd - * CSS class name to add + * @param {...string} classesToAdd + * One or more CSS class name to add. */ - addClass(classToAdd) { - Dom.addClass(this.el_, classToAdd); + addClass(...classesToAdd) { + Dom.addClass(this.el_, ...classesToAdd); } /** * Remove a CSS class name from the `Component`s element. * - * @param {string} classToRemove - * CSS class name to remove + * @param {...string} classesToRemove + * One or more CSS class name to remove. */ - removeClass(classToRemove) { - Dom.removeClass(this.el_, classToRemove); + removeClass(...classesToRemove) { + Dom.removeClass(this.el_, ...classesToRemove); } /** diff --git a/src/js/control-bar/mute-toggle.js b/src/js/control-bar/mute-toggle.js index 0d9fcf29bd..6545ac58e8 100644 --- a/src/js/control-bar/mute-toggle.js +++ b/src/js/control-bar/mute-toggle.js @@ -113,10 +113,7 @@ class MuteToggle extends Button { level = 2; } - // TODO improve muted icon classes - for (let i = 0; i < 4; i++) { - Dom.removeClass(this.el_, `vjs-vol-${i}`); - } + Dom.removeClass(this.el_, [0, 1, 2, 3].reduce((str, i) => str + `${i ? ' ' : ''}vjs-vol-${i}`, '')); Dom.addClass(this.el_, `vjs-vol-${level}`); } diff --git a/src/js/control-bar/play-toggle.js b/src/js/control-bar/play-toggle.js index cf3bcd7a85..d86f9bfc9c 100644 --- a/src/js/control-bar/play-toggle.js +++ b/src/js/control-bar/play-toggle.js @@ -92,8 +92,7 @@ class PlayToggle extends Button { * @listens Player#play */ handlePlay(event) { - this.removeClass('vjs-ended'); - this.removeClass('vjs-paused'); + this.removeClass('vjs-ended', 'vjs-paused'); this.addClass('vjs-playing'); // change the button text to "Pause" this.controlText('Pause'); diff --git a/src/js/player.js b/src/js/player.js index e8bef81d5a..2073537867 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -1384,8 +1384,7 @@ class Player extends Component { handleTechLoadStart_() { // TODO: Update to use `emptied` event instead. See #1277. - this.removeClass('vjs-ended'); - this.removeClass('vjs-seeking'); + this.removeClass('vjs-ended', 'vjs-seeking'); // reset the error state this.error(null); @@ -1662,8 +1661,7 @@ class Player extends Component { * @private */ handleTechPlay_() { - this.removeClass('vjs-ended'); - this.removeClass('vjs-paused'); + this.removeClass('vjs-ended', 'vjs-paused'); this.addClass('vjs-playing'); // hide the poster when the user hits play @@ -1816,8 +1814,7 @@ class Player extends Component { * @private */ handleTechSeeked_() { - this.removeClass('vjs-seeking'); - this.removeClass('vjs-ended'); + this.removeClass('vjs-seeking', 'vjs-ended'); /** * Fired when the player has finished jumping to a new time * diff --git a/src/js/tracks/text-track-display.js b/src/js/tracks/text-track-display.js index 533000c709..408ddc7668 100644 --- a/src/js/tracks/text-track-display.js +++ b/src/js/tracks/text-track-display.js @@ -418,8 +418,7 @@ class TextTrackDisplay extends Component { for (let j = 0; j < track.activeCues.length; ++j) { const cueEl = track.activeCues[j].displayState; - Dom.addClass(cueEl, 'vjs-text-track-cue'); - Dom.addClass(cueEl, 'vjs-text-track-cue-' + ((track.language) ? track.language : i)); + Dom.addClass(cueEl, 'vjs-text-track-cue', 'vjs-text-track-cue-' + ((track.language) ? track.language : i)); if (track.language) { Dom.setAttribute(cueEl, 'lang', track.language); } diff --git a/src/js/utils/dom.js b/src/js/utils/dom.js index c6321ee0d2..d86700f6ec 100644 --- a/src/js/utils/dom.js +++ b/src/js/utils/dom.js @@ -47,21 +47,6 @@ function throwIfWhitespace(str) { } } -/** - * Produce a regular expression for matching a className within an elements className. - * - * @private - * @param {string} className - * The className to generate the RegExp for. - * - * @return {RegExp} - * The RegExp that will check for a specific `className` in an elements - * className. - */ -function classRegExp(className) { - return new RegExp('(^|\\s)' + className + '($|\\s)'); -} - /** * Whether the current DOM interface appears to be real (i.e. not simulated). * @@ -228,10 +213,8 @@ export function prependTo(child, parent) { */ export function hasClass(element, classToCheck) { throwIfWhitespace(classToCheck); - if (element.classList) { - return element.classList.contains(classToCheck); - } - return classRegExp(classToCheck).test(element.className); + + return element.classList.contains(classToCheck); } /** @@ -240,21 +223,14 @@ export function hasClass(element, classToCheck) { * @param {Element} element * Element to add class name to. * - * @param {string} classToAdd - * Class name to add. + * @param {...string} classesToAdd + * One or more class name to add. * * @return {Element} * The DOM element with the added class name. */ -export function addClass(element, classToAdd) { - if (element.classList) { - element.classList.add(classToAdd); - - // Don't need to `throwIfWhitespace` here because `hasElClass` will do it - // in the case of classList not being supported. - } else if (!hasClass(element, classToAdd)) { - element.className = (element.className + ' ' + classToAdd).trim(); - } +export function addClass(element, ...classesToAdd) { + element.classList.add(...classesToAdd.reduce((prev, current) => prev.concat(current.split(/\s+/)), [])); return element; } @@ -265,26 +241,19 @@ export function addClass(element, classToAdd) { * @param {Element} element * Element to remove a class name from. * - * @param {string} classToRemove - * Class name to remove + * @param {...string} classesToRemove + * One or more class name to remove. * * @return {Element} * The DOM element with class name removed. */ -export function removeClass(element, classToRemove) { +export function removeClass(element, ...classesToRemove) { // Protect in case the player gets disposed if (!element) { log.warn("removeClass was called with an element that doesn't exist"); return null; } - if (element.classList) { - element.classList.remove(classToRemove); - } else { - throwIfWhitespace(classToRemove); - element.className = element.className.split(/\s+/).filter(function(c) { - return c !== classToRemove; - }).join(' '); - } + element.classList.remove(...classesToRemove.reduce((prev, current) => prev.concat(current.split(/\s+/)), [])); return element; } @@ -322,31 +291,13 @@ export function removeClass(element, classToRemove) { * The element with a class that has been toggled. */ export function toggleClass(element, classToToggle, predicate) { - - // This CANNOT use `classList` internally because IE11 does not support the - // second parameter to the `classList.toggle()` method! Which is fine because - // `classList` will be used by the add/remove functions. - const has = hasClass(element, classToToggle); - if (typeof predicate === 'function') { predicate = predicate(element, classToToggle); } - if (typeof predicate !== 'boolean') { - predicate = !has; - } - - // If the necessary class operation matches the current state of the - // element, no action is required. - if (predicate === has) { - return; - } - - if (predicate) { - addClass(element, classToToggle); - } else { - removeClass(element, classToToggle); + predicate = undefined; } + classToToggle.split(/\s+/).forEach(className => element.classList.toggle(className, predicate)); return element; } diff --git a/test/unit/component.test.js b/test/unit/component.test.js index 169d1f7a65..0eeafd9331 100644 --- a/test/unit/component.test.js +++ b/test/unit/component.test.js @@ -677,6 +677,41 @@ QUnit.test('should add and remove a CSS class', function(assert) { comp.dispose(); }); +QUnit.test('should add and remove CSS classes', function(assert) { + const comp = new Component(this.player, {}); + + comp.addClass('first-class', 'second-class'); + assert.ok(comp.el().className.indexOf('first-class') !== -1); + assert.ok(comp.el().className.indexOf('second-class') !== -1); + comp.removeClass('first-class', 'second-class'); + assert.ok(comp.el().className.indexOf('first-class') === -1); + assert.ok(comp.el().className.indexOf('second-class') === -1); + + comp.addClass('first-class second-class'); + assert.ok(comp.el().className.indexOf('first-class') !== -1); + assert.ok(comp.el().className.indexOf('second-class') !== -1); + comp.removeClass('first-class second-class'); + assert.ok(comp.el().className.indexOf('first-class') === -1); + assert.ok(comp.el().className.indexOf('second-class') === -1); + + comp.addClass('be cool', 'scooby', 'doo'); + assert.ok(comp.el().className.indexOf('be cool scooby doo') !== -1); + comp.removeClass('be cool', 'scooby', 'doo'); + assert.ok(comp.el().className.indexOf('be cool scooby doo') === -1); + + comp.addClass('multiple spaces between words'); + assert.ok(comp.el().className.indexOf('multiple spaces between words') !== -1); + comp.removeClass('multiple spaces between words'); + assert.ok(comp.el().className.indexOf('multiple spaces between words') === -1); + + comp.toggleClass('first-class second-class'); + assert.ok(comp.el().className.indexOf('first-class second-class') !== -1); + comp.toggleClass('first-class second-class'); + assert.ok(comp.el().className.indexOf('first-class second-class') === -1); + + comp.dispose(); +}); + QUnit.test('should add CSS class passed in options', function(assert) { const comp = new Component(this.player, {className: 'class1 class2'}); diff --git a/test/unit/utils/dom.test.js b/test/unit/utils/dom.test.js index 85d58dfe67..811cd4c617 100644 --- a/test/unit/utils/dom.test.js +++ b/test/unit/utils/dom.test.js @@ -65,7 +65,7 @@ QUnit.test('should insert an element first in another', function(assert) { QUnit.test('addClass()', function(assert) { const el = document.createElement('div'); - assert.expect(5); + assert.expect(6); Dom.addClass(el, 'test-class'); assert.strictEqual(el.className, 'test-class', 'adds a single class'); @@ -73,15 +73,21 @@ QUnit.test('addClass()', function(assert) { Dom.addClass(el, 'test-class'); assert.strictEqual(el.className, 'test-class', 'does not duplicate classes'); - assert.throws(function() { - Dom.addClass(el, 'foo foo-bar'); - }, 'throws when attempting to add a class with whitespace'); - Dom.addClass(el, 'test2_className'); assert.strictEqual(el.className, 'test-class test2_className', 'adds second class'); Dom.addClass(el, 'FOO'); assert.strictEqual(el.className, 'test-class test2_className FOO', 'adds third class'); + + Dom.addClass(el, 'left-class', 'right-class'); + assert.strictEqual(el.className, 'test-class test2_className FOO left-class right-class', 'adds two classes'); + + Dom.addClass(el, 'l-class r-class'); + assert.strictEqual( + el.className, + 'test-class test2_className FOO left-class right-class l-class r-class', + 'adds two classes via one string' + ); }); QUnit.test('removeClass()', function(assert) { @@ -89,20 +95,26 @@ QUnit.test('removeClass()', function(assert) { el.className = 'test-class test2_className FOO bar'; - assert.expect(4); + assert.expect(5); Dom.removeClass(el, 'test-class'); assert.strictEqual(el.className, 'test2_className FOO bar', 'removes one class'); - assert.throws(function() { - Dom.removeClass(el, 'test2_className bar'); - }, 'throws when attempting to remove a class with whitespace'); - Dom.removeClass(el, 'test2_className'); assert.strictEqual(el.className, 'FOO bar', 'removes another class'); Dom.removeClass(el, 'FOO'); assert.strictEqual(el.className, 'bar', 'removes another class'); + + el.className = 'bar left-class right-class'; + + Dom.removeClass(el, 'left-class', 'right-class'); + assert.strictEqual(el.className, 'bar', 'removes two classes'); + + el.className = 'bar l-class r-class'; + + Dom.removeClass(el, 'l-class r-class'); + assert.strictEqual(el.className, 'bar', 'removes two classes via one string'); }); QUnit.test('hasClass()', function(assert) { @@ -217,7 +229,7 @@ QUnit.test('toggleClass()', function(assert) { } ]; - assert.expect(3 + predicateToggles.length); + assert.expect(4 + predicateToggles.length); Dom.toggleClass(el, 'bar'); assert.strictEqual(el.className, 'foo', 'toggles a class off, if present'); @@ -225,9 +237,11 @@ QUnit.test('toggleClass()', function(assert) { Dom.toggleClass(el, 'bar'); assert.strictEqual(el.className, 'foo bar', 'toggles a class on, if absent'); - assert.throws(function() { - Dom.toggleClass(el, 'foo bar'); - }, 'throws when attempting to toggle a class with whitespace'); + Dom.toggleClass(el, 'bla ok'); + assert.strictEqual(el.className, 'foo bar bla ok', 'toggles a classes on, if absent'); + + Dom.toggleClass(el, 'bla ok'); + assert.strictEqual(el.className, 'foo bar', 'toggles a classes off, if present'); predicateToggles.forEach(x => { Dom.toggleClass(el, x.toggle, x.predicate);