Tidy up FilePlayer audio/video switching logic

Follow up to cookpete/react-player#234 Checks if the element to be rendered will change, if so then remove listeners on willReceiveProps, and add listeners on didUpdate This also fixes a bug where media was pausing when unmounting
albanqoku · Sep 13, 2017 · 8f3ec89 · 8f3ec89
1 parent 5b16094
commit 8f3ec89
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 35 deletions.
diff --git a/src/demo/App.js b/src/demo/App.js
@@ -259,6 +259,7 @@ export default class App extends Component {
                 {this.renderLoadButton('http://clips.vorwaerts-gmbh.de/big_buck_bunny.mp4', 'mp4')}
                 {this.renderLoadButton('http://clips.vorwaerts-gmbh.de/big_buck_bunny.ogv', 'ogv')}
                 {this.renderLoadButton('http://clips.vorwaerts-gmbh.de/big_buck_bunny.webm', 'webm')}
+                {this.renderLoadButton('http://www.sample-videos.com/audio/mp3/crowd-cheering.mp3', 'mp3')}
                 {this.renderLoadButton(MULTIPLE_SOURCES, 'Multiple')}
                 {this.renderLoadButton('https://bitdash-a.akamaihd.net/content/MI201109210084_1/m3u8s/f08e80da-bf1d-4e3d-8899-f0f6155f6efa.m3u8', 'HLS (m3u8)')}
                 {this.renderLoadButton('http://dash.edgesuite.net/envivio/EnvivioDash3/manifest.mpd', 'DASH (mpd)')}

diff --git a/src/players/FilePlayer.js b/src/players/FilePlayer.js
@@ -34,25 +34,52 @@ export default class FilePlayer extends Base {
     )
   }
   componentDidMount () {
-    this.addDOMListeners()
+    this.addListeners()
     super.componentDidMount()
   }
+  componentWillReceiveProps (nextProps) {
+    if (this.shouldUseAudio(this.props) !== this.shouldUseAudio(nextProps)) {
+      this.removeListeners()
+    }
+    super.componentWillReceiveProps(nextProps)
+  }
   componentDidUpdate (prevProps) {
-    const { url, config } = this.props
-    const wasAudio = AUDIO_EXTENSIONS.test(prevProps.url) || prevProps.config.file.forceAudio
-    const isAudio = AUDIO_EXTENSIONS.test(url) || config.file.forceAudio
-    if (wasAudio !== isAudio) {
-      this.removeDOMListeners()
-      this.addDOMListeners()
+    if (this.shouldUseAudio(this.props) !== this.shouldUseAudio(prevProps)) {
+      this.addListeners()
     }
   }
   componentWillUnmount () {
-    this.removeDOMListeners()
+    this.removeListeners()
     super.componentWillUnmount()
   }
+  addListeners () {
+    const { playsinline, onPause, onEnded, onError } = this.props
+    this.player.addEventListener('canplay', this.onReady)
+    this.player.addEventListener('play', this.onPlay)
+    this.player.addEventListener('pause', onPause)
+    this.player.addEventListener('seeked', this.onSeek)
+    this.player.addEventListener('ended', onEnded)
+    this.player.addEventListener('error', onError)
+    if (playsinline) {
+      this.player.setAttribute('playsinline', '')
+      this.player.setAttribute('webkit-playsinline', '')
+    }
+  }
+  removeListeners () {
+    const { onPause, onEnded, onError } = this.props
+    this.player.removeEventListener('canplay', this.onReady)
+    this.player.removeEventListener('play', this.onPlay)
+    this.player.removeEventListener('pause', onPause)
+    this.player.removeEventListener('seeked', this.onSeek)
+    this.player.removeEventListener('ended', onEnded)
+    this.player.removeEventListener('error', onError)
+  }
   onSeek = e => {
     this.props.onSeek(e.target.currentTime)
   }
+  shouldUseAudio (props) {
+    return AUDIO_EXTENSIONS.test(props.url) || props.config.file.forceAudio
+  }
   shouldUseHLS (url) {
     return HLS_EXTENSIONS.test(url) || this.props.config.file.forceHLS
   }
@@ -122,35 +149,9 @@ export default class FilePlayer extends Base {
   ref = player => {
     this.player = player
   }
-  addDOMListeners () {
-    const { playsinline, onPause, onEnded, onError } = this.props
-    this.player.addEventListener('canplay', this.onReady)
-    this.player.addEventListener('play', this.onPlay)
-    this.player.addEventListener('pause', () => {
-      if (this.mounted) {
-        onPause()
-      }
-    })
-    this.player.addEventListener('seeked', this.onSeek)
-    this.player.addEventListener('ended', onEnded)
-    this.player.addEventListener('error', onError)
-    if (playsinline) {
-      this.player.setAttribute('playsinline', '')
-      this.player.setAttribute('webkit-playsinline', '')
-    }
-  }
-  removeDOMListeners () {
-    const { onPause, onEnded, onError } = this.props
-    this.player.removeEventListener('canplay', this.onReady)
-    this.player.removeEventListener('play', this.onPlay)
-    this.player.removeEventListener('pause', onPause)
-    this.player.removeEventListener('seeked', this.onSeek)
-    this.player.removeEventListener('ended', onEnded)
-    this.player.removeEventListener('error', onError)
-  }
   render () {
     const { url, loop, controls, config, width, height } = this.props
-    const useAudio = AUDIO_EXTENSIONS.test(url) || config.file.forceAudio
+    const useAudio = this.shouldUseAudio(this.props)
     const useHLS = this.shouldUseHLS(url)
     const useDASH = this.shouldUseDASH(url)
     const Element = useAudio ? 'audio' : 'video'