From eb8f802391392406ad9801a76934aa7a98dd1bdb Mon Sep 17 00:00:00 2001 From: Brandon Casey <2381475+brandonocasey@users.noreply.github.com> Date: Wed, 28 Jul 2021 16:42:23 -0400 Subject: [PATCH] refactor: remove most usage of innerHTML (#7337) --- src/js/button.js | 10 +++++-- src/js/clickable-component.js | 9 +++++-- .../audio-track-menu-item.js | 23 ++++++++-------- src/js/control-bar/live-display.js | 10 +++++-- .../playback-rate-menu-button.js | 4 +-- src/js/control-bar/seek-to-live.js | 2 +- .../spacer-controls/custom-control-spacer.js | 12 ++++----- src/js/control-bar/spacer-controls/spacer.js | 10 ++++--- .../subs-caps-menu-item.js | 26 ++++++++++--------- .../control-bar/time-controls/time-display.js | 11 ++++++-- .../control-bar/time-controls/time-divider.js | 15 ++++++++--- .../volume-control/volume-level.js | 11 +++++--- src/js/loading-spinner.js | 2 +- src/js/menu/menu-button.js | 2 +- src/js/menu/menu-item.js | 12 +++++++-- 15 files changed, 103 insertions(+), 56 deletions(-) diff --git a/src/js/button.js b/src/js/button.js index bbfd2f14d5..010f9de647 100644 --- a/src/js/button.js +++ b/src/js/button.js @@ -6,6 +6,7 @@ import Component from './component'; import log from './utils/log.js'; import {assign} from './utils/obj'; import keycode from 'keycode'; +import {createEl} from './utils/dom.js'; /** * Base class for all buttons. @@ -34,7 +35,6 @@ class Button extends ClickableComponent { tag = 'button'; props = assign({ - innerHTML: '', className: this.buildCSSClass() }, props); @@ -45,7 +45,13 @@ class Button extends ClickableComponent { type: 'button' }, attributes); - const el = Component.prototype.createEl.call(this, tag, props, attributes); + const el = createEl(tag, props, attributes); + + el.appendChild(createEl('span', { + className: 'vjs-icon-placeholder' + }, { + 'aria-hidden': true + })); this.createControlTextEl(el); diff --git a/src/js/clickable-component.js b/src/js/clickable-component.js index fbdecce10a..f9d5021bad 100644 --- a/src/js/clickable-component.js +++ b/src/js/clickable-component.js @@ -57,7 +57,6 @@ class ClickableComponent extends Component { */ createEl(tag = 'div', props = {}, attributes = {}) { props = assign({ - innerHTML: '', className: this.buildCSSClass(), tabIndex: 0 }, props); @@ -73,7 +72,13 @@ class ClickableComponent extends Component { this.tabIndex_ = props.tabIndex; - const el = super.createEl(tag, props, attributes); + const el = Dom.createEl(tag, props, attributes); + + el.appendChild(Dom.createEl('span', { + className: 'vjs-icon-placeholder' + }, { + 'aria-hidden': true + })); this.createControlTextEl(el); diff --git a/src/js/control-bar/audio-track-controls/audio-track-menu-item.js b/src/js/control-bar/audio-track-controls/audio-track-menu-item.js index 86dfe0f010..e8820a53be 100644 --- a/src/js/control-bar/audio-track-controls/audio-track-menu-item.js +++ b/src/js/control-bar/audio-track-controls/audio-track-menu-item.js @@ -3,7 +3,6 @@ */ import MenuItem from '../../menu/menu-item.js'; import Component from '../../component.js'; -import {assign} from '../../utils/obj'; /** * An {@link AudioTrack} {@link MenuItem} @@ -46,21 +45,21 @@ class AudioTrackMenuItem extends MenuItem { } createEl(type, props, attrs) { - let innerHTML = `${this.localize(this.options_.label)}`; + const el = super.createEl(type, props, attrs); + const parentSpan = el.querySelector('.vjs-menu-item-text'); if (this.options_.track.kind === 'main-desc') { - innerHTML += ` - - ${this.localize('Descriptions')} - `; + parentSpan.appendChild(super.createEl('span', { + className: 'vjs-icon-placeholder' + }, { + 'aria-hidden': true + })); + parentSpan.appendChild(super.createEl('span', { + className: 'vjs-control-text', + textContent: this.localize('Descriptions') + })); } - innerHTML += ''; - - const el = super.createEl(type, assign({ - innerHTML - }, props), attrs); - return el; } diff --git a/src/js/control-bar/live-display.js b/src/js/control-bar/live-display.js index f3c0beddca..bf0a9c9408 100644 --- a/src/js/control-bar/live-display.js +++ b/src/js/control-bar/live-display.js @@ -3,6 +3,7 @@ */ import Component from '../component'; import * as Dom from '../utils/dom.js'; +import document from 'global/document'; // TODO - Future make it click to snap to live @@ -41,12 +42,17 @@ class LiveDisplay extends Component { }); this.contentEl_ = Dom.createEl('div', { - className: 'vjs-live-display', - innerHTML: `${this.localize('Stream Type')}\u00a0${this.localize('LIVE')}` + className: 'vjs-live-display' }, { 'aria-live': 'off' }); + this.contentEl_.appendChild(Dom.createEl('span', { + className: 'vjs-control-text', + textContent: `${this.localize('Stream Type')}\u00a0` + })); + this.contentEl_.appendChild(document.createTextNode(this.localize('LIVE'))); + el.appendChild(this.contentEl_); return el; } diff --git a/src/js/control-bar/playback-rate-menu/playback-rate-menu-button.js b/src/js/control-bar/playback-rate-menu/playback-rate-menu-button.js index 17dc275163..d9c5d585ff 100644 --- a/src/js/control-bar/playback-rate-menu/playback-rate-menu-button.js +++ b/src/js/control-bar/playback-rate-menu/playback-rate-menu-button.js @@ -49,7 +49,7 @@ class PlaybackRateMenuButton extends MenuButton { this.labelEl_ = Dom.createEl('div', { className: 'vjs-playback-rate-value', id: this.labelElId_, - innerHTML: '1x' + textContent: '1x' }); el.appendChild(this.labelEl_); @@ -190,7 +190,7 @@ class PlaybackRateMenuButton extends MenuButton { */ updateLabel(event) { if (this.playbackRateSupported()) { - this.labelEl_.innerHTML = this.player().playbackRate() + 'x'; + this.labelEl_.textContent = this.player().playbackRate() + 'x'; } } diff --git a/src/js/control-bar/seek-to-live.js b/src/js/control-bar/seek-to-live.js index 82c3fc82c1..9d896f03d0 100644 --- a/src/js/control-bar/seek-to-live.js +++ b/src/js/control-bar/seek-to-live.js @@ -45,7 +45,7 @@ class SeekToLive extends Button { this.textEl_ = Dom.createEl('span', { className: 'vjs-seek-to-live-text', - innerHTML: this.localize('LIVE') + textContent: this.localize('LIVE') }, { 'aria-hidden': 'true' }); diff --git a/src/js/control-bar/spacer-controls/custom-control-spacer.js b/src/js/control-bar/spacer-controls/custom-control-spacer.js index bc14447953..3913a8a8c0 100644 --- a/src/js/control-bar/spacer-controls/custom-control-spacer.js +++ b/src/js/control-bar/spacer-controls/custom-control-spacer.js @@ -28,14 +28,12 @@ class CustomControlSpacer extends Spacer { * The element that was created. */ createEl() { - const el = super.createEl({ - className: this.buildCSSClass() + return super.createEl('div', { + className: this.buildCSSClass(), + // No-flex/table-cell mode requires there be some content + // in the cell to fill the remaining space of the table. + textContent: '\u00a0' }); - - // No-flex/table-cell mode requires there be some content - // in the cell to fill the remaining space of the table. - el.innerHTML = '\u00a0'; - return el; } } diff --git a/src/js/control-bar/spacer-controls/spacer.js b/src/js/control-bar/spacer-controls/spacer.js index 4a8386997c..eff67f9d07 100644 --- a/src/js/control-bar/spacer-controls/spacer.js +++ b/src/js/control-bar/spacer-controls/spacer.js @@ -27,10 +27,12 @@ class Spacer extends Component { * @return {Element} * The element that was created. */ - createEl() { - return super.createEl('div', { - className: this.buildCSSClass() - }); + createEl(tag = 'div', props = {}, attributes = {}) { + if (!props.className) { + props.className = this.buildCSSClass(); + } + + return super.createEl(tag, props, attributes); } } diff --git a/src/js/control-bar/text-track-controls/subs-caps-menu-item.js b/src/js/control-bar/text-track-controls/subs-caps-menu-item.js index 403b9867d8..fde8f89608 100644 --- a/src/js/control-bar/text-track-controls/subs-caps-menu-item.js +++ b/src/js/control-bar/text-track-controls/subs-caps-menu-item.js @@ -3,7 +3,7 @@ */ import TextTrackMenuItem from './text-track-menu-item.js'; import Component from '../../component.js'; -import {assign} from '../../utils/obj'; +import {createEl} from '../../utils/dom.js'; /** * SubsCapsMenuItem has an [cc] icon to distinguish captions from subtitles @@ -14,21 +14,23 @@ import {assign} from '../../utils/obj'; class SubsCapsMenuItem extends TextTrackMenuItem { createEl(type, props, attrs) { - let innerHTML = `${this.localize(this.options_.label)}`; + const el = super.createEl(type, props, attrs); + const parentSpan = el.querySelector('.vjs-menu-item-text'); if (this.options_.track.kind === 'captions') { - innerHTML += ` - - ${this.localize('Captions')} - `; + parentSpan.appendChild(createEl('span', { + className: 'vjs-icon-placeholder' + }, { + 'aria-hidden': true + })); + parentSpan.appendChild(createEl('span', { + className: 'vjs-control-text', + // space added as the text will visually flow with the + // label + textContent: ` ${this.localize('Captions')}` + })); } - innerHTML += ''; - - const el = super.createEl(type, assign({ - innerHTML - }, props), attrs); - return el; } } diff --git a/src/js/control-bar/time-controls/time-display.js b/src/js/control-bar/time-controls/time-display.js index 7fbea15615..99999ee628 100644 --- a/src/js/control-bar/time-controls/time-display.js +++ b/src/js/control-bar/time-controls/time-display.js @@ -39,9 +39,16 @@ class TimeDisplay extends Component { createEl() { const className = this.buildCSSClass(); const el = super.createEl('div', { - className: `${className} vjs-time-control vjs-control`, - innerHTML: `${this.localize(this.labelText_)}\u00a0` + className: `${className} vjs-time-control vjs-control` }); + const span = Dom.createEl('span', { + className: 'vjs-control-text', + textContent: `${this.localize(this.labelText_)}\u00a0` + }, { + role: 'presentation' + }); + + el.appendChild(span); this.contentEl_ = Dom.createEl('span', { className: `${className}-display` diff --git a/src/js/control-bar/time-controls/time-divider.js b/src/js/control-bar/time-controls/time-divider.js index d30e09a546..970e2d121d 100644 --- a/src/js/control-bar/time-controls/time-divider.js +++ b/src/js/control-bar/time-controls/time-divider.js @@ -18,15 +18,24 @@ class TimeDivider extends Component { * The element that was created. */ createEl() { - return super.createEl('div', { - className: 'vjs-time-control vjs-time-divider', - innerHTML: '
/
' + const el = super.createEl('div', { + className: 'vjs-time-control vjs-time-divider' }, { // this element and its contents can be hidden from assistive techs since // it is made extraneous by the announcement of the control text // for the current time and duration displays 'aria-hidden': true }); + + const div = super.createEl('div'); + const span = super.createEl('span', { + textContent: '/' + }); + + div.appendChild(span); + el.appendChild(div); + + return el; } } diff --git a/src/js/control-bar/volume-control/volume-level.js b/src/js/control-bar/volume-control/volume-level.js index 88bb0fe448..679035a21d 100644 --- a/src/js/control-bar/volume-control/volume-level.js +++ b/src/js/control-bar/volume-control/volume-level.js @@ -17,10 +17,15 @@ class VolumeLevel extends Component { * The element that was created. */ createEl() { - return super.createEl('div', { - className: 'vjs-volume-level', - innerHTML: '' + const el = super.createEl('div', { + className: 'vjs-volume-level' }); + + el.appendChild(super.createEl('span', { + className: 'vjs-control-text' + })); + + return el; } } diff --git a/src/js/loading-spinner.js b/src/js/loading-spinner.js index 93b6e49600..02e5f952f5 100644 --- a/src/js/loading-spinner.js +++ b/src/js/loading-spinner.js @@ -22,7 +22,7 @@ class LoadingSpinner extends Component { const playerType = this.localize(isAudio ? 'Audio Player' : 'Video Player'); const controlText = dom.createEl('span', { className: 'vjs-control-text', - innerHTML: this.localize('{1} is loading.', [playerType]) + textContent: this.localize('{1} is loading.', [playerType]) }); const el = super.createEl('div', { diff --git a/src/js/menu/menu-button.js b/src/js/menu/menu-button.js index c341d45f72..6b9c48f9d7 100644 --- a/src/js/menu/menu-button.js +++ b/src/js/menu/menu-button.js @@ -116,7 +116,7 @@ class MenuButton extends Component { if (this.options_.title) { const titleEl = Dom.createEl('li', { className: 'vjs-menu-title', - innerHTML: toTitleCase(this.options_.title), + textContent: toTitleCase(this.options_.title), tabIndex: -1 }); diff --git a/src/js/menu/menu-item.js b/src/js/menu/menu-item.js index c34622f103..f8b660f2b9 100644 --- a/src/js/menu/menu-item.js +++ b/src/js/menu/menu-item.js @@ -6,6 +6,7 @@ import Component from '../component.js'; import {assign} from '../utils/obj'; import {MenuKeys} from './menu-keys.js'; import keycode from 'keycode'; +import {createEl} from '../utils/dom.js'; /** * The component for a menu item. `
  • ` @@ -63,11 +64,18 @@ class MenuItem extends ClickableComponent { // The control is textual, not just an icon this.nonIconControl = true; - return super.createEl('li', assign({ + const el = super.createEl('li', assign({ className: 'vjs-menu-item', - innerHTML: `${this.localize(this.options_.label)}`, tabIndex: -1 }, props), attrs); + + // swap icon with menu item text. + el.replaceChild(createEl('span', { + className: 'vjs-menu-item-text', + textContent: this.localize(this.options_.label) + }), el.querySelector('.vjs-icon-placeholder')); + + return el; } /**