Property passing. Merged click handler. Poster management. Children option.

[tomsseisums]

Property passing Basically, with this, one can pass arbitrary props down to <video>, including event handlers.

Merged click handler Also merged onClick handler with default behaviour, though, I wonder if we should provide for a way to go around default altogether?

Poster management

• passing true (which is also the default) will load poster provided by Mux.
• passing <string> should (cause I didn't test specifically, but why wouldn't it, eh?) provide for all the ways to load a video poster.
• finally, passing false will skip poster altogether.

Children option Could provide useful to implement, for instance, custom controls, though, as of writing this, it could have been brought up the tree... matter of preference, I guess. But I'm starting to shift more towards it being an overkill here, the component becomes too knowledgable. But hey, it's a PR for a reason!

Fixes #3.

Sorry for so much changes in one PR, didn't want to juggle around, testing locally is a little painful for now.

[skogsmaskin]

Thank you @joltmode, just one issue as reviewed.

[tomsseisums]

I reworked the prop filtering to use omit utility instead.

Improves code readability and predictabilty by a lot. Also cleans up scope of unused variables.

Now there is an open question, do we use the lodash one (simple way) or write a smaller footprint custom one?

[skogsmaskin]

Hmm.. I'm not sure about the mixing of props here. Could it be a better idea to have a prop videoProps which is exactly that, props for the video element? Then all other props are for the SanityMuxPlayer API and clearly defined via PropTypes.

[tomsseisums]

Thought of that option also.

Yes, could go with it.

I didn't go with it initially, because I kind of disliked the idea that most of the props you can pass to SanityMuxPlayer are actually the ones you'd pass to video (assetDocument being the sole exception?), therefore the extra prop to pass props of same family to the same component doesn't mesh well. Basically, I am not a fan of the public API in videoProps scenario.

On Sat, Dec 8, 2018, 20:15 Per-Kristian Nordnes <notifications@github.com wrote:

Hmm.. I'm not sure about the mixing of props here. Could it be a better idea to have a prop videoProps which is exactly that, props for the video element? Then all other props are for the SanityMuxPlayer and clearly defined via PropTypes.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sanity-io/sanity-mux-player/pull/4#issuecomment-445478818, or mute the thread https://github.com/notifications/unsubscribe-auth/AA-3wHXio7hbC0a3IvVSQeOe0lOpYMxNks5u3AGsgaJpZM4ZJRSJ .

[tomsseisums]

@skogsmaskin so - extra prop videoProps or keep it the current way? Your call, chief! 😄

[skogsmaskin]

@joltmode, I've begun refactoring what first begun as abstracting and move the error handling outside the player. That kind of escalated a bit, and I've begun working on a custom control interface as well that should be easy to style.

The way I see it, there should be two "modes" to use this module. One mode being just plug and play where you get error handling and some easy way to style the player to fit your webpage (drop in video player). The other mode would be to just facilitate the loading of the video (src, poster and any future Sanity-specific stuff). In this mode the developer should be able to take total control of how to present and play the video.

Here is a suggestion for relevant props, to see if they can facilitate this:

Player spesific

• assetDocument - The video meta data
• className - root element className
• style - root element style object
• autoLoad Automatically start to buffer the video.
• showPoster Show the Sanity-saved poster image (if any, and prop videoElement.poster is not set, which would override it).
• onError (by passing this you do the error handling outside the player yourself)
• theme (or similar name, indicating that you want to use the custom player control interface, either with the standard theme or your own style). The plan in to keep this small and simple. If you want it too look totally different, one should build your own which can be achieved with children , player.getVideoElement() and videoProps.
• showControls - toggles the themed control interface on/off (requires theme to be set as well)
• children (extra stuff to render inside the player container)

Video spesific

• videoElementProps
  • src
  • poster
  • loop
  • muted
  • autoPlay
  • controls
  • onClick
  • style
  • className
  • All the rest...

What do you think?

[skogsmaskin]

OK! After discussing on Slack, let's release this for now allowing for some more control in the current API, and aim for a version 0.1.0 soon, which will have a new and much better API.

[skogsmaskin]

Thanks @joltmode! Released as sanity-mux-player@0.0.26