Skip to content

Shorthand style properties overwriting the more specific properties #2007

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

Closed
chrisgreen1993 opened this issue May 1, 2021 · 2 comments
Closed
Labels
has: pr Subject of a pull request

Comments

@chrisgreen1993
Copy link
Contributor

First i'd like to say I really appreciate this project, thanks everyone ❤️

The problem

Less specific shorthand style properties are being applied instead of the more specific styles if there is another element also with one of the more specific styles.

This is due to the common styles being pulled out as class names for de-duplication.

How to reproduce

Simplified test case: https://codesandbox.io/s/dazzling-gould-ihiig

Steps to reproduce:

  1. Open the link
  2. The second View has horizontal paddings of 8px instead of 40px

Note that moving the first View below the second causes the issue to go away.

Expected behavior

I expect the horizontal padding to be 40px as this matches the behaviour in React Native.

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

  • React Native for Web (version): 0.16.1
  • React (version): 17.0.1
  • Browser: Chrome

This appears to be an issue in previous versions as well

Additional context

I did some digging and this appears to be caused by the way that styles are de-duplicated. The first View get a class name applied with a padding-left & padding-right of 40px.

The second View then applies the same class and removes the horizontal padding in order to de-duplicate the styles. However this just leaves the padding: 8px in the style prop which then takes precedence over our de-duplicated padding class name. See here:

Screenshot 2021-05-01 at 15 36 15

I believe this is caused by _resolveStyle here

I'm also willing to help out with a PR if someone can give me some pointers on the expected behaviour for how these styles should be applied.

Thanks 🙏

@necolas necolas added this to the 0.17 milestone May 3, 2021
@necolas
Copy link
Owner

necolas commented May 3, 2021

Thanks, this is a good catch! I can handle the fix but if you could submit a PR with a failing test case, that would be great.

@necolas necolas modified the milestones: 0.17, 0.17.x Jun 14, 2021
owenjeon pushed a commit to zigbang/react-native-web that referenced this issue Sep 4, 2021
@necolas necolas removed this from the 0.17.x milestone Sep 28, 2021
necolas pushed a commit that referenced this issue Sep 29, 2021
Less specific shorthand style properties are being applied instead of the more
specific styles if there is another element also with one of the more specific
styles. This is due to the common styles being pulled out as class names for
de-duplication.

Ref #2007
Close #2010
@necolas
Copy link
Owner

necolas commented Jan 31, 2022

See #2208 for a build that should fix this

@necolas necolas added this to the 0.18: StyleSheet milestone Mar 1, 2022
necolas added a commit that referenced this issue Mar 2, 2022
* Improves React Native compatibility by making StyleSheet.create the identify function.
* Improves React 18 support by inserting styles on eval.
* Supports use with multiple windows (i.e., iframes) and shadow roots.
* Supports nested LTR/RTL layouts.
* Supports 3rd party compilers and extraction to static CSS.
* Fixes static and dynamic short/longform deduplication.
* Reduces browser support: Safari 10.1+, Chromium Edge, no IE, no legacy Android browsers.
* Removes automatic vendor-prefixing of inline styles (for better perf).
* Removes focus-visible polyfill as modern browsers no longer show focus rings for mouse interactions.

Close #2208
Fix #2138
Fix #2196
Fix #2007
Fix #1517
@necolas necolas added the has: pr Subject of a pull request label Mar 2, 2022
rnike pushed a commit to VeryBuy/react-native-web that referenced this issue Sep 13, 2022
Less specific shorthand style properties are being applied instead of the more
specific styles if there is another element also with one of the more specific
styles. This is due to the common styles being pulled out as class names for
de-duplication.

Ref necolas#2007
Close necolas#2010
rnike pushed a commit to VeryBuy/react-native-web that referenced this issue Sep 13, 2022
* Improves React Native compatibility by making StyleSheet.create the identify function.
* Improves React 18 support by inserting styles on eval.
* Supports use with multiple windows (i.e., iframes) and shadow roots.
* Supports nested LTR/RTL layouts.
* Supports 3rd party compilers and extraction to static CSS.
* Fixes static and dynamic short/longform deduplication.
* Reduces browser support: Safari 10.1+, Chromium Edge, no IE, no legacy Android browsers.
* Removes automatic vendor-prefixing of inline styles (for better perf).
* Removes focus-visible polyfill as modern browsers no longer show focus rings for mouse interactions.

Close necolas#2208
Fix necolas#2138
Fix necolas#2196
Fix necolas#2007
Fix necolas#1517
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has: pr Subject of a pull request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants