-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for setting screens
in JS config
#14415
Conversation
d4de6cf
to
c8889d5
Compare
screens
in JS config
84a2a16
to
affd493
Compare
552af20
to
8bdbee9
Compare
3ec0449
to
fd36a58
Compare
} | ||
|
||
if (coreOrder && insertOrder === undefined) { | ||
insertOrder = allocateOrderAfter(designSystem, coreOrder) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible for us to do this all at one time at the end rather than after every inserted variant?
We're definitely doing more traversals and object replacements than we need to but if it proves to be too complex then this is fine for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have to do it once for every utility that is inserted this way. Each one will handed their own order. I do need a test for this since I think it might now be adding it in inverse insertion order.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it would make sense that this is the reason some things are going being inserted in reverse order. I was thinking of something like this:
- Get a list of all the variants after
min
(and their compare fns) - Insert our appropriate screens into the
min
group - Insert other screens after the
min
group - Move the previously recorded variants past the last variant we just inserted (only if there is one)
I think this would work but I'm not sure if the separate compareFn map complicates that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm I’m not sure I follow, I think we can keep doing what we do right now but only loop over the variants in reverse order, this way we:
- Still insert appropriate screens into the
min
group. For that, the order does not matter since it'll use the compare function - Insert complex variants after the
min
group. But since we loop in reverse order, we will will always move the previously inserted complex variant down.
I pushed a commit that changes this and it fixes the order.
This PR adds support for the simple case of the
screens
option inside JS config paths. This allows JS configs to extend the responsive theme by adding custom breakpoints. Here's an example from our v3 docs:For simple breakpoints, this will extend the core breakpoints and will work with the
min-*
andmax-*
utilities. However, we also support complex ways of setting up custom screens like this:For these complex setups, we only generate the shorthand variant (e.g.
tall
) but those won't integrate withinmin-*
andmax-*
. In v3, adding any of these complex configurations would omit anymin-*
andmax-*
variants.