Skip to content

Commit

Permalink
refactor: remove most usage of innerHTML (#7337)
Browse files Browse the repository at this point in the history
  • Loading branch information
brandonocasey authored Jul 28, 2021
1 parent ada25c4 commit eb8f802
Show file tree
Hide file tree
Showing 15 changed files with 103 additions and 56 deletions.
10 changes: 8 additions & 2 deletions src/js/button.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -34,7 +35,6 @@ class Button extends ClickableComponent {
tag = 'button';

props = assign({
innerHTML: '<span aria-hidden="true" class="vjs-icon-placeholder"></span>',
className: this.buildCSSClass()
}, props);

Expand All @@ -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);

Expand Down
9 changes: 7 additions & 2 deletions src/js/clickable-component.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ class ClickableComponent extends Component {
*/
createEl(tag = 'div', props = {}, attributes = {}) {
props = assign({
innerHTML: '<span aria-hidden="true" class="vjs-icon-placeholder"></span>',
className: this.buildCSSClass(),
tabIndex: 0
}, props);
Expand All @@ -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);

Expand Down
23 changes: 11 additions & 12 deletions src/js/control-bar/audio-track-controls/audio-track-menu-item.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -46,21 +45,21 @@ class AudioTrackMenuItem extends MenuItem {
}

createEl(type, props, attrs) {
let innerHTML = `<span class="vjs-menu-item-text">${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 += `
<span aria-hidden="true" class="vjs-icon-placeholder"></span>
<span class="vjs-control-text"> ${this.localize('Descriptions')}</span>
`;
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 += '</span>';

const el = super.createEl(type, assign({
innerHTML
}, props), attrs);

return el;
}

Expand Down
10 changes: 8 additions & 2 deletions src/js/control-bar/live-display.js
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -41,12 +42,17 @@ class LiveDisplay extends Component {
});

this.contentEl_ = Dom.createEl('div', {
className: 'vjs-live-display',
innerHTML: `<span class="vjs-control-text">${this.localize('Stream Type')}\u00a0</span>${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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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_);
Expand Down Expand Up @@ -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';
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/js/control-bar/seek-to-live.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
});
Expand Down
12 changes: 5 additions & 7 deletions src/js/control-bar/spacer-controls/custom-control-spacer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
10 changes: 6 additions & 4 deletions src/js/control-bar/spacer-controls/spacer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
26 changes: 14 additions & 12 deletions src/js/control-bar/text-track-controls/subs-caps-menu-item.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -14,21 +14,23 @@ import {assign} from '../../utils/obj';
class SubsCapsMenuItem extends TextTrackMenuItem {

createEl(type, props, attrs) {
let innerHTML = `<span class="vjs-menu-item-text">${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 += `
<span aria-hidden="true" class="vjs-icon-placeholder"></span>
<span class="vjs-control-text"> ${this.localize('Captions')}</span>
`;
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 += '</span>';

const el = super.createEl(type, assign({
innerHTML
}, props), attrs);

return el;
}
}
Expand Down
11 changes: 9 additions & 2 deletions src/js/control-bar/time-controls/time-display.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: `<span class="vjs-control-text" role="presentation">${this.localize(this.labelText_)}\u00a0</span>`
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`
Expand Down
15 changes: 12 additions & 3 deletions src/js/control-bar/time-controls/time-divider.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: '<div><span>/</span></div>'
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;
}

}
Expand Down
11 changes: 8 additions & 3 deletions src/js/control-bar/volume-control/volume-level.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,15 @@ class VolumeLevel extends Component {
* The element that was created.
*/
createEl() {
return super.createEl('div', {
className: 'vjs-volume-level',
innerHTML: '<span class="vjs-control-text"></span>'
const el = super.createEl('div', {
className: 'vjs-volume-level'
});

el.appendChild(super.createEl('span', {
className: 'vjs-control-text'
}));

return el;
}

}
Expand Down
2 changes: 1 addition & 1 deletion src/js/loading-spinner.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand Down
2 changes: 1 addition & 1 deletion src/js/menu/menu-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
});

Expand Down
12 changes: 10 additions & 2 deletions src/js/menu/menu-item.js
Original file line number Diff line number Diff line change
Expand Up @@ -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. `<li>`
Expand Down Expand Up @@ -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: `<span class="vjs-menu-item-text">${this.localize(this.options_.label)}</span>`,
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;
}

/**
Expand Down

0 comments on commit eb8f802

Please sign in to comment.