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

Use startingTime from playlist tracks & onTimeUpdate prop #369

Closed danielr18 closed 5 years ago

danielr18 commented 5 years ago

Solves #365

danielr18 commented 5 years ago

@benwiley4000 Do you want to help me with the documentation?

benwiley4000 commented 5 years ago

So it seems like this could actually be two different pull requests:

  1. Using canplay to set trackLoading: false
  2. The onTimeUpdate callback prop and the startingTime API

I think 1 is basically ready to merge as soon as my comment about force load is addressed.

I'm not totally sure about 2 yet, needs more more thought.. so maybe we can pull out 1 and merge it now?

Also to answer your earlier question: yes I would always love help with documentation!! ☺️

danielr18 commented 5 years ago

Makes sense, let me get 1 prepared.

benwiley4000 commented 5 years ago

Ok honestly.. I've spent some time thinking through alternatives again and I think the solution you have implemented makes the most sense. This is an advanced feature and anyone using it will probably be dynamically updating the playlist anyway, so having to update the playlist contents with an updated startingTime should be no big deal.

danielr18 commented 5 years ago

Is there any file to document startingTime? If not, we only have to add the onTimeUpdate info, and it would be ready to merge.

benwiley4000 commented 5 years ago

Everything about the playlist is documented currently here (might be a bit out of date?). I'd stick the info there. I need to integrate that somehow into the docs website eventually. If you have any ideas, go for it!

danielr18 commented 5 years ago

Yeah, I saw that one, but it doesn't have anything related to playlist tracks structure/fields

benwiley4000 commented 5 years ago

Sorry you're right. I got confused.

My plan was to eventually document some of the more complicated PropTypes for PlayerContextProvider, maybe in its markdown file? The info would be similar to what's currently in the README.md but updated

danielr18 commented 5 years ago

Maybe we can work on it in another PR, to document all the playlisy track fields.

On Sat, Feb 16, 2019, 9:24 PM Ben Wiley <notifications@github.com wrote:

Sorry you're right. I got confused.

My plan was to eventually document some of the more complicated PropTypes for PlayerContextProvider, maybe in its markdown file? The info would be similar to what's currently in the README.md but updated for version 2

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/benwiley4000/cassette/pull/369#issuecomment-464402482, or mute the thread https://github.com/notifications/unsubscribe-auth/AIoKWL5aq0YIWemEjjy1JN2NhFx93eg_ks5vOKFbgaJpZM4a-yht .

benwiley4000 commented 5 years ago

That sounds good to me!

benwiley4000 commented 5 years ago

And to leave comments for all the PropTypes so they show up in the docs

benwiley4000 commented 5 years ago

Thanks!

benwiley4000 commented 5 years ago

Released in v2.0.0-alpha.27 (has breaking changes, see the release notes)

benwiley4000 commented 5 years ago

BTW, if you think onActiveTrackUpdate should be passed info on the previous track and the currentTime to make things easier.. I'd be open to having that in a separate PR.

I ended up doing this in the latest release. Not there are some small breaking changes in the API. https://github.com/benwiley4000/cassette/releases/tag/v2.0.0-alpha.32