Skip to content

Commit

Permalink
fix: Do not create element for MediaLoader (#4097)
Browse files Browse the repository at this point in the history
MediaLoader has a div that's unnecessary. See #4070. Also, make sure that Component#dispose does a null check for the element.
  • Loading branch information
mister-ben authored and gkatsev committed Feb 21, 2017
1 parent 127cd78 commit 1cb0a97
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 9 deletions.
4 changes: 2 additions & 2 deletions docs/guides/components.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ The architecture of the Video.js player is centered around components. The `Play

A component is a JavaScript object that has the following features:

* An associated DOM element.
* An associated DOM element, in almost all cases.
* An association to a `Player` object.
* The ability to manage any number of child components.
* The ability to listen for and trigger events.
Expand Down Expand Up @@ -285,7 +285,7 @@ The default component structure of the Video.js player looks something like this

```tree
Player
├── MediaLoader (has no UI)
├── MediaLoader (has no DOM element)
├── PosterImage
├── TextTrackDisplay
├── LoadingSpinner
Expand Down
14 changes: 8 additions & 6 deletions src/js/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -139,13 +139,15 @@ class Component {
this.childIndex_ = null;
this.childNameIndex_ = null;

// Remove element from DOM
if (this.el_.parentNode) {
this.el_.parentNode.removeChild(this.el_);
}
if (this.el_) {
// Remove element from DOM
if (this.el_.parentNode) {
this.el_.parentNode.removeChild(this.el_);
}

DomData.removeData(this.el_);
this.el_ = null;
DomData.removeData(this.el_);
this.el_ = null;
}
}

/**
Expand Down
6 changes: 5 additions & 1 deletion src/js/tech/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import Component from '../component.js';
import Tech from './tech.js';
import toTitleCase from '../utils/to-title-case.js';
import mergeOptions from '../utils/merge-options.js';

/**
* The `MediaLoader` is the `Component` that decides which playback technology to load
Expand All @@ -26,7 +27,10 @@ class MediaLoader extends Component {
* The function that is run when this component is ready.
*/
constructor(player, options, ready) {
super(player, options, ready);
// MediaLoader has no element
const options_ = mergeOptions({createEl: false}, options);

super(player, options_, ready);

// If there are no sources when the player is initialized,
// load the first supported playback technology.
Expand Down

0 comments on commit 1cb0a97

Please sign in to comment.