alexanderwallin / react-player-controls

⏯ Dumb and (re)useful React components for media players.
http://alexanderwallin.github.io/react-player-controls/
ISC License
191 stars 35 forks source link

Let ProgressBar support more use cases #18

Closed pakerfeldt closed 7 years ago

pakerfeldt commented 7 years ago

I'm completely new to React so bear with me. AFAICT, ProgressBar only have one callback, namely onSeek. However, I don't want to do seeking until seek knob is released but I can't find any means of getting that feedback? I tried to add onClick and onMouseUp to the ProgressBar but apparently that does not work.

Also, to move the seek knob during seeking I need to update currentTime -- however, that field is also updated by events from my media player which effectively makes the progress position jump around. Ideally I suppose the progress meter and the knob should be separated so that progress meter can still be updated with current progress (from media player) and seek knob moved freely until released. When released, seeking would happen and progress meter is updated to match the knob.

Are there any ways around my issues? How is it supposed to be used?

alexanderwallin commented 7 years ago

Well, welcome aboard! 😊

If I understand it correctly, you want to display the current playback time when not seeking, but show the seeking time while dragging the seek handle?

I think it would definitely make sense to have an onSeekEnd prop on <ProgressBar />, and will see to it as quickly as I can.

alexanderwallin commented 7 years ago

To answer your question: no, I can't think of a non-hacky way to accomplish what you want to do.

pakerfeldt commented 7 years ago

Why I'm asking is because maybe I'm just using the component in a way it wasn't designed for.

This is how I would expect a player (and the player controls) to work:

And, an even neater way would be, like said, to separate knob from progress meter. So I can move the knob independently from the current progress. I.e. knob is not (while seeking) tied to the end of the current progress. When I move the knob, the progress meter is still progressing according to currentTime (feedback from media player). Maybe that involves a lot more changes though.

For know, I managed to solve my problems with a mousedown/mouseup listener and disable feeding of currentTime from media player to ProgressBar while user is performing a seek. However, it felt very hackish.

Don't get me wrong though! This library is great and I'm definitely using it for the little side project I'm working on.

alexanderwallin commented 7 years ago

I agree with your expectations. I checked the controls of Spotify, YouTube, Vimeo and SoundCloud, and they all behave this way. That makes me reluctant to implement an option to detach the handle and progress bar while seeking. Do you have an example of this in the wild?

I thinks it's reasonable for you to manage currentTime. With an onSeekEnd callback, you can use the time passed to onSeek up until onSeekEnd is called.

Thanks! I'm glad it's being put to use. :)

pakerfeldt commented 7 years ago

Yeah I know. It's probably best to just go with what seems to be default behavior. At least users won't be confused.

onSeekEnd would be very much appreciated, however it does not remove my need for the hacky mousedown/mouseup events. For that, ProgressBar needs to manage it's own current position while seeking and not relying on currentTime, right?

alexanderwallin commented 7 years ago

Nope, no need for hacking. You could just keep track of whether the user is seeking and use either the time coming from the media playback (the currentTime prop) or the seeking time (the seekTime state value).

Something like:

class Player extends React.Component {
  static propTypes = {
    totalTime: React.PropTypes.number.isRequired,
    currentTime: React.PropTypes.number.isRequired,
  }

  constructor() {
    this.handleSeek = this.handleSeek.bind(this)
    this.handleSeekEnd = this.handleSeekEnd.bind(this)

    this.state = {
      isSeeking: false,
      seekTime: 0,
    }
  }

  handleSeek(time) {
    this.setState({
      isSeeking: true,
      seekTime: time,
    })
  }

  handleSeekEnd(time) {
    this.setState({
      isSeeking: false,
      seekTime: time,
    })

    // Update the media source
  }

  render() {
    const { totalTime, currentTime } = this.props
    const { isSeeking, seekTime } = this.state

    return (
      <div>
        <ProgressBar
          totalTime={totalTime}
          currentTime={isSeeking ? seekTime : currentTime}
          onSeek={this.handleSeek}
          onSeekEnd={this.handleSeekEnd}
        />
      </div>
    )
  }
}
pakerfeldt commented 7 years ago

Well, yes of course! You're right.

alexanderwallin commented 7 years ago

0.5.10 is out, so give it a go @pakerfeldt!

pakerfeldt commented 7 years ago

Works beautifully! Thanks!