styled-components / xstyled

A utility-first CSS-in-JS framework built for React. 💅👩‍🎤⚡️
https://xstyled.dev
MIT License
2.28k stars 105 forks source link

Style priority with @xstyled/emotion v3 #272

Closed vincentaudebert closed 3 years ago

vincentaudebert commented 3 years ago

🐛 Bug Report

When composant has a property css set, it looks like any other overwriting with a "string" style property isn't working.

To Reproduce

Have a component like this:

const styles = {
  item: theme => css`
    display: block;
  `,
  itemButton: css`
    border: 0;
  `,
}

DropdownMenu.Item = ({ variant, disabled, onClick, ...props }) => (
  <Box
    as="button"
    {...props}
    onClick={!disabled ? onClick : null}
    css={[
      styles.item,
      styles.itemButton,
      styles[disabled ? 'disabled' : variant],
    ]}
  />
)

Then use it with something like this:

<DropdownMenu.Item
  onClick={() => console.log('click')}
  px={0}
  display="flex"
  width="100%"
>
  Item
</DropdownMenu.Item>

Use case won't be display flex but display block

Expected behavior

It should be display flex instead of display block

Run npx envinfo --system --binaries --npmPackages @xstyled/system,@xstyled/styled-components,styled-components --markdown --clipboard

Paste the results here:

- Node: 16.1.0 - /usr/local/bin/node
 - Yarn: 1.22.10 - /usr/local/bin/yarn
 - npm: 7.8.0 - /Volumes/Projets/shire/node_modules/.bin/npm

Could be linked to this commit: https://github.com/gregberge/xstyled/commit/9ac9a21 ?

agriffis commented 3 years ago

The problem here is that you were depending on a bug :joy:

Specifying css and style props in a single component is effectively undefined. Who says which should win?

Instead you should wrap an inner component so there's a clear hierarchy and priority:

/** @jsx jsx */
import { jsx } from '@xstyled/emotion'

const InnerItem = ({disabled, onClick, variant, children}) => (
  <button css={...} onClick={onClick} type="button">{children}</button>
)

DropdownMenu.Item = props => <Box {...props} as={InnerItem} /> 

and then your original example should work:

<DropdownMenu.Item
  onClick={() => console.log('click')}
  px={0}
  display="flex"
  width="100%"
>
  Item
</DropdownMenu.Item>