New variant API

[dburles]

This is a work in progress and a bit of a proof of concept refactor to replace the variant sx property with the variant prop. See #800. This would be a breaking change, but theme-ui is not v1.0 yet 🔨😄. I've also refactored the Box and Flex components and removed the dependencies on both styled-system and @emotion/styled, which also addresses issues with #817 and #690. We could probably drop the usage of Box from all the other components now as it's not doing all that much. I've also decided to drop the use of the few styled-system props that were previously part of the Box component in favour of using the sx prop instead.

There is also a new API for components with multiple elements: Select, Checkbox and Radio. It's now possible to supply styles to each part. Such as for Select:

const theme = {
  ...
  forms: {
    select: {
      // styles for select element
      select: {
        borderRadius: 5,
      },
      container: {
        // styles for root element
      },
      arrow: {
        // styles for arrow icon go here
      },
    },
  },
}

Todo:

• MDX
• Tests
• Update docs examples
• Update docs theme object
• Update other themes
• Remove variant handling from css function
• Make useVariant hook public? – could be quite useful for custom components.

[jxnblk]

Thanks! There's some interesting ideas here -- I need to add the v1 roadmap issue I've been drafting, but some of this could potentially fit in line with some other parts of that

[jxnblk]

I'd be considering changing the private API a bit here, but does this mean that props.sx is no longer forwarded to the component here? This looks like it'd break in MDX usage

[dburles]

Yes you're right, sorry had overlooked that, I've re-added the __css API.

[jxnblk]

This word/meaning bothers me, sorry...

Suggested change

	const variation = useVariant('badges', variant)
	const variantStyle = useVariant('badges', variant)

[dburles]

all good!

[dburles]

Here's an example my latest thoughts on a unified method to this whole approach:

const modifiers = {
  // Considering adding a special `default` key to automatically apply and allow overwriting sets of default styles:
  default: {
    title: { ... },
    subtitle: { ... },
  },
  variant: {
    primary: {
      title: { ... },
      subtitle: { ... },
    }
  },
  size: {
    small: {
      title: { fontSize: 3 },
      subtitle: { fontSize: 0 },
    },
    large: {
      title: { fontSize: 5 },
      subtitle: { fontSize: 2 },
    },
  },
  // etc
};

const Component = ({ variant: 'primary', size = 'small', modifiers: customModifiers }) => {
  const modifierStyle = useModifiers(
    { variant, size },
    modifiers,
    customModifiers // Optional secondary modifiers object that will merge with `modifiers`.
  );

  return (
    <div>
      <div sx={[{ fontFamily: 'heading' }, modifierStyle.title]}>{title}</div>
      <div sx={modifierStyle.subtitle}>{subtitle}</div>
    </div>
  );
};

Some example usage:

// Normal usage:
<Component size="large" />

// Overwriting some default styles:
<Component modifiers={{ default: { title: { ... }, subtitle: { ... } } }} />

// MyComponent.js

// Custom sizes:
const modifiers = {
  small: {
    title: { fontSize: 4 },
    subtitle: { fontSize: 1  },
  },
  large: {
    title: { fontSize: 6 },
    subtitle: { fontSize: 2 },
  },
};

const MyComponent = (props) => <Component {...props} modifiers={modifiers} />;

General ideas:

Much of these changes have come from ideas presented in #586.
This introduces new API with the useModifiers hook, but also gives a prescribed approach to building library components. The core component library should also use this same API.

• One universal term for prop based variations, I'm using 'modifiers' here, it fits nicely when using the variant term as the main type of modifier, type would work too, but it's overloaded by the HTML type attribute
• The theme does not need to be overloaded with modifiers objects, I'm just defining them alongside the component as it's unlikely they'll be shared with other components, but if they need to be, you can also put them on the theme
• It's possible to easily extend components defined this way by passing in a modifiers prop with the parts that you wish to alter, e.g. <Component modifiers={{ size: { small: { title: fontSize: 2 } } }} />
• The sx prop will need to support array syntax to automatically deep merge styles
• I don't think MDX styles will need a variant prop

I'd really like to get some discussion going to fine tune it. It's been a great API to work with so far in my own experience. I don't know if I am stepping outside of the core ideas behind theme-ui, but this solves a bunch of problems inherent with the current approach.

One more example illustrating supplying custom styles to @theme-ui/components Select (each is optional):

const MySelect = (props) => <Select {...props} modifiers={{ container: { ... }, select: { ... }, arrow: { ... } }}>...</Select>;

[LekoArts]

This is purely a note from a conversation I had in private with @jxnblk for documentation sake:

Coming from #990 I tested out the changes to the Box component as the shouldForwardProp in https://github.com/system-ui/theme-ui/blob/master/packages/components/src/Box.js#L18 isn't passing e.g. activeClassName. Copying the changes here that were made to the Box component fixed said issue.

[jxnblk]

Hey @dburles sorry for the lack of activity/discussion here, but really appreciate the thought and effort that went into this! I think to help move our TS conversion along, we'd like to ditch the use of the styled API, which you've done here. In an effort to try to unblock some of that, I'd like to branch off from where you've started here and get a new branch in shape to go in, but just wanted to check to see if that sounds okay to you. I think we could continue some of the other parts of the discussion here around API changes in other issues or a new PR, if that makes sense

[dburles]

@jxnblk No problem. There is definitely some more discussion needed with these changes.

There are a bunch of scattered conversations on the variant API in general, what are your thoughts on creating a new banner issue to bring the discussion together as it's a little scattered right now. #800 is the main issue for this PR, there's also PR's #294 and #1017.

Just to confirm, would you like me to create a new PR from these changes, but without the variant alterations? Let me know if there's anything I've missed.

I'm not sure where that leaves this PR for the future, but I can move some of my comments from here over to any new discussion on the variant API changes in general.

[jxnblk]

what are your thoughts on creating a new banner issue to bring the discussion together as it's a little scattered right now.

Yeah, a new issue or a few new issues might make sense. I think it should be generally inline with #832 or a proposal for a new API that can be evaluated on its own. e.g. useVariant is an interesting idea, but some of the other ideas don't require that or can be decoupled

Just to confirm, would you like me to create a new PR from these changes, but without the variant alterations?

No, no, no. Sorry if I wasn't clear. I can create a new branch from this PR, descope a couple things and tidy it up for a new PR which would supersede this one, but keeping your commit history -- just wanted to make sure you were cool with that.

I'm not sure where that leaves this PR for the future, but I can move some of my comments from here over to any new discussion on the variant API changes in general.

Starting from from a new issue is fine IMO, linking to prior art or past issues and PRs where needed.

[dburles]

Sounds good to me

[lachlanjc]

Closing because we’re not planning on implementing this, & https://github.com/dburles/mystical is a successor if you’re looking for this functionality :) Thank you so much for the thoughtful PR though—we really appreciate it.