Responsive values is variant styles override sx styles

[rpavlovs]

Responsive values in variant styles override sx styles.

To Reproduce

// theme.ts

variants: {
  buttons: {
    primary: {
      paddingX: ['24px', '32px'],
      paddingY: ['8px', '12px'],
	}
  }
}

// component.styles.ts

// twitter button is a primary button

twitterButton: {
	variant: 'button.primary',
    px: '5px',
    py: '5px'
}
// final style: => {px: '24px', py: '8px'}
// expected: { px: '5px', py: '5px'}

twitterButton: {
	variant: 'button.primary',
    px: ['5px'],
    py: ['5px'],
}
// final style: => {px: '24px', py: '8px'}
// expected: { px: '5px', py: '5px'}

twitterButton: {
	variant: 'button.primary',
    px: ['5px', '5px', '5px'],
    py: ['5px', '5px', '5px'],
}
// final style: => {px: '5px', py: '5px'}
// as expected 

Expected behavior
sx prop styles should always override variant styles.

[jxnblk]

Thanks! I'm not sure I understand the issue here. Would you mind creating a Code Sandbox to demonstrate what's happening?

[lachlanjc]

Pretty sure I’m having the same issue: My cards.primary defines p: [2, 3]. To set padding to 0, I have to do sx={{ p: [0, 0] }}—just p: 0 only applies on mobile.

[jxnblk]

@lachlanjc if that's the same thing happening in the issue description, then yeah, that's a known issue without a clear solution, but I suspect the example above might be pointing to something else??

[rpavlovs]

Yeah, it’s the same issue. Thanks for a better explanation @lachlanjc

[kensongzhu]

Just start using theme-ui and hit the same issue
I reproduced the codesandbox: https://codesandbox.io/s/trusting-matsumoto-ebnc5

It seems related to the Box. The system props styles of base, variant and sx
are generated seprately and then merged.

In other words, e.g. whatever media queries generated in variant is taken anyway and won't be removed with sx, right?

If I twist the code a bit as following, does it make sense?

const aliases = {
  bg: "backgroundColor",
  m: "margin",
  mt: "marginTop",
  mr: "marginRight",
  mb: "marginBottom",
  ml: "marginLeft",
  mx: "marginX",
  my: "marginY",
  p: "padding",
  pt: "paddingTop",
  pr: "paddingRight",
  pb: "paddingBottom",
  pl: "paddingLeft",
  px: "paddingX",
  py: "paddingY"
};

const keys = Object.keys;
const toString = Object.prototype.toString;
const isObject = arg => toString.call(arg) === "[object Object]";

// replace the aliases
const normalize = arg =>
  !arg
    ? {}
    : keys(arg).reduce((acc, curr) => {
        const key = curr in aliases ? aliases[curr] : curr;
        const value = arg[curr];

        return {
          ...acc,
          // recursive for hover, focus etc.
          [key]: isObject(value) ? normalize(value) : value
        };
      }, {});

const variant = ({ theme, variant, __themeKey = "variants" }) =>
  get(theme, __themeKey + "." + variant, get(theme, variant));

// apply css ONCE
const merged = css({
  // base
  ...normalize(props.__css),
  // variant
  ...normalize(variant(props)),
  // sx
  ...normalize(props.sx)
})(props.theme);

[folz]

Saw this in the v1 roadmap and wanted to +1. Our Card component sets a responsive array on border per our designs, so every place that wants to use card with a different border has had to set ['styles', 'styles', 'styles'], which seems redundant.