Skip to content

Commit

Permalink
Merge pull request #392 from benwiley4000/fix-time-jumping-issues
Browse files Browse the repository at this point in the history
Fixes to help avoid progressbar jumping around during track load.
  • Loading branch information
benwiley4000 authored Feb 27, 2019
2 parents bc0b478 + 4a1fab9 commit ecfa17a
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 23 deletions.
56 changes: 33 additions & 23 deletions packages/core/src/PlayerContextProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 => ({
Expand Down Expand Up @@ -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);
}
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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();
Expand Down
15 changes: 15 additions & 0 deletions packages/core/src/utils/getInitialDuration.js
Original file line number Diff line number Diff line change
@@ -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;

0 comments on commit ecfa17a

Please sign in to comment.