-
Notifications
You must be signed in to change notification settings - Fork 201
chore: harden recently updated scripts #3680
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
base: main
Are you sure you want to change the base?
Conversation
|
6805649
to
f4571b3
Compare
File metricsSummaryTotal size: 2.25 MB* 🎉 No changes detected in any packages * Size is the sum of all main files for packages in the library.* An ASCII character in UTF-8 is 8 bits or 1 byte. |
🚀 Deployed on https://pr-3680--spectrum-css.netlify.app |
@@ -137,6 +137,8 @@ jobs: | |||
styles_modified_files: ${{ needs.changed_files.outputs.styles_modified_files }} | |||
eslint_added_files: ${{ needs.changed_files.outputs.eslint_added_files }} | |||
eslint_modified_files: ${{ needs.changed_files.outputs.eslint_modified_files }} | |||
mdlint_added_files: ${{ needs.changed_files.outputs.mdlint_added_files }} |
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.
These exports don't seem to be working so we're linking just all the files!
@@ -29,16 +29,31 @@ body { | |||
} | |||
|
|||
.spectrum { | |||
/* Gradient that changes with the color theme. */ | |||
--spectrum-examples-gradient: linear-gradient(45deg, var(--spectrum-magenta-1500), var(--spectrum-blue-1500)); |
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.
These variables are specific to the Storybook so I think it makes sense to move their definitions here instead of inside the tokens package. I didn't remove them though in case downstream customers are leveraging them as well.
color: var(--spectrum-neutral-content-color-default); | ||
background-color: var(--spectrum-background-base-color); | ||
-webkit-tap-highlight-color: rgba(0, 0, 0, 0%); | ||
-webkit-tap-highlight-color: rgb(0, 0, 0, 0%); |
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.
This was just an auto-fix ala VSCode stylelinting
# Legacy tools folder (included storybook & generator) | ||
# test -d "tools" && rm -rf tools | ||
|
||
test -d "tools" && rm -rf tools |
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.
Not sure why this is still here - we like and need the tools folder!!
@@ -9,7 +9,7 @@ export const format = ({ dictionary, platform, file, options }) => { | |||
); | |||
|
|||
const convertRef = (ref) => { | |||
return ref.replace(/\{(.*?)\}/g, `var(--${prefix}-$1)`); | |||
return ref?.toString()?.replace(/\{(.*?)\}/g, `var(--${prefix}-$1)`); |
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.
Sometimes ref is a number so this converts it to a string so we can tidy it up first
@@ -23,11 +23,11 @@ export const format = ({ dictionary, platform, file, options }) => { | |||
if (data.ref) { | |||
data.ref = convertRef(data.ref); | |||
|
|||
if (data.ref === data.value) { | |||
if (data.ref?.toString() === data.value?.toString()) { |
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.
This just ensures that the ref is converted to the same type before comparing (sometimes they're numbers).
Description
Hind-sight is 20:20! I found a couple super minor updates to the scripts I recently worked on for the style-dictionary and Windows compatibility effort. I added notes inline to the code to help clarify these updates. It should just ensure we get more predictable results.
How and where has this been tested?
Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.
Validation steps
yarn builder tokens
Regression testing
Validate:
To-do list