Do not pass flex to RN views (new flex shadow prop) #2527
Conversation
M-i-k-e-l
requested review from
ethanshar,
adids1221,
Inbal-Tish and
lidord-wix
| @@ -66,7 +66,7 @@ function asBaseComponent<PROPS, STATICS = {}>(WrappedComponent: React.ComponentT | |||
| const {forwardedRef, ...others} = themeProps; | |||
| return ( | |||
| (this.state.error && UIComponent.defaultProps?.renderError) || ( | |||
| <WrappedComponent {...others} modifiers={modifiers} ref={forwardedRef}/> | |||
| <WrappedComponent {...others} modifiers={modifiers} ref={forwardedRef} flex={undefined}/> | |||
There was a problem hiding this comment.
If someone wraps not a RN component with flex modifier the layout will break. What about passing the flex modifier's value to the 'flex' prop or undefined if there is none?
There was a problem hiding this comment.
We could do something like the following:
// @ts-ignore
const flex = typeof themeProps.flex === 'boolean' ? themeProps.flex ? 1 : undefined : themeProps.flex;
return (
(this.state.error && UIComponent.defaultProps?.renderError) || (
<WrappedComponent {...others} modifiers={modifiers} ref={forwardedRef} flex={flex}/>
)
);
@ethanshar @Inbal-Tish @lidord-wix @adids1221
Please vote with thumbs \ reply with your opinion.
There was a problem hiding this comment.
modifier flex can be a value other than 1
There was a problem hiding this comment.
Yes, this should be fixed.
Do you generally like this better?
I think both solutions have a risk to them.
There was a problem hiding this comment.
We could block all of our flex and flex={false} (only flex and not flex-2 etc) and send undefined for false and flex-1 instead of flex.
I think we should talk about this in the dev sync before we merge.
|
Opened #2536 instead |
Description
Do not pass flex to RN views (new flex shadow prop)
This currently breaks at least one screen (
NumberInputScreen).Changelog