(doc) Adds guide for removing players from the page #1803

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
3 participants
@brycefisher
Contributor

brycefisher commented Jan 14, 2015

Added a guide to explain how to safely remove players from the page.

Ran into a situation like this:
http://jsbin.com/gahususoju

I couldn't find this info anywhere in the guides or documentation, so thought I'd chip in :-)

brycefisher added some commits Jan 14, 2015

Create removing-players.md
Added a guide to explain how to safely remove players from the page
Update index.md
Added link to removing players guide

@brycefisher brycefisher changed the title from Create removing-players.md to (doc) Adds guide for removing players from the page Jan 14, 2015

@gkatsev

This comment has been minimized.

Show comment
Hide comment
@gkatsev

gkatsev Jan 15, 2015

Member

LGTM. @mmcc?

Member

gkatsev commented Jan 15, 2015

LGTM. @mmcc?

@mmcc

View changes

docs/guides/removing-players.md
+Or...Use Unique Ids
+-------------------
+
+If you prefer not to call `dispose()` on a player, you can always create new players with unique ids for that page load.

This comment has been minimized.

@mmcc

mmcc Jan 15, 2015

Member

I'd probably suggest just removing this bit altogether. Any time you remove a player from the DOM (or hide it, or just don't need it anymore), it's a good idea to remove it, even if just from a resource management perspective.

@mmcc

mmcc Jan 15, 2015

Member

I'd probably suggest just removing this bit altogether. Any time you remove a player from the DOM (or hide it, or just don't need it anymore), it's a good idea to remove it, even if just from a resource management perspective.

+```
+TypeError: this.el_.vjs_getProperty is not a function
+"VIDEOJS:" "Video.js: buffered unavailable on Hls playback technology element." TypeError: this.el_.vjs_getProperty is not a function
+Stack trace:

This comment has been minimized.

@mmcc

mmcc Jan 15, 2015

Member

Might want to explicitly mention this would happen when using the contrib-hls plugin.

@mmcc

mmcc Jan 15, 2015

Member

Might want to explicitly mention this would happen when using the contrib-hls plugin.

@mmcc

This comment has been minimized.

Show comment
Hide comment
@mmcc

mmcc Jan 15, 2015

Member

Looks great! The only other thing I'd suggest is adding a section about hiding / showing a player. For instance, if you've got a modal that a player pops up in, you should create and dispose the player when the player shows / hides. If the Flash tech is used and you try to hide it, things will go poorly because, Flash.

Thanks for the PR!

Member

mmcc commented Jan 15, 2015

Looks great! The only other thing I'd suggest is adding a section about hiding / showing a player. For instance, if you've got a modal that a player pops up in, you should create and dispose the player when the player shows / hides. If the Flash tech is used and you try to hide it, things will go poorly because, Flash.

Thanks for the PR!

@brycefisher

This comment has been minimized.

Show comment
Hide comment
@brycefisher

brycefisher Jan 15, 2015

Contributor

Thanks for the extra info! I'll update the branch with your feedback.

Contributor

brycefisher commented Jan 15, 2015

Thanks for the extra info! I'll update the branch with your feedback.

@brycefisher

This comment has been minimized.

Show comment
Hide comment
@brycefisher

brycefisher Jan 15, 2015

Contributor

If this looks good to you @gkatsev and @mmcc, I'll squash this down into a single commit in a new PR

Contributor

brycefisher commented Jan 15, 2015

If this looks good to you @gkatsev and @mmcc, I'll squash this down into a single commit in a new PR

@gkatsev

This comment has been minimized.

Show comment
Hide comment
@gkatsev

gkatsev Jan 15, 2015

Member

Probably no need, since we squash it up during out merge process.

Member

gkatsev commented Jan 15, 2015

Probably no need, since we squash it up during out merge process.

@brycefisher

This comment has been minimized.

Show comment
Hide comment
@brycefisher

brycefisher Jan 15, 2015

Contributor

okey-dokey!

Contributor

brycefisher commented Jan 15, 2015

okey-dokey!

mmcc added a commit to mmcc/video.js that referenced this pull request Jan 16, 2015

@mmcc mmcc closed this Jan 16, 2015

mmcc added a commit to mmcc/video.js that referenced this pull request Jan 16, 2015

@brycefisher

This comment has been minimized.

Show comment
Hide comment
@brycefisher

brycefisher Jan 18, 2015

Contributor

Woot! Thanks for accepting my PR @mmcc and @gkatsev

Contributor

brycefisher commented Jan 18, 2015

Woot! Thanks for accepting my PR @mmcc and @gkatsev

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment