Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inline style flex properties are ignored by React #798

Closed
kristerkari opened this issue Feb 7, 2018 · 16 comments
Closed

Inline style flex properties are ignored by React #798

kristerkari opened this issue Feb 7, 2018 · 16 comments

Comments

@kristerkari
Copy link
Contributor

kristerkari commented Feb 7, 2018

Do you want to request a feature or report a bug?

a bug.

What is the current behavior?

If the current behavior is a bug, please provide the steps to reproduce

Currently setting any flex-box props with style property <View style={{ flexGrow: 0, flexBasis: '33.333%', }} /> will result in !important to be added to the properties, which means that the properties get ignored by React.

Setting flex props with style prop works just fine in React Native.

My use case is that I want to do a responsive layout where flex-basis gets changed based on the viewport width. That is why I need to use the style property.

What is the expected behavior?

react-native-web should match the behavior in React native and support setting flex props using inline styles.

Environment (include versions). Did this work in previous versions?

  • OS: OS X
  • Device:
  • Browser: Chrome
  • React Native for Web (version): 0.4.0
  • React (version): 16.2.0
@kristerkari
Copy link
Contributor Author

I can obviously also set the flex props using a div:

<div style={{ flexGrow: 0, flexBasis: '33.333%', }} />

... but I want to use <View> so that it also works with React Native.

@necolas
Copy link
Owner

necolas commented Feb 7, 2018

This is an issue in react-dom facebook/react#1881

@kristerkari
Copy link
Contributor Author

That React issue has been open since 2014, so I'm not expecting a quick fix. Is there any way of avoiding using !important when setting inline styles?

@necolas
Copy link
Owner

necolas commented Feb 7, 2018

Yes, I can see how long that issue has been open and I'm currently looking into this.

@necolas
Copy link
Owner

necolas commented Feb 8, 2018

Upstream PR facebook/react#12181

@kristerkari
Copy link
Contributor Author

kristerkari commented Feb 8, 2018

Thanks for having a look at this! It would be nice to see it fixed in React :)

necolas added a commit that referenced this issue Feb 12, 2018
Updates the 'setValueForStyle' implementation to support style values
that contain "!important". This allows the 'flex{Basis,Grow,Shrink}'
values created by the style resolver to be applied. They currently use
the important priority as a work-around for browser-inconsistencies in
the 'flex' shorthand.

Upstream fix: facebook/react#12181

Ref #798
necolas added a commit that referenced this issue Feb 14, 2018
Updates the 'setValueForStyle' implementation to support style values
that contain "!important". This allows the 'flex{Basis,Grow,Shrink}'
values created by the style resolver to be applied. They currently use
the important priority as a work-around for browser-inconsistencies in
the 'flex' shorthand.

Upstream fix: facebook/react#12181

Ref #798
necolas added a commit that referenced this issue Feb 14, 2018
Updates the 'setValueForStyle' implementation to support style values
that contain "!important". This allows the 'flex{Basis,Grow,Shrink}'
values created by the style resolver to be applied. They currently use
the important priority as a work-around for browser-inconsistencies in
the 'flex' shorthand.

Upstream fix: facebook/react#12181

Ref #798
Close #813
@necolas necolas changed the title Inline style flex-box properties get ignored by the browser Inline style flex properties are ignored by React Feb 19, 2018
@necolas
Copy link
Owner

necolas commented Mar 22, 2018

Looks like React won't accept a bugfix for this until React 17. Avoid setting inline flex styles in the meantime.

@kristerkari
Copy link
Contributor Author

Yeah I can kind of understand that they don't want to take the risk of something breaking because some existing code relying on React to ignore !important.

Avoid setting inline flex styles in the meantime.

Unfortunately I can't really do that if I want to create a responsive layout with flexbox for both web and native.

As a workaround I have created an extra wrapping component that uses <View> in React Native and <div> + inline-style-prefixer for web. It seems to work, but it's ugly and a confusing thing to have in a project. I'm also not sure if I'm breaking compatibility with some browser by doing that.

@necolas
Copy link
Owner

necolas commented Apr 2, 2018

Unfortunately I can't really do that if I want to create a responsive layout with flexbox for both web and native.

Do you really need to dynamically calculate a new value for flexBasis every render? Or do you have known values that it can be?

@kristerkari
Copy link
Contributor Author

kristerkari commented Apr 2, 2018

I have a few breakpoints, e.g. flexBasis: "50%", "flexBasis: "33.333333%", "flexBasis: "25%". So, yeah the values are known.

@necolas
Copy link
Owner

necolas commented Apr 2, 2018

Ok that makes it easy for you to define the styles ahead of time (which also improves render perf) and avoid this issue.

@kristerkari
Copy link
Contributor Author

Sorry, I don't quite get what you mean. Is there a way for me to avoid setting inline styles (e.g. using StyleSheet)?

I already have the styles defined for each breakpoint, and then when the window size hits a breakpoint, I use style prop to set the styles.

@necolas
Copy link
Owner

necolas commented Apr 3, 2018

You don't need inline styles. Conditionally apply the flex style as needed

const styles = StyleSheet.create({
  flexBasisSmall: { flexBasis: '25%' },
  flexBasisBig: { flexBasis: '50%' },
})

<View style={[
  small && styles.flexBasisSmall,
  big && styles.flexBasisBig,
]} />

There's more about how to work with styles in the docs

@kristerkari
Copy link
Contributor Author

I was also thinking that it would look something like that. The reason why I chose to use inline styles is that the code is a bit easier to read that way.

In this case it makes sense to use StyleSheet instead to avoid the issue with !important. Thanks for the suggestion!

@kristerkari
Copy link
Contributor Author

@necolas what about adding a mention to the FAQ to not use flex props with inline styles, but to use StyleSheet instead?

https://github.com/necolas/react-native-web/blob/master/website/guides/style.md#faqs

@necolas
Copy link
Owner

necolas commented Apr 30, 2018

This will be fixed in the next release.

There are also warnings thrown when !important is included in user-land styles: StyleSheetValidation. This already ensures the information is communicated to developers without needing to patch React DOM's warnings.

And setNativeProps will support !important as an escape hatch for any rare cases where that might be required: setValueForStyles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants