search

benwiley4000 / cassette

📼 A flexible media player component library for React that requires no up-front config
https://benwiley4000.github.io/cassette/styleguide
MIT License
185 stars 28 forks source link

Missing seekbackward and seekforward in PlayerProviderContext #305

Closed danielr18 closed 5 years ago

danielr18 commented 5 years ago

I'm sure this could be achieved with a combination of onSeekPreview + timeout onSeekComplete (due to having to wait for state to change), but I think it would be easier to have functions for seekbackward and seekforward in the context, similar to the MediaSession ones.

seekbackward: (seekLength) => (this.media.currentTime -= seekLength); seekforward: (seekLength) => (this.media.currentTime += seekLength);

benwiley4000 commented 5 years ago

Right, this makes sense. The onSeekPreview + onSeekComplete weren't really designed to do this.

However I wonder if the best thing might be to allow onSeekComplete to take a parameter (the seek time) so you can use it without onSeekPreview to skip immediately. If onSeekComplete receives a time, then it ignores the preview time in the state. You could do:

seekForward() {
  const { onSeekComplete, currentTime } = playerContext;
  onSeekComplete(currentTime + 5);
}
seekBackward() {
  const { onSeekComplete, currentTime } = playerContext;
  onSeekComplete(currentTime - 5);
}

Seems pretty straightforward and more flexible than adding extra API for that one use case. What do you think?

benwiley4000 commented 5 years ago

I made some typos the first time I wrote that comment so make sure to re-read. :) sorry.

danielr18 commented 5 years ago

I like it, that's how I tried to use onSeekComplete in the beginning until I went to the source code and saw how it worked.

benwiley4000 commented 5 years ago

@danielr18 that sounds like a good sign that this is how it should work then! 😄

If you want to make a PR and update some documentation, that would be great - otherwise I can try to get to it later.

danielr18 commented 5 years ago

I can make the PR 👍

benwiley4000 commented 5 years ago

Hey @danielr18 any update on this? :)

danielr18 commented 5 years ago

I have a weird bug that isn't really related to seek, but the ProgressBar component. image

Somehow when I click ~2 times over the seek buttons, red squares in the image, the player seeks to the position above of where I clicked. If you want, you can try to recreate it here: http://54.224.96.152:3001/, perhaps it's a bug we can solve in the library?

benwiley4000 commented 5 years ago

@danielr18 that's a really peculiar bug! I'm able to repro it as well when I visit your site. Without access to the source code it might be hard for me to debug it. Is it possible to either share the code, or if not, share a minimal repro that demonstrates the bug so I can mess around with it locally?

danielr18 commented 5 years ago

I'll try to create a minimal repro, and share it

benwiley4000 commented 5 years ago

Thanks so much! And once you do, if you could log this as a separate issue that would be much appreciated.

danielr18 commented 5 years ago

Yes, planning to do that, we can close this issue once the PR gets merged.

benwiley4000 commented 5 years ago

Closed by #309.