Allow overriding style on web

[M-i-k-e-l]

Summary

Allow overriding style on web

Motivation

When overriding the style with {} we can allow the (web) video to resize to its real size without further effort.

Changes

Sending style will now affect video on web.

Test plan

I assume that if someone is already sending style this can affect them, however the release of the web support was quite recent so I'm not sure how risky this is; not sure how to test it though...

[moskalakamil]

I'm just wondering- if you have a react-native app you have one

[M-i-k-e-l]

I'm just wondering- if you have a react-native app you have one

If you're referring to the testing, then I've tested it; I assumed the the Test plan is for unit test of some sort

[moskalakamil]

Sorry, I'm not sure why my comment was cut off. What I meant was: if you have a Video component in your app that works for both web and native, do the types work correctly? Do they change, for example, when you use Platform.select? Also, what is the default type in that case?

[M-i-k-e-l]

Sorry, I'm not sure why my comment was cut off. What I meant was: if you have a Video component in your app that works for both web and native, do the types work correctly? Do they change, for example, when you use Platform.select? Also, what is the default type in that case?

If this is about the typing change, then I did not test that, I needed to send an empty style {}...
I made the change to the type because of the error in this project, not because of a usage issue - perhaps the correct thing was to ignore it.

[freeboub]

why don't you patch directly ReactVideoProps instead of creating WebStyleProp in this common file ?

[freeboub]

something like:

type CustomViewProps = Omit<ViewProps, 'style'> & {
  style?: ViewProps['style'] | CSSProperties;
};

export interface ReactVideoProps extends ReactVideoEvents, CustomViewProps {

[freeboub]

• ping @zoriya

[zoriya]

I purposely omitted the style props because this is not compatible with react native styles. this means you'll need a different style for web & native. Since types are different, Platform.select is bothersome to use & styling libs would just error.
Existing projects using the style property for native would probably have a type error due to the new style typing.

Maybe we could add a webStyle instead to prevent those issues.

[M-i-k-e-l]

Another option is to add a prop that will have hug (style={{}}) and fill (style={videoStyle}) as values.

[freeboub]

just a ping on this PR, I think we definitively need to integrate such change !

[M-i-k-e-l]

Hi @freeboub,
Not sure if the ping is for me :)
I would love to get the prop name selected before changing the PR.
Technically resizeMode can be used since it's not used in web ATM (so maybe contain (for hug - style={{}}) and stretch (for fill - style={videoStyle}).
If you don't like this option, then maybe you'll like one of these layoutMode or wrapStyle?

[freeboub]

@M-i-k-e-l Yes it was for you :)

I think you just need to check the patch in Video.ts
please change:

export interface ReactVideoProps extends ReactVideoEvents, ViewProps {

to export

interface ReactVideoProps extends ReactVideoEvents, Omit<ViewProps, 'style'> {
   style?: CSSProperties | Pick<ViewProps, 'style'>

I am not fan of having some Web dedicated interface. I think we should keep the api common (even if here we don't really have the choice)

Would that work for you ?

[zoriya]

Wouldn't it make more sense to use https://necolas.github.io/react-native-web/docs/unstable-apis/#use-with-existing-react-dom-components to use the style prop from react-native with the web element?

[freeboub]

@zoriya I am not confident in using unstable apis ... I am not a web dev, what do we gain by using this api ?

If it is only to support in 1 line: accessibilityLabel, accessible, style, I would prefer to handle it prop per prop, it gives a thiner control on the api and allow easier integration when we will enable them on android/ios

[zoriya]

This unstable api allows using react native's style prop on the web. Without using it, a lot of style are different. This also means that all native styling libraries (rnw's default, native-wind, tamagui, unistyle...) won't work on the web (and give type errors on native).

This api is marked as unstable but it hasn't changed in a lot of years, I think it's safe to use there honestly.

[freeboub]

OK, @zoriya it looks good to me, but not sure on how to integrate it :D @M-i-k-e-l

[zoriya]

Should be as simple as adding

import { unstable_createElement } from 'react-native-web';
const Video = (props) => unstable_createElement('video', props);

at the top of the file and changing the component from <video> to <Video>. (might need to type the props in the declaration above, idk)