Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
gracesilge
left a comment
gracesilge
left a comment
There was a problem hiding this comment.
Looks great overall!
Can you add a comment at the top of tokens.ts explaining why you chose to represent the pixel count as 4x what is listed (e.g. 8px -> gap-2).
Can you add a line to your Pull Request description about why you moved away from one Spacing token? Can you also change the plain padding type to be a Padding token instead of a Spacing token to match the rest?
|
And please go through and resolve the merge conflicts! Let me know if you haven't done that before and need assistance! |
|
The merge errors should have been fixed. I ended up messing up a little bit and had to fiddle with git so if there's any issues let me know! I accepted all the incoming changes and had to fix some file accordingly. Updated the original comment with more info about the switch to multiple spacing tokens. Also the comment and the plain padding should also be fixed now. |
This branch was successfully deployed
2 participants
Updated to sm, md, ..., 2XL labels to be pixel sizings ranging from 8 to 32px. So changed how they are defined in tokens.ts then changed where the labels were referenced in other files as well as tests.
Moved away from a single Spacing token since padding and gap values have different purposes and need their own token types -> Splitting into PaddingX, PaddingY, Gap, and ColumnGap improves type specificity and makes it clearer for future developers.