CSS properties only with sx prop and variant API thoughts

[dburles]

Bringing across a set of comments I had made against this PR.

There is a fair case that it would make sense for the sx prop to only support CSS properties, and that was @jxnblk's original thought too. Especially so if support is added to reference theme values within CSS shorthand.

The other thought that came out of this was then how to handle variants. It would mean that variant could no longer be accessed within the sx prop.
However, perhaps that's not too much of a problem. One thing I've found is that it quickly breaks down once the variant needs to affect multiple elements at once.

So in aiming to define a consistent API across a component library, I opted instead to always provide a variant prop (when needed), that way I can handle both cases. The problem is then a source of confusion around having the two options available. I haven't yet found a nice, (ideally declarative) approach to handling these kinds of variants. What are your thoughts?

In the world of the sx prop only allowing CSS properties, the variant API could be built around the variant prop and also better accomodate the ability to cover multiple elements at once.

Here's an example of a basic idea:

const variants = {
  primary: {
    wrapper: {
      bg: 'primary',
    },
    heading: {
      color: 'white',
    },
    subtitle: {
      color: 'textWhiteMediumEmphasis',
    },
  },
  secondary: {
    wrapper: {
      bg: 'none',
    },
    heading: {
      color: 'text',
    },
    subtitle: {
      color: 'textMediumEmphasis',
    },
  },
};

const CardHeader = ({
  title,
  subtitle,
  variant,
  ...props
}) => {
  return (
    <Flex
      {...props}
      sx={{
        borderTopLeftRadius: 'large',
        borderTopRightRadius: 'large',
        ...variants[variant].wrapper,
      }}
    >
      <Box sx={{ flex: 1 }}>
        <Heading
          as="h6"
          sx={variants[variant].heading}
        >
          {title}
        </Heading>
        {subtitle && (
          <Text
            sx={variants[variant].subtitle}
          >
            {subtitle}
          </Text>
        )}
      </Box>
    </Flex>
  );
};

[lachlanjc]

Forgive me if I’m misunderstanding, I’ve read your message a few times & not sure I’m getting it. Does the behavior you’re looking for here go beyond having array support in the variant prop? The example you’re showing seems like it should work fine with how Theme UI already works.

[dburles]

Hey @lachlanjc this is an early issue on the topic and since then I've expanded on the idea somewhat. See the mentions on this issue, #800 covers the latest thoughts. I moved off theme-ui some time ago in favor of https://github.com/dburles/mystical which contains this API in the form of a useModifiers hook. So happy for you to close out these issues.

[lachlanjc]

No worries, thanks for this! Mystical looks awesome, those are some really elegant evolutions. Let’s close this issue here then.

[dburles]

Thanks! I think let's close this one, but #800 is still open for discussion as part of the 1.0 roadmap. The ideas may or may not fit with theme-hi but either way I think they're definitely worth consideration.

I've been building components with the useModifiers API for close to a year now and it covers pretty much all cases quite elegantly. It does break down a little once you get into more complicated interrelated styles, I am not sure there is a way to handle those cases more elegantly. Instead, I'll opt for simply writing the logic directly in the component (as you might without useModifiers) in combination with the modifiers. I think that is probably the way to go in those situations, but it may be worth continuing to explore.

[lachlanjc]

@hasparus can you close this?