From 4a1fab93965854e6150f0575f4a811a1be790a14 Mon Sep 17 00:00:00 2001 From: Ben Wiley Date: Mon, 25 Feb 2019 21:39:31 -0500 Subject: [PATCH] Fixes to help avoid progressbar jumping around during track load. --- packages/core/src/PlayerContextProvider.js | 56 +++++++++++-------- packages/core/src/utils/getInitialDuration.js | 15 +++++ 2 files changed, 48 insertions(+), 23 deletions(-) create mode 100644 packages/core/src/utils/getInitialDuration.js diff --git a/packages/core/src/PlayerContextProvider.js b/packages/core/src/PlayerContextProvider.js index 6b859c80..b236de3d 100644 --- a/packages/core/src/PlayerContextProvider.js +++ b/packages/core/src/PlayerContextProvider.js @@ -17,7 +17,7 @@ import getRepeatStrategy from './utils/getRepeatStrategy'; import convertToNumberWithinIntervalBounds from './utils/convertToNumberWithinIntervalBounds'; import { logError, logWarning } from './utils/console'; import getDisplayText from './utils/getDisplayText'; -import parseTimeString from './utils/parseTimeString'; +import getInitialDuration from './utils/getInitialDuration'; import { repeatStrategyOptions } from './constants'; function playErrorHandler(err) { @@ -90,16 +90,8 @@ function getGoToTrackState({ const isNewTrack = prevState.activeTrackIndex !== index; const shouldLoadAsNew = Boolean(isNewTrack || shouldForceLoad); const currentTime = track.startingTime || 0; - let duration = 0; - if (track.duration) { - if (typeof track.duration === 'string') { - duration = parseTimeString(track.duration); - } else { - duration = track.duration; - } - } return { - duration, + duration: getInitialDuration(track), activeTrackIndex: index, trackLoading: shouldLoadAsNew, mediaCannotPlay: prevState.mediaCannotPlay && !shouldLoadAsNew, @@ -123,7 +115,8 @@ export class PlayerContextProvider extends Component { props.startingTrackIndex, 0 ); - if (isPlaylistValid(props.playlist) && props.playlist[activeTrackIndex]) { + const playlistIsValid = isPlaylistValid(props.playlist); + if (playlistIsValid && props.playlist[activeTrackIndex]) { currentTime = props.playlist[activeTrackIndex].startingTime || 0; } const { initialStateSnapshot } = props; @@ -165,9 +158,12 @@ export class PlayerContextProvider extends Component { // true if user is currently dragging mouse to change the volume setVolumeInProgress: false, // initialize shouldRequestPlayOnNextUpdate from autoplay prop - shouldRequestPlayOnNextUpdate: - props.autoplay && isPlaylistValid(props.playlist), + shouldRequestPlayOnNextUpdate: props.autoplay && playlistIsValid, awaitingForceLoad: false, + // duration might be set on track object + duration: getInitialDuration( + playlistIsValid && props.playlist[activeTrackIndex] + ), // playlist prop copied to state (for getDerivedStateFromProps) __playlist__: props.playlist, // load overrides from previously-captured state snapshot @@ -725,10 +721,12 @@ export class PlayerContextProvider extends Component { handleMediaTimeupdate() { const { currentTime, played } = this.media; const { onTimeUpdate, playlist } = this.props; - const { activeTrackIndex } = this.state; - if (this.state.trackLoading) { - // correct currentTime to preset, if applicable, during load - this.media.currentTime = this.state.currentTime; + const { activeTrackIndex, trackLoading } = this.state; + if (trackLoading) { + // we'll get another time update when the track loads + // but for now this helps us avoid unnecessarily + // jumping back to currentTime: 0 in the UI while + // the track is loading. return; } this.setState(state => ({ @@ -925,9 +923,12 @@ export class PlayerContextProvider extends Component { ...baseStateUpdate, awaitingResumeOnSeekComplete: paused ? awaitingResumeOnSeekComplete - : true + : true, + currentTime: targetTime })); - this.media.currentTime = targetTime; + if (!this.state.trackLoading) { + this.media.currentTime = targetTime; + } if (!this.state.paused) { this.togglePause(true); } @@ -937,9 +938,12 @@ export class PlayerContextProvider extends Component { ...baseStateUpdate, awaitingResumeOnSeekComplete: paused ? awaitingResumeOnSeekComplete - : true + : true, + currentTime: targetTime })); - this.media.currentTime = targetTime; + if (!this.state.trackLoading) { + this.media.currentTime = targetTime; + } if (this.state.awaitingResumeOnSeekComplete && !this.media.ended) { // if we earlier encountered an 'ended' state, // un-pausing becomes necessary to resume playback @@ -953,7 +957,11 @@ export class PlayerContextProvider extends Component { } seekComplete(targetTime) { - const { seekPreviewTime, awaitingResumeOnSeekComplete } = this.state; + const { + seekPreviewTime, + awaitingResumeOnSeekComplete, + trackLoading + } = this.state; const baseStateUpdate = { seekInProgress: false, awaitingResumeOnSeekComplete: false @@ -974,7 +982,9 @@ export class PlayerContextProvider extends Component { */ currentTime }); - this.media.currentTime = currentTime; + if (!trackLoading) { + this.media.currentTime = currentTime; + } if (awaitingResumeOnSeekComplete) { if (this.media.ended) { this.forwardSkip(); diff --git a/packages/core/src/utils/getInitialDuration.js b/packages/core/src/utils/getInitialDuration.js new file mode 100644 index 00000000..df308563 --- /dev/null +++ b/packages/core/src/utils/getInitialDuration.js @@ -0,0 +1,15 @@ +import parseTimeString from './parseTimeString'; + +function getInitialDuration(track) { + let duration = 0; + if (track.duration) { + if (typeof track.duration === 'string') { + duration = parseTimeString(track.duration); + } else { + duration = track.duration; + } + } + return duration; +} + +export default getInitialDuration;