Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prime players to enable autoplay when out of focus #13

Merged
merged 6 commits into from
Dec 24, 2015

Conversation

cookpete
Copy link
Owner

My version of #12 that attempts to fix autoplay issues when ReactPlayer is not in a focused tab. In short:

  • Render all players at once, but only give props to the player that can play the current url
  • Each player then sets display: none on itself if url is not present
  • When ReactPlayer mounts, if YouTube or Vimeo players don't have a url, then prime the player by playing a short, silent video whilst hidden. This allows the players to autoplay any URLs given later on, even if not in a focused tab.

@Fauntleroy, any thoughts?

* Allows us to render the Vimeo player without a valid URL, which helps us fix bugs
* More or less reverts commit d308aa6
@Fauntleroy
Copy link
Contributor

I was thinking of making the priming functionality manual by using a configuration parameter:

const reactPlayerProps = {
  youtubeConfig: {
    preload: true
  },
  vimeoConfig: {
    preload: true
  }
};
<ReactPlayer {...reactPlayerProps} />

This way we could avoid loading those APIs for those who don't intend on using them.

@cookpete
Copy link
Owner Author

Updated. Moved priming logic from Base into Youtube and Vimeo players, so they can check their config and decide whether to prime or not. Should probably align the config name (currently preload) with the variable name (currently priming) and add some documentation. It also makes prop logic in ReactPlayer rather awkward so there's potential for improvement there.

@cookpete
Copy link
Owner Author

Updated. Now using slightly less ambiguous preloading var name

@cookpete
Copy link
Owner Author

Updated (again) with a short explanation in README.md

@cookpete
Copy link
Owner Author

Looking to merge this soon even if it's a bit weird.. @Fauntleroy are you happy enough?

@Fauntleroy
Copy link
Contributor

@cookpete I'll take a look at this soon to ensure that it works properly. My initial test earlier this week was having some problems. Can you check and see if youtubeConfig makes it all the way to the player?

@cookpete
Copy link
Owner Author

Can you check and see if youtubeConfig makes it all the way to the player?

Ah yeah that should have been sorted by 52574f8

@Fauntleroy
Copy link
Contributor

Something is definitely wrong when I try to use this in my application. I'm going to try a few things and see if I can resolve the problem.

@Fauntleroy
Copy link
Contributor

Autoplay seems to behave differently than it did before, but with playing and youtubeConfig i've managed to get that settled.

Volume, however, is broken. Looks like you just need to make sure volume is included in shouldComponentUpdate of ReactPlayer:

shouldComponentUpdate (nextProps) {
    return (
      this.props.url !== nextProps.url ||
      this.props.playing !== nextProps.playing ||
      this.props.volume !== nextProps.volume
    )
  }

@Fauntleroy
Copy link
Contributor

I'm also running into Uncaught TypeError: Cannot read property 'getFractionLoaded' of undefined when using the SoundCloud player, though I'm trying to figure out how it's even happening.

@Fauntleroy
Copy link
Contributor

The issue with getFractionLoaded can be solved by checking to see if this.refs.player exists during the if statement wrapping the contents of progress in ReactPlayer:

progress = () => {
    if (this.props.url && this.refs.player) {
      let progress = {}
      const loaded = this.refs.player.getFractionLoaded()
      const played = this.refs.player.getFractionPlayed()

@Fauntleroy
Copy link
Contributor

I'm also having issues rendering all of the players inside my application. For some reason only YouTube will bother to render. Here's what my React inspector is showing me:

screen shot 2015-12-18 at 11 55 52 pm

I imagine something horrible is happening to important parts of the code when minification occurs. Maybe all of the component names are being destroyed, and that's causing a few things to break here and there?

@cookpete
Copy link
Owner Author

Wow, thanks very much @Fauntleroy for the testing! I've added some more fixups to hopefully solve most of the issues.

  • Separate load and play methods are now used to clarify between actually loading media and playing it. Media will play after load if props.playing is true at the time. This should clear up the autoplay issues we've been seeing, and solidify the logic between loading and playing. For example, you can now keep playing: false but click through the demo and load media ready for playing whenever playing becomes true.
    • This actually makes autoplay config overrides sort of obsolete/pointless. If you have playing: true, media should autoplay no matter what, and if playing is false then it shouldn't. Any other autoplay settings would just confuse the logic and the players' state.
  • I've added the volume and getFractionLoaded fixes you suggested. Good spot!
  • I need to look more at the minified problems. What exact compressor/options are you using?

@Fauntleroy
Copy link
Contributor

@cookpete it might not have to do with minification. I seem to remember being able to replicate the issue without minified code, but I can't remember exactly how that came about. I'll look into it more soon.

@cookpete
Copy link
Owner Author

I'm gonna merge this in an attempt to avoid conflicts with other fixes. Hopefully all problems (other than the weird minified issue) have been sorted

* All players now need to update when url changes
* Now we're rendering all players at once, we don't want multiple progress loops going at once
* Renamed `update` to `progress` – makes a bit more sense
cookpete added a commit that referenced this pull request Dec 24, 2015
Prime players to enable autoplay when out of focus
@cookpete cookpete merged commit 9240861 into master Dec 24, 2015
@cookpete cookpete deleted the fix-autoplay-prime branch December 24, 2015 20:29
@cookpete cookpete mentioned this pull request Jan 28, 2016
@mgw-sbex mgw-sbex mentioned this pull request Jan 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

YouTube player will not autoplay if window is not focused
2 participants