react-native-web icon indicating copy to clipboard operation
react-native-web copied to clipboard

[v0.18 regression] Shadow rules in different StyleSheet objects no longer combine

Open jamesisaac opened this issue 3 years ago • 4 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Describe the issue

I noticed this with a shadow on a Pressable, where I wanted to just tweak the shadowOpacity on hover, but leave all the other style rules (shadowColor, shadowRadius, etc) unchanged. Trying to just change shadowOpacity seems to reset all other rules to their default values.

Expected behavior

The following code works as you would expect on [email protected] - just the shadow opacity changes on hover, but it remains a large red box-shadow. This seems consistent with how React Native StyleSheets are intended to flatten.

<Pressable style={({ hovered }) => [styles.btn, hovered && styles.btnHover]} />

// ...

const styles = StyleSheet.create({
  btn: {
    width: 100,
    height: 100,
    backgroundColor: "#ccf",
    shadowColor: "red",
    shadowOpacity: 0.5,
    shadowRadius: 64
  },
  btnHover: {
    shadowOpacity: 0.9
  }
});

Steps to reproduce

Attempt to run the above code after upgrading to [email protected], and the shadow becomes a small grey box-shadow on hover.

Test case

https://codesandbox.io/s/confident-morning-ird6wz

Additional comments

This can be worked around by re-declaring all the shadow rules in each style object, but obviously this can lead to a lot of repetition / visual glitches due to forgetting to re-declare something, so the old behaviour was preferable.

jamesisaac avatar Jul 28 '22 19:07 jamesisaac

This is because of the change to styling for React 18. All the style objects are now resolved to CSS at create time, rather than during render, which means shadows can't be partially adjusted anymore.

What you could do for web is use the Platform API to set boxShadow instead, and use CSS Custom Properties to change the color / opacity component of the shadow. Just as you would in a normal React DOM app.

I'll likely close this issue as "wontfix" for the following reasons:

  1. No simple fix given what I mentioned about optimization to how styles are produced.
  2. React Native shadow props are iOS-only, have numerous bugs/differences to how CSS shadows work (e.g., they can be inhereted by text content if there is no background color set), and lack many features of CSS boxShadow.
  3. Using boxShadow on web is going to be the recommended way going forward, and support for iOS shadow style props will likely be removed in React Native for Web 0.20, due to the issues mentioned above.
  4. I'm working on a proposal for adding CSS boxShadow and Custom Properties to React Native, which would eventually replace the buggy iOS shadow style props and introduce proper shadow controls for Android.

necolas avatar Jul 29 '22 18:07 necolas

I thought something like that might be the case. Thanks for the detailed explanation, makes perfect sense.

Nice workaround with boxShadow & CSS Custom Properties, thanks, will likely use that.

Your proposal sounds excellent, would be great to have customisable shadows for Android, and consistency between the platforms. Looking forward to seeing more on that.

jamesisaac avatar Jul 29 '22 19:07 jamesisaac

@necolas I tested out that workaround in the Code Sandbox where it worked exactly as needed. To confirm we're on same page, here's the updated working example: https://codesandbox.io/s/optimistic-bash-1fcg45

However, when integrating this into an actual project, I've realised that the idea of bringing CSS Custom Properties into React Native, could potentially be at odds with static analysis (TypeScript, Flow, etc). Wondering if this is something you've considered / have any plan for in your proposal?

Unless there's a different way you're thinking of defining the Custom Properties, or there's some more advanced capabilities in TS/Flow than I'm aware of, I think having arbitrarily named keys like...

StyleSheet.create({
  button: {
    '--x': 10,
    borderRadius: 'var(--x)',
  }
})

...will either leave you in a situation where the object can't be typechecked at all (so potential for typos/wrong value types on borderRadius), or the --x key has to be awkwardly defined separately and spread back into the object with the any-type escape hatch.

While the convergence with the web sounds great, I'm wondering if the departure from simpler JS objects/variables, to something that's more of a DSL built on top of objects, might come with quite a productivity trade-off, if it's reducing the amount tools like TS/Flow can understand RN StyleSheet rules?

jamesisaac avatar Jul 31 '22 15:07 jamesisaac

To confirm we're on same page

Yes, in practice something more like this: https://codesandbox.io/s/dazzling-rain-hic40q

might come with quite a productivity trade-off, if it's reducing the amount tools like TS/Flow can understand RN StyleSheet rules?

TypeScript-referenced guidelines for working with CSS: https://github.com/frenic/csstype#what-should-i-do-when-i-get-type-errors

necolas avatar Aug 01 '22 20:08 necolas

It might be nice to get a console warning about this because I got some very unexpected behavior from this change I have something that ends up with a style like this:

const style = [
 {
        boxShadow: "rgba(0, 0, 0, 0.1) 0px 5px 20px",
      },
      {
      
        shadowColor: "#000000FF",
      }
      ]
      <View style={style}>

And unexpectedly this completely removes the shadow from the view, even if its never changed. Its fine if we are moving towards redoing this, but the only way to figure out why that happened was to find this issue.

ToyboxZach avatar May 16 '23 17:05 ToyboxZach