-
Notifications
You must be signed in to change notification settings - Fork 101
feat(height, width): update to SHINE sizing #2122
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
Conversation
🦋 Changeset detectedLatest commit: 344be1f The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for stacks-svelte ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for stacks ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
WOAH! So nice! The docs are looking good with the new split.
(I didn't want to make it a blocker for this PR but since you mentioned looking at ai-* pages I noticed we have a custom orange badging all over the flex pages for marking something as default. Can we swap that custom code to use .s-badge__info (the blue informational badge component)?
ttaylor-stack
left a comment
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.
Thank you for doing these changes. height and width pages look good, unchanged values look the same and the new values look correct. Approved
giamir
left a comment
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.
Fantastic work @dancormier. I loved how you took the time to test out the resulting output of the less mixins. It made the review so much easier and honestly the snapshots act as great documentation for whoever will have to touch these utilities going forward. ❤️
packages/stacks-classic/lib/atomic/__snapshots__/sizing.less.test.ts.snap
Show resolved
Hide resolved
|
I'm getting a weird issue where a single visual regression test image is failing in Github but not locally. I'm going to merge to get this in asap and will investigate the failing image issue. |
SPARK-154
This PR updates our set of "sizing units" to the new batch detailed in SPARK-154. I also took this opportunity to clean up our sizing/spacing in general to make it easier to maintain.
Note
All the same classes are available. Only their values have changed. None have been removed unless they were undocumented and accidentally included previously.
How to test
Tip
Use your inspector's
computedtab to see computed pixel valuesScreenshot example of inspector
.ai-*and.ac-*).hs*,.hmn*,.hmx*).ws*,.wmn*,.wmx*)Notable changes
.ws*and.hs*values are updated to the new sizeswidth-height.htmlintowidth.htmlandheight.htmlTests
Spacing class generation mixins
.generate-spacingmixin to.generate-sizing.generate-sizingtopackages/stacks-classic/lib/exports/mixins.lesspackages/stacks-classic/lib/exports/spacing-mixins.less.generate-su-sizingmixin for generating classes based on sizing units.--s-fulland--s-stepwith their sizing unit counterparts (and retained them for legacy compatibility)calc(var(--s-step) * 10)becomes--su1024Negative sizing unit custom properties
--sun*calcs of negative values with--sun*custom propertiescalc(var(--su1) * -1)) becomesvar(--sun1).lessfiles changedUnder the hood
#build-classesLess functions into a single onelib/exports/constants-helpers.lessandlib/atomic/spacing.lesstolib/atomic/sizing.lesslib/atomic/spacing.lesstolib/atomic/positioning.lesswidth-height.lessintowidth.lessandheight.lessOther changes only indirectly related to sizing
font-size: var(--fs-body1);is now only present onbodyand nothtml. This will allow the client's preferred font size to scale the UI.