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
feat(website): show text color token pairings #3034
Conversation
π¦ Changeset detectedLatest commit: ca7f45e The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 paste-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
β Deploy Preview for paste-theme-designer ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Passing run #5502 βοΈ
Details:
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit ca7f45e:
|
Size Change: -105 B (0%) Total Size: 897 kB
βΉοΈ View Unchanged
|
...e-website/src/components/site-wrapper/sidebar/sidebar-disclosure/SidebarDisclosureButton.tsx
Outdated
Show resolved
Hide resolved
6362461
to
6b66773
Compare
3e33e80
to
ac7b454
Compare
0f5279c
to
554f298
Compare
color-text-decorative-40: | ||
value: "{!palette-purple-60}" | ||
comment: Text color with no semantic meaning, used for decorative purposes only. Should generally be used with matching decorative background and/or border tokens. | ||
text_contrast_pairing: | ||
- color-background | ||
- color-background-body | ||
- color-background-decorative-40 | ||
- color-background-decorative-40-weakest |
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.
incorrect annotations here. These tokens don't exist.
@@ -1,8 +1,8 @@ | |||
imports: | |||
- ../../../aliases/box-shadow.yml | |||
- ../../../aliases/offset.yml |
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.
offset file was empty, might as well inherit the base
@@ -155,31 +155,15 @@ props: | |||
color-text-decorative-10: | |||
value: "{!palette-gray-20}" | |||
comment: Text color with no semantic meaning, used for decorative purposes only. Should generally be used with matching decorative background and/or border tokens. | |||
text_contrast_pairing: |
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.
No need to declare the pairings at the theme level. The pairings are set at the system level, so every theme should meet the same token pair requirements as they are used in the same places across the theme.
- ./color-palette.yml | ||
- ./color.yml | ||
- ./offset.yml |
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.
copy pasta from the other theme that didn't need the empty file
@@ -3,7 +3,6 @@ global: | |||
category: box-shadow | |||
imports: | |||
- ../../../global/box-shadow.yml | |||
- ../aliases/box-shadow.yml |
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.
redundant file
@@ -49,10 +49,6 @@ props: | |||
color-text-decorative-30: | |||
value: "{!palette-green-80}" | |||
comment: Text color with no semantic meaning, used for decorative purposes only. Should generally be used with matching decorative background and/or border tokens. | |||
text_contrast_pairing: |
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.
More copy pasta
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.
I'm seeing a strange issue where opening the popover near the bottom of the screen overflows the viewport and not the page's scroll area, but that's not a blocker for this. A ticket to investigate would be appreciated, however
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.
there you goooooo
@@ -91,6 +91,7 @@ const Template: StoryFn<typeof TokenCard> = ({name, category, useCamelCase}) => | |||
textColorInverse={textColorInverse} | |||
borderColor={borderColor} | |||
useCamelCase={useCamelCase} | |||
text_contrast_pairing={text_contrast_pairing} |
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.
Question: why is this prop snake case? Could it be like the others?
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.
It totally could, essentially all this stuff is typed from the Tokens Object itself, and that's how it's named on the object, which is unfortunate. it's just lazy typing to be able to pass the object around without too much hassle and map the props to the object keys
Description and useful links to discussions / issues / tickets
Changes in this PR:
Change One
Details of change...
Change Two
Details of change...
Checklist
required
checks have passedπ΅π»ββοΈ Run website visual regression
label