-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Button block: add color support via block.json #29382
Conversation
"reusable": false, | ||
"__experimentalSelector": ".wp-block-button > a" | ||
"__experimentalSelector": ".wp-block-button__link" |
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.
We added the block selector in this PR #25768 and it's used to generate preset classes of this block, if any. I've reduced its specificity to the minimum. cc @youknowriad for thoughts about what selector to use.
Note that if we wanted to use the existing selector we'd need to fix #29381 first.
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 don't have objections here, the simpler the selector the better it seems.
Size Change: -819 B (0%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
37ce296
to
f6f628e
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.
This looks good. Love all the removals :) more red please
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.
Nice! LGTM 👍
Actually, looking at the mobile tests I've noticed we can't remove |
f6f628e
to
d7e04d8
Compare
I reached out to @chipsnyder and @geriux for feedback on this. |
Hey 👋 I'm going to test this now on mobile and see if it's all working correctly. |
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.
All good from the mobile side ✅ Thanks for reaching out to give it a quick test!
Part of #28913
Depends on #29378
This PR removes the ad-hoc color support for the block and uses recently added features to the block support mechanism #28913 that allows us to target elements other than the block wrapper.
How has this been tested?
This PR depends on #29378 so, you'd need to apply those changes to your local branch. For example, check out 29378 locally and then
git merge --squash <local-name-of-29378-branch>
.The expected result is that the button without user styles has the colors declared via theme.json and the other button have the user style colors.