feat: Add merging array of variants

[atanasster]

issues #1209 #1208

allows to specify array of variants, merged right to left

<Box variant={['boxes.beep', 'boxes.bop']} />

[hasparus]

Okay, the code is correct, but I think we have some prior work to unpack.

This is sort of a duplicate of #1017, which was already a duplicate itself.

Sort of a duplicate of #586 and related to issue #832
#1017 (comment)

Also connected to Pull Request #823.

I have a feeling this will be harder to merge, than we might expect looking at the code. I think we should try to understand the previous work first, and discover the concerns which blocked its finalization.

[lachlanjc]

Do you want to add in the documentation I wrote on #586? Feel free to edit it of course but that could be a starting point.

[atanasster]

thanks @lachlanjc - I added your documentation now. Sorry I didnt see your PR when submitting this one, but we are getting there :)

[lachlanjc]

Oh you’re good! I made that awhile ago, before TS & all, so we needed a new implementation.

[lachlanjc]

Closes #174
Closes #704
Closes #740
Closes #1017
Closes #1208

[hasparus]

shouldn't we also support

sx={{
  variant: ['a', 'b']
}}

?

[hasparus]

We could move this logic up to css package.

[atanasster]

shouldn't we also support

sx={{
  variant: ['a', 'b']
}}

?

This is the responsive variants syntax, right?

[hasparus]

This is the responsive variants syntax, right?

Well, now it is.

from V1 roadmap #832

• Variant composition, e.g. variant: ['large', 'primary'] (or plural variants). This would potentially be a breaking change, meaning that the variant property itself would no longer support responsive arrays, but the values within the variant could. Add support for combining multiple variants #586, Add variant composition feature #1017

sx={{
  variant: ['a', 'b'] // new combined variant
  variant: [['a', 'b']] // new responsive tuple variant
}}

[atanasster]

I think we should create a new issue with that feature.

[hasparus]

Wait... can variant inside sx be a function?

[hasparus]

@atanasster I've invaded your branch to add a test again. We need to talk about it. Why does variant: (theme) => pickRandomVariant(theme) work? It's not documented :D

[hasparus]

also added possibility to use variant as a function both in sx and in variant props and added to variant guide docs.

I feel we now have duplicate code for this in sx.variant execution path.
What my test showed is that it already worked, because all functions were fed with theme.

const key = typeof variant === 'function' ? variant(theme) : variant

[joernroeder]

Just want to check on this PR, as lost track of the discussion. Is this finally going to add variant={[['primary', 'medium'], ['primary', 'large']]}?

[a-y-u-s-h]

@atanasster Any progress on this issue? This feature will be a blessing.

@joernroeder I think this PR is supposed to close #1208 (using variant as an array inside theme file, to compose bigger variants, not sure if it handles variant as an array in sx prop as well).

[atanasster]

@joernroeder - no, this PR does not handle responsive arrays of arrays of variants (thats a mouthful ). Can expand on the "finally" part - was this discussed previously?

@a-y-u-s-h I think we are waiting on a "stable" 0.6.0 release before merging other PRs.

[joernroeder]

@atanasster @a-y-u-s-h maybe that's just me, but when I see arrays in theme-ui I immediately "think in breakpoints". As far as I know this change is the only place where that pattern breaks and an array means something else – merging in this case – which might be confusing. There were several issues and discussions which mentioned responsive variants before and several people worked on it, "finally" was my enthusiasm that this might be now landing and I can reorg and clean my theme soon :)

[a-y-u-s-h]

@atanasster @a-y-u-s-h maybe that's just me, but when I see arrays in theme-ui I immediately "think in breakpoints". As far as I know this change is the only place where that pattern breaks and an array means something else – merging in this case – which might be confusing. There were several issues and discussions which mentioned responsive variants before and several people worked on it, "finally" was my enthusiasm that this might be now landing and I can reorg and clean my theme soon :)

Good point. I think this needs more discussion (I'm not sure whether conclusions have already been made about it) Both interpretations of array in variants: merge, and treating it as breakpoint differences are actually kind of good valid feature ideas now that I think about it. There can be multiple solutions for this (at the top of my head, these are the ones I can think about right now):

1. If variant key works the way it's described it Extending styles of multiple variants to form another variant #1208 then it becomes an exception like you mentioned, a distinction can be made by thinking that breakpoints are valid only for primitive CSS properties and not for variants.

2. Though I like the first solution, there could be ways to have both -

If variant key accepts an array, and if it allows breakpoint variants - this also makes sense, and to handle merging variants - another variants (or even more descriptive: composition or compose) key could be introduced which makes it clear that it merges the variant array passed to it and apply the formed variant to the variant we're describing, that may give us best of both worlds. Maybe composition or variants are not the best names for it but the idea remains same, names can be debated later.

If this means breakpoint variants:

box:
  variant: [a, b, c, d]

Then this could mean merging variants:

box:
  extend: [a, b, c, d]

3. Can there be a way to specify responsive variants and merge variants in the same key? Probably.

If this means breakpoint variants:

box:
  variant: [a, b, c, d]

Then this could mean merging variants:

box:
  variant: [[ a, b, c, d ]]

Then this (an array of arrays) could mean breakpoint and merged variants (but this can also be confusing, We'll have to remember that merging stops after innermost array level):

box:
  variant: [[ a, b, c, d ], [e, f], [g, h]]

[atanasster]

@joernroeder - I see what you mean by thinking first in breakpoints, I myself came a bit late to this 'variants arrays' party and this PR is specifically for multiple variants that need to be merged, not for responsive arrays of variants.
Overall, I do agree there is a need for also responsive variants, however the final design of such feature is not yet 100% and also I really dislike lumping together many different features/fixes into a single PR as it gets lost long-term from too much noise.

@a-y-u-s-h - very good points, can you please start a new thread for responsive arrays of variants. I would personally support a notation such as variant: [[ a, b, c, d ], [e, f], [g, h]] - in any case it will be confusing, but having different fields for breakpoints and merging can be very difficult to maintain.

[lachlanjc]

Agreed. Responsive variants would be cool down the road, but I think we should prioritize merging, since there's currently no approachable way in Theme UI's API to accomplish that, whereas you can make variants responsive with media queries/responsive values without too much monkeypatching.

[joernroeder]

thanks @a-y-u-s-h for the write up variant: [[ a, b, c, d ], [e, f], [g, h]] is what I had in mind. I understand the point @lachlanjc is making, just wanted to raise this as it was not clear to me.
thanks @atanasster for the work, I'm happy to join the discussion how the responsive variant API could be designed.

<ThankYou variant={[['blue', 'text-small', 'full-width'], ['blue', 'text-medium'], ['blue', 'text-large']]} />

[hmaracic]

As of v0.3.5, code

{
  somevariant2: {
    fontSize: 20,
  },
  'somevariant2-large': {
    color: 'pink',
    fontSize: 30,
  },
  somevariant3: {
    variant: ['somevariant2', 'somevariant2-large'],
    textDecoration: ['underline', 'overline'],
  },
}

works in a way that it constructs a variant somevariant3 which for smaller viewports inherits somevariant2, while inheriting somevariant2-large for larger viewports.

You can see it in action here https://codesandbox.io/s/theme-ui-starter-forked-gsfjq

Considering this feature, is this going to break in future versions, since the brackets syntax for variants now seems to be used for merging instead?

[balintpeak]

Hi, I was wondering if there are still plans to implement this?
I feel like this is one of the top missing features in the lib.

The use case for me would be to support different variants of a button, ie combining color, size and style.
Eg: colors: primary, secondary; sizes: big, small; style: solid, outline. A combination can be made by picking one item from each group, eg: primary, small, outline. Is there an easy way currently to achieve this?
There can be 8 different legitimate combinations - it is not ideal to manually set these up in the theme file one by one.

Regarding this PR and some conversations beforehand, some people have raised issues with nested arrays - and the changing API.
How about using strings with commas for merging variants? Dot notation is already used in the lib so "stylistically" could fit in.

This would not be a breaking change, given that variant prop is currently expecting a string.
This way, arrays would still exclusively be used for responsive design/ breakpoints as a new feature for the variant prop - backwards compatible. It would also keep to current convention on using arrays.

// button with a merged "primary" and "outline" variants for all screen sizes
<Button variant="primary, outline">click me</Button>


// primary+square on small devices and primary+rounded on bigger ones
<Button variant=['primary, square', 'primary, rounded']>click me</Button>

EDIT: I could actaually live without the responsive array API on the variant prop. The responsive look and feel should be setup in the theme file for the respective variants (ie what a "small" means on a mobile device etc).

[lachlanjc]

@hasparus Thought about this a bit, & I think the pure array implementation is rather confusing since it overlaps with responsive syntax in an invisible way. I think I’d prefer making this a separate variants prop or key name that accepts an array of strings, so it’s very clear what’s going on. This isn’t something we can easily change down the road if we did decide to support array-syntax responsive variants. If we’re instead using variants, that could become an array of an array of strings for the (somewhat wild) use-case of merging multiple variants that each change across breakpoints, without breaking any existing usages. When the variants key is passed, we could either merge in variant or ignore it.

[fcisio]

I'm down with the variants props, but I also liked the idea of keeping that logic in a single string like so 'primary,large'.

At the object level, it would be fine, but I'm thinking about other libs that could also use this prop. For instance, Framer Motion has a variants prop.

[lachlanjc]

I'm down with the variants props, but I also liked the idea of keeping that logic in a single string like so 'primary,large'.

This is great if you're just typing a few, but I can imagine variant merging to be super useful depending on other UI state, & having to do variant={['primary', active && 'active'].filter(Boolean).join(',')} seems rather awkward vs variants={['primary', active && 'active']} (which we can internally filter). Plus, with nested arrays, I think the syntax gets super confusing.

With Framer Motion—not sure how to solve that problem.

[balintpeak]

Agreed, variant={['primary', active && 'active'].filter(Boolean).join(',')} looks weird.
You could do this instead, which maybe looks a bit better

variant={`primary, ${active ? 'active' : ''}, ${colour ? colour : ''}`}

[lachlanjc]

I feel like if we're going to use a string type, using a different operator to make it clearer we're merging might be better: variant="primary + outline + small"? But I still prefer a variants array.

[hasparus]

This isn’t something we can easily change down the road if we did decide to support array-syntax responsive variants.

Yeah, totally agreed. It's also a fair explanation of why this issue is open for 2 years.
Let's go with variants prop with an array. Conditionals inside are a huge advantage for ergonomics and this

variant={`primary + ${isHuge && "huge"}`}

is a total monstrosity compared to the following :D

variants={["primary", isHuge && "huge"]}

[balintpeak]

After thinking a bit more it, I have a new (extended) proposal for the API.

@hasparus : if isHuge is undefined, in variants={["primary", isHuge && "huge"]} you would end up with 'primary + undefined' which is not ideal. One way to fix this is, that the package could strip out certain words like undefined, false and null - although this might be weird and too "magical" just to facilitate an easy way to use conditional logic.

I still like the string approach to adhere to the excellent usage of arrays for breakpoints across the entire package.
Addressing @lachlanjc concerns around developer experience and usability, I think probably the best way would be to borrow the API from the old classnames package.

variant would accept strings as well as objects, like so:
variant="primary + outline + small" (or variant="primary, outline, small") and

variant={{ primary: true, outline, small: isSmall }}

where outline and small would be conditional on outline and isSmall variables.

Now it would be easy to do this

variant={[{ primary: true, outline, small: true }], [{ primary: true, outline }]}

Primary button which might be outlined depending on logic, small on mobile and "normal" sized on other devices

As a side note, a comma separated string seems more natural to me, rather than using the plus sign, but could live with either or.

[lachlanjc]

@balintpeak Really appreciate the detailed proposal! I’m still leaning toward @hasparus’s suggestion because I prefer that it centers on combining variants while elegantly supporting conditionals, whereas the object syntax centers on conditionals & less elegantly supports unconditional combinations. I can also see the object syntax being easy to add down the road.

Closing this PR since we need a new implementation, but still very much want to add this!