-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 usage of useSetting('color.palette') #37108
Conversation
a3608e7
to
d008d0b
Compare
Size Change: +26 B (0%) Total Size: 1.11 MB
ℹ️ View Unchanged
|
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.
The changes here look good to me and fixes a regression of colors not being applied by slug for all palettes.
Thanks for fixing this one 🙇 I've got a PR going that addresses the (latest) PHP unit test fails. If it's not merged by an admin before, would you mind sending it on its way with this PR? (I can't merge due to CI errors). Thank you! |
d008d0b
to
8c52346
Compare
Thanks for following up with this fix! I've pushed another commit 5af4d2e (hope you don't mind @youknowriad!) that adds back in fallback empty arrays when using the spread syntax as the mobile unit tests had some failures that appear to be related to that change. |
* Fix usage of useSetting('color.palette') * Add fallback empty array to prevent spread syntax error with undefined values Co-authored-by: Andrew Serong <14988353+andrewserong@users.noreply.github.com>
Two recent PRs changing how we consume color palette settings conflicted and resulted in broken unit test (and broken behavior as well)
useSetting( 'color.palette' )
to useSetting( 'color') in order to consume all subways (theme, default, custom): this is not great since useSetting( 'color' ) doesn't handle backward compatibility properly and shouldn't be used (ideally should be deprecated in favor of leaf keys. Fix: apply by slug on all origins #36841useSetting('color.palette')
is different thanuseSetting('color').palette
resulting in some breakage. Gradient: Fix clearing a custom gradient from throwing a React warning #37051The unstable php tests (from core) didn't help identify that the test failures were actually valid.
This one tries to reconcile both.