-
Notifications
You must be signed in to change notification settings - Fork 72
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
fix: Changed type for design token print-page-size #3388
fix: Changed type for design token print-page-size #3388
Conversation
Thanks for the pull request, @PKulkoRaccoonGang! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.
|
✅ Deploy Preview for paragon-openedx-v23 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -218,7 +218,7 @@ const COMMANDS = { | |||
sendTrackInfo('openedx.paragon.cli-command.used', { command, status: 'success' }); | |||
} catch (error) { | |||
// eslint-disable-next-line no-console | |||
console.error(chalk.red.bold('An error occurred:', error.message)); | |||
console.error(chalk.red.bold('An error occurred:', error)); |
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.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release-23.x #3388 +/- ##
=============================================
Coverage 93.91% 93.91%
=============================================
Files 232 232
Lines 3943 3943
Branches 901 930 +29
=============================================
Hits 3703 3703
+ Misses 236 232 -4
- Partials 4 8 +4 ☔ View full report in Codecov by Sentry. |
"print-page-size": { | ||
"source": "$print-page-size", | ||
"$value": "a3", | ||
"$type": "text" |
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 agree with the previous use of the dimension
type for the print-page-size design
token. However, I believe this type contradicts the documented description of the dimension type provided by the DTCG.
After reading through #3389
stood out to me. If nothing else we should update The more concerning thing is that it sounds like the minor version bump in |
I chose this option, if in the future we have similar problems we can always upgrade to the latest patch version (~4.3.x) |
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.
After the update, the order of the variables in the files with the CSS has changed and units of measurement for 0 have been added (in sizes where necessary)
@PKulkoRaccoonGang thanks for making the updates! Now that the React 18 support changes from 22.x have made it into 23.x it seems this needs a rebase. If you can rebase this and resolve conflicts I'd love to land it! |
f99960d
to
66eed9f
Compare
Description
This PR provides changes to correct the correct building of the token design.
print-page-size
fromdimension
totext
.More details: #3389
Merge Checklist
example
app?Post-merge Checklist