Fix stacking variant order when variants inside a group are treated as equal #14431
Conversation
philipp-spiess
changed the title
philipp-spiess
force-pushed
the
fix/variant-order-for-variants-in-group
branch
from
0288b79 to
e67a792
Compare
There was a problem hiding this comment.
These were actually unordered before. If no custom order is provided, we now follow back to ordering the argument if everything else is the same, This will make baked appear before yellow now.
| @media (width >= 640px) { | ||
| .sm\\:flex { | ||
| display: flex; | ||
| } | ||
| } | ||
| @media (width >= 640px) { | ||
| @media (width < 1024px) { | ||
| .min-sm\\:max-lg\\:flex { | ||
| display: flex; | ||
| } | ||
| } | ||
| } | ||
| @media (width >= 768px) { | ||
| .md\\:flex { | ||
| display: flex; | ||
| } | ||
| } | ||
| @media (width >= 768px) { | ||
| @media (width < 1024px) { | ||
| .min-md\\:max-lg\\:flex { | ||
| display: flex; | ||
| } | ||
| } | ||
| }" |
There was a problem hiding this comment.
This now has the non-stacked rules (sm:flex and md:flex) before the stacked ones, regardless of sm or md being used.
| @container name (width < 1024px) { | ||
| .\\@max-lg\\/name\\:flex { | ||
| display: flex; | ||
| } | ||
| } | ||
| @container name (width < 1024px) { | ||
| .\\@max-lg\\/name\\:flex { | ||
| @container (width < 1024px) { | ||
| .\\@max-lg\\:flex { | ||
| display: flex; | ||
| } |
There was a problem hiding this comment.
Right now the @max container queries are using the compareBreakpoints() helper which will return 0 for the same numeric value, effectively ignoring the value of the modifier. I guess this makes this "undefined behavior". The new behavior now matches what we do for group- compounds variants though (where we first show the ones having a name followed by those that won't), so this seems like a reasonable change.
Previously, both the named and non-named variant would be getting a separate variant order but now they follow through and are ordered based on the raw candidate name I think.
| let order = this.compare(a.variant, z.variant) | ||
| if (order === 0) { | ||
| if (a.modifier && z.modifier) { | ||
| return a.modifier.value < z.modifier.value ? -1 : 1 | ||
| } else if (a.modifier) { | ||
| return 1 | ||
| } else if (z.modifier) { | ||
| return -1 | ||
| } | ||
| } |
There was a problem hiding this comment.
This now explicitly handles named modifiers for compound variants, so named groups will appear before their equivalent unnamed group.
philipp-spiess
force-pushed
the
fix/variant-order-for-variants-in-group
branch
from
c8363e6 to
993c895
Compare
…nk equal entries the same way
Co-authored-by: Adam Wathan <[email protected]>
philipp-spiess
force-pushed
the
fix/variant-order-for-variants-in-group
branch
from
a2a9d87 to
e784d21
Compare
philipp-spiess
force-pushed
the
fix/variant-order-for-variants-in-group
branch
from
e784d21 to
9700a1d
Compare
| getVariantOrder() { | ||
| let variants = Array.from(parsedVariants.values()) | ||
| variants.sort((a, z) => this.variants.compare(a, z)) | ||
|
|
||
| let order = new Map<Variant, number>() | ||
| let prevVariant: Variant | undefined = undefined | ||
| let index: number = 0 | ||
|
|
||
| for (let variant of variants) { | ||
| if (variant === null) { | ||
| continue | ||
| } | ||
| // This variant is not the same order as the previous one | ||
| // so it goes into a new group | ||
| if (prevVariant !== undefined && this.variants.compare(prevVariant, variant) !== 0) { | ||
| index++ | ||
| } | ||
|
|
||
| order.set(variant, index) | ||
| prevVariant = variant | ||
| } | ||
|
|
||
| return order |
There was a problem hiding this comment.
@thecrypticace came up with this nugget: We can use aMap<Variant, number> which helps us:
- Avoid creating n sets
- Avoid doing a linear search in the compile function for constant lookups
packages/tailwindcss/src/compile.ts
Outdated
| for (let variant of candidate.variants) { | ||
| variantOrder |= 1n << BigInt(variants.indexOf(variant)) | ||
| let order = variantOrderMap.get(variant) | ||
| if (order === undefined) continue |
There was a problem hiding this comment.
We can use ! instead of the if check because the variants inside candidate.variants are guaranteed to be present in getVariantOrder().
This PR fixes an issue with the order of CSS when using stacked variants when two variants have the same order (as defined by the custom comperator function).
The problem
Take, for example, our breakpoint variants. Those are split into
max-*variants and a group containing allmin-*variants as well as the unprefixed static ones (e.g.lg,sm).We currently define a custom sort order for all breakpoints variants that will compare their order based on the resolved value provided. So if you define
--breakpoint-sm: 100pxand--breakpoint-lg: 200px, we first check if both breakpoints have the same unit and then we rank based on the numerical value, makingsmappear beforelg.But since the
min-*variant and thesmvariant share the same group, this also means thatmin-smandsmas well asmin-lgandlgwill always have the same order (which makes sense—they also have the exact same CSS they generate!)The issue now arises when you use these together with variant stacking. So, say you want to stack the two variants
max-lg:min-sm. We always want stacked variants to appear after their non-stacked individual parts (since they are more specific). To do this right now, we generate a bitfield based on the variant order. If you have four variants like this:max-lgmax-smmin-smmin-lgWe will assign one bit for each used variant starting from the lowest bit, so for the stack
max-lg:min-smwe will set the bitfield to0101and those for the individual variants would result in0100(formin-sm) and0001(formax-lg). We then convert this bitfield to a number and order based on that number. This ensures that the stack always sorts higher.The issue now arises from the fact that the variant order also include the unprefixed variants for a breakpoint. So in our case of
lgandsm, the full list would look like this:max-lgmax-smmin-smsmmin-lglgThis logic now breaks when you start to compute a stack for something like
max-lg:min-lg_while also using thelgutility:max-lg:min-lg010001lg100000As you can see here, the sole
lgvariant will now sort higher than the compound ofmax-lg:min-lg. That's not something we want!Proposed solution
To fix this, we need to encode the information of same variant order somehow. A single array like the example above is not sufficient for this, since it will remove the information of the similar sort order. Instead, we now computed a list of nested arrays for the order lookup that will combine variants of similar values (while keeping the order the same). So from the 6 item array above, we now have the following nested array:
max-lg]max-sm]min-sm,sm]min-lg,lg]When we use the first layer index for the bitfield, we can now see how this solves the issue:
max-lg:min-lg1001lg1000That's pretty-much it! There are a few other changes in this PR that mostly handles with a small regression by this change where now, named
groupvariants and unnamedgroupvariants would now have the same order (something that was undefined behavior before).