-
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
FocalPointPicker with __next40pxDefaultSize #56021
Conversation
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.
Early green check on this one.
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.
Changes LGTM. Good to be merged after adding a CHANGELOG entry 🚀
Actually, @brookewp (who has been recently working on updating some components to the new |
Ah, so instead of setting |
Yes, exactly. Similarly to how That way, the size can be updated by opting in to use the prop. Otherwise, it'll stay the same since the default value will be There are some recent examples of how this was done with other components here: #55471, #55789, and #53819 - I'm also happy to help if needed! |
Thanks @brookewp, feel free to jump in here if you'd like to. I don't mind! :) |
5281210
to
f2fc8b1
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.
LGTM 🚀
Thank you all for the collaboration!
packages/components/src/focal-point-picker/styles/focal-point-picker-style.ts
Show resolved
Hide resolved
We're good to merge! |
ccf5de6
to
718a033
Compare
Flaky tests detected in 718a033. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7121747313
|
What?
Part of the effort towards #46734.
Testing Instructions
Screenshots or screencast