feat: spacing token classes #99
Conversation
✅ Deploy Preview for storybook-ui-seeds ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
I think this will be a big help! 👏🏻
I'd love to group chat a bit about what kinds of spacings we want to support. I might suggest we only support logical margins and paddings, and not left/top/right/bottom.
I think we might want gap utilities as well.
I also wonder about things like mbl and if we could try m-bl or even margin-bl…I know it's more verbose but it's also more readable.
I really like https://nordhealth.design/css/#spacing-utilities when it comes to prior art, and that might be a good discussion starting point.
|
@jaredcwhite That all sounds great - in between this PR and the ticket refining I've been doing, I did come across tickets where we need to support RTL for Arabic, and that might require changing every spacing style we have to the logical versions, like margin-inline-start vs margin-left - which we'd have to do retroactively. I'll go ahead and update the styles here! |
|
That all looks good to me! Any opinion on gap? (that could also just be a later PR too) |
|
@jaredcwhite I would be down for gap! If it's all good, a separate PR is my preference, or as the need comes up in core? Hoping to pull these into the new listing design PR this week :) |
|
🎉 This PR is included in version 1.19.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Issue Overview
This PR addresses #100
Description
In the process of uptaking seeds in core, we're having to add classes to module files just to add a single style like a bottom margin. These are the only kinds of styles we're experiencing this with. These new classes use both our semantic and global tokens for seeds spacings.
I don't think we generally want to follow this pattern to avoid where we can the inline messiness we experienced with Tailwind, but I think for now just spacings would be really helpful.
How Can This Be Tested/Reviewed?
I have self-assigned a followup ticket to add this into Zeroheight documentation after merge: #101
Checklist: