-
Notifications
You must be signed in to change notification settings - Fork 4.6k
@wordpress/theme: update colorjs.io to version 0.6.0
#74278
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: trunk
Are you sure you want to change the base?
Conversation
|
Size Change: +1.11 kB (+0.04%) Total Size: 2.6 MB
ℹ️ View Unchanged
|
d1d206a to
8c27650
Compare
8c27650 to
28f7cd8
Compare
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 double-checked (with some help from AI too) that the line removals about packages/babel-preset-default/node_modules/** are correct, as npm is deduping some unnecessary dependencies — this is because the root-level @babel/* packages (version 7.25.9) should now satisfy all the requirements, thus making the nested 7.25.7 versions unnecessary.
b4a5437 to
38a4e6c
Compare
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
| const sStops = [ 100, 80, 60, 40, 20, 0 ]; | ||
| const hstops = [ 0, 60, 120, 180, 240, 300 ]; | ||
|
|
||
| describe.skip( 'buildRamps', () => { |
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.
Let's see if, by using colorjs.io's internal rounding, we can get the tests to pass in CI and therefore close #73726
|
Reviewing the release notes around "More control over serialization", does this mean we could remove the
...plus a few more in color ramps tests. |
aduth
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.
LGTM 👍
| // With 3 decimal places rounding to the nearest 0.001, the maximum rounding | ||
| // error is 0.0005. With 256 possible hex values, 0.0005 × 256 = 0.128, | ||
| // guaranteeing the rounded value stays within 0.5 of the original value. | ||
| const ROUNDING_PRECISION = 3; |
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.
Nit: I saw some value in having the "hex" qualifier that we're dropping with these changes, as the file could foreseeably include some other rounding to disambiguate, and also that a rounding precision of 3 is not just some arbitrary value we want to apply to all rounding, but is specifically focused around this need for lossless hex serialization.
|
When @jsnajdr originally proposed the version update, I also considered touching-up his other comment about redundant parses as part of the upgrade. Worth including? |
38a4e6c to
41ed638
Compare
Stacked on top of #74281
What?
Update the
colorjs.iodependency in the@wordpress/themepackage to the latest released version0.6.0Why?
To benefit from the latest fixes and improvements https://github.com/color-js/color.js/releases/tag/v0.6.0
How?
[email protected]to match the CI environmentpackage.jsonfile for the@wordpress/themepackagenpm installto updatepackage-lock.jsonnullinstead ofNaNfor "none" valuesNumberobjectsTesting Instructions