-
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
Add support for button elements to theme.json #40260
Conversation
Size Change: +92 B (0%) Total Size: 1.24 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 change seems simple enough. The main questions for me are:
- do we really want this new element? cc @mtias @jasmussen
- Any impact on the UI in the global styles panel? How does a user define these customizations.
@@ -60,7 +60,8 @@ export default function save( { attributes, className } ) { | |||
|
|||
return ( | |||
<div { ...useBlockProps.save( { className: wrapperClasses } ) }> | |||
<RichText.Content | |||
<BlockButton |
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 is going to require a deprecation for all blocks that uses it in save functions because of the extra classname.
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.
It seems to cope OK without a deprecation - I see this message in the console:
Block successfully updated for `core/button` ({name: 'core/button', icon: {…}, keywords: Array(1), attributes: {…}, providesContext: {…}, …}).
New content generated by `save` function:
<div class="wp-block-button"><a class="wp-block-button__link wp-element-button">button 2</a></div>
Content retrieved from post body:
<div class="wp-block-button"><a class="wp-block-button__link">button 2</a></div>
Is it still better to provide a deprecation?
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 think you got lucky that an existing deprecation handled this for you. If your block included a custom font family, the old deprecation wouldn't have worked because the markup would be unrecognized by the old deprecation as well.
The rule is simple any change to the save function (saved markup) requires a deprecation.
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 tried with a custom font family, but I still didn't get an error. Any clues how I can trigger one?
I have added a deprecation but it doesn't have isEligable
or migrate
properties as I'm not sure what it is I need to be migrating from and to.
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 new deprecation don't need isEligable
or migrate
. The best way to trigger an invalidation (without the newly added deprecation) is to check what changed between v10 (the last deprecation) and the save function of the block. It seemed to me like it was the font family style. It seems v10 triggers if there's a font family assigned, so actually you shouldn't have a font family style, but you should also check older ones. One of the old ones is being triggered for some reason. One other way to test your own deprecation would be to remove all the older ones temporarily.
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 have tested my deprecation and it seems to work correctly, but I do get this message in the console, is that expected?
Block validation: Block validation failed for `core/button` ({name: 'core/button', icon: {…}, keywords: Array(1), attributes: {…}, providesContext: {…}, …}).
Content generated by `save` function:
<div class="wp-block-button is-style-outline"><a class="wp-block-button__link has-text-color has-background wp-element-button" style="background-color:#000000;color:#ffffff">Visit</a></div>
Content retrieved from post body:
<div class="wp-block-button is-style-outline"><a class="wp-block-button__link has-text-color has-background no-border-radius" style="background-color:#000000;color:#ffffff">Visit</a></div>
Thanks for the ping. Irecently have challenges styling the search button using just the Global Styles UI, how will this particular block be reflected there? There may be a larger question here on what is the ideal approach to styling the theme in a UI only-world (i.e. when you don't have access to editing theme.json), perhaps especially inner-blocks. Do we nest those inner-blocks, for example Global Styles → Blocks → Search → Button? Or is there a case for generic form widgets for input and buttons? Ensuring a good UI-first flow, even if in the future, would be my primary question on this approach. |
@jasmussen we talked about using an inner block for the search block button but there were many reasons why it wasn't a good idea (see #39463). The aim of this PR is to provide a generic Button "element" which can be used by all blocks that want buttons. These buttons would be styled using the "elements" section of theme.json for now, but ultimately we'll want to add a new "Elements" section to the Global Style UI like we do for "Blocks". One advantage of a "Button" element is that third party blocks can also use it. |
9094d37
to
0a5d300
Compare
Thanks for the reviews. I have addressed the feedback! |
return newProps; | ||
}; | ||
|
||
export default class ElementButton extends Component { |
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 should be a function component, there's not reason to use a class.
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.
When I use a function component I get this error:
react-dom.js?ver=6.0-beta2-53224-src:61 Warning: Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?
I assume that's because the Button block passes a ref down to this component. Is it worth trying to change that approach 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.
Oh I see, I think the problem is that you're trying to absorb "RichText" in this component while you could do this when you need RichText
<RichText tagName={ ElementButton } />
That said, I see that you're forcing ElementButton to be a button
tag while we at least have two use-cases: an a
for the case of the button block and a button
in the case of the search block. Are these the only two use cases for this element?
If that's the case we can consider having two "components" ElementButtonAsLink
ElementButtonAsButton
or something like that.
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.
Are these the only two use cases for this element?
At present yes, but I expect we'll want to add more for other blocks that want buttons.
This does raises more questions though - does it make sense for the "button" element to sometimes apply to a link tag?
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.
Indeed it's a great question that would benefit from a broader perspective. I think it does personally, we had a lengthy discussion about it in the context of the Button
component of Gutenberg as it's also a component that can render both as a link or a "button" depending on its props.
As long as we stick with these two tags (a, button), I think it's fine.
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.
(That's said about the "ref" error, I think the component should forwardRef anyway :) )
A thought: is this the beginning of the creation of a subset of similar blocks, such as inputs, textareas, etc, and if yes, is that a good thing? (Maybe?) |
A great example of why we need #34180 |
This commit backports the original PRs from Gutenberg repository: * [WordPress/gutenberg#40260 #40260 Add support for button elements to theme.json] * [WordPress/gutenberg#40889 #40889 Theme Json: Don't output double selectors for elements inside blocks] * [WordPress/gutenberg#41140 #41140 Global Styles: Add support for caption elements] * [WordPress/gutenberg#41160 #41160 Global Styles: Load block CSS conditionally] * [WordPress/gutenberg#41240 #41240 Global Styles: Button Element: update button element selector] * [WordPress/gutenberg#41335 #41335 Duotone: Fix CSS Selectors rendered by theme.json duotone/filter settings for blocks on public pages] * [WordPress/gutenberg#41446 #41446 Block styles: Account for style block nodes that have no name] * [WordPress/gutenberg#41696 #41696 Global Styles: Allow references to values in other locations in the tree] * [WordPress/gutenberg#41753 #41753 Elements: Add an API make it easier to get class names] * [WordPress/gutenberg#41786 #41786 Support pseudo selectors on elements in theme json] * [WordPress/gutenberg#41822 #41822 Elements: Button - Fix element selectors] * [WordPress/gutenberg#41981 #41981 Global Styles: Add support for heading elements] * [WordPress/gutenberg#42072 #42072 Fix link element hover bleeding into button element default styles] * [WordPress/gutenberg#42096 #42096 Add visited to link element allowed pseudo selector list] * [WordPress/gutenberg#42669 #42669 Link elements: Add a :where selector to the :not to lower specificity] * [WordPress/gutenberg#42776 #42776 Theme JSON: Add a static $blocks_metadata data definition to the Gutenberg instance of WP_Theme_JSON] * [WordPress/gutenberg#43088 #43088 Pseudo elements supports on button elements] * [WordPress/gutenberg#43167 #43167 Theme_JSON: Use existing append_to_selector for pseudo elements] * [WordPress/gutenberg#43988 #43988 Styles API: Fixed selectors for nested elements] Props onemaggie, bernhard-reiter, cbravobernal, mmaattiiaass, scruffian, andraganescu, dpcalhoun, get_dave, Mamaduka, SergeyBiryukov. See #56467. git-svn-id: https://develop.svn.wordpress.org/trunk@54118 602fd350-edb4-49c9-b593-d223f7449a82
This commit backports the original PRs from Gutenberg repository: * [WordPress/gutenberg#40260 #40260 Add support for button elements to theme.json] * [WordPress/gutenberg#40889 #40889 Theme Json: Don't output double selectors for elements inside blocks] * [WordPress/gutenberg#41140 #41140 Global Styles: Add support for caption elements] * [WordPress/gutenberg#41160 #41160 Global Styles: Load block CSS conditionally] * [WordPress/gutenberg#41240 #41240 Global Styles: Button Element: update button element selector] * [WordPress/gutenberg#41335 #41335 Duotone: Fix CSS Selectors rendered by theme.json duotone/filter settings for blocks on public pages] * [WordPress/gutenberg#41446 #41446 Block styles: Account for style block nodes that have no name] * [WordPress/gutenberg#41696 #41696 Global Styles: Allow references to values in other locations in the tree] * [WordPress/gutenberg#41753 #41753 Elements: Add an API make it easier to get class names] * [WordPress/gutenberg#41786 #41786 Support pseudo selectors on elements in theme json] * [WordPress/gutenberg#41822 #41822 Elements: Button - Fix element selectors] * [WordPress/gutenberg#41981 #41981 Global Styles: Add support for heading elements] * [WordPress/gutenberg#42072 #42072 Fix link element hover bleeding into button element default styles] * [WordPress/gutenberg#42096 #42096 Add visited to link element allowed pseudo selector list] * [WordPress/gutenberg#42669 #42669 Link elements: Add a :where selector to the :not to lower specificity] * [WordPress/gutenberg#42776 #42776 Theme JSON: Add a static $blocks_metadata data definition to the Gutenberg instance of WP_Theme_JSON] * [WordPress/gutenberg#43088 #43088 Pseudo elements supports on button elements] * [WordPress/gutenberg#43167 #43167 Theme_JSON: Use existing append_to_selector for pseudo elements] * [WordPress/gutenberg#43988 #43988 Styles API: Fixed selectors for nested elements] Props onemaggie, bernhard-reiter, cbravobernal, mmaattiiaass, scruffian, andraganescu, dpcalhoun, get_dave, Mamaduka, SergeyBiryukov. See #56467. Built from https://develop.svn.wordpress.org/trunk@54118 git-svn-id: http://core.svn.wordpress.org/trunk@53677 1a063a9b-81f0-0310-95a4-ce76da25c4cd
This commit backports the original PRs from Gutenberg repository: * [WordPress/gutenberg#40260 #40260 Add support for button elements to theme.json] * [WordPress/gutenberg#40889 #40889 Theme Json: Don't output double selectors for elements inside blocks] * [WordPress/gutenberg#41140 #41140 Global Styles: Add support for caption elements] * [WordPress/gutenberg#41160 #41160 Global Styles: Load block CSS conditionally] * [WordPress/gutenberg#41240 #41240 Global Styles: Button Element: update button element selector] * [WordPress/gutenberg#41335 #41335 Duotone: Fix CSS Selectors rendered by theme.json duotone/filter settings for blocks on public pages] * [WordPress/gutenberg#41446 #41446 Block styles: Account for style block nodes that have no name] * [WordPress/gutenberg#41696 #41696 Global Styles: Allow references to values in other locations in the tree] * [WordPress/gutenberg#41753 #41753 Elements: Add an API make it easier to get class names] * [WordPress/gutenberg#41786 #41786 Support pseudo selectors on elements in theme json] * [WordPress/gutenberg#41822 #41822 Elements: Button - Fix element selectors] * [WordPress/gutenberg#41981 #41981 Global Styles: Add support for heading elements] * [WordPress/gutenberg#42072 #42072 Fix link element hover bleeding into button element default styles] * [WordPress/gutenberg#42096 #42096 Add visited to link element allowed pseudo selector list] * [WordPress/gutenberg#42669 #42669 Link elements: Add a :where selector to the :not to lower specificity] * [WordPress/gutenberg#42776 #42776 Theme JSON: Add a static $blocks_metadata data definition to the Gutenberg instance of WP_Theme_JSON] * [WordPress/gutenberg#43088 #43088 Pseudo elements supports on button elements] * [WordPress/gutenberg#43167 #43167 Theme_JSON: Use existing append_to_selector for pseudo elements] * [WordPress/gutenberg#43988 #43988 Styles API: Fixed selectors for nested elements] Props onemaggie, bernhard-reiter, cbravobernal, mmaattiiaass, scruffian, andraganescu, dpcalhoun, get_dave, Mamaduka, SergeyBiryukov. See #56467. Built from https://develop.svn.wordpress.org/trunk@54118 git-svn-id: https://core.svn.wordpress.org/trunk@53677 1a063a9b-81f0-0310-95a4-ce76da25c4cd
This PR migrates the Style Engine PHP functions, classes and tests into Core for 6.1. It backports the original [WordPress/gutenberg#40260 PR #40260] from Gutenberg repository. Props ramonopoly, bernhard-reiter, costdev, azaozz, andrewserong, mukesh27, aristath. See #56467. Built from https://develop.svn.wordpress.org/trunk@54156 git-svn-id: http://core.svn.wordpress.org/trunk@53715 1a063a9b-81f0-0310-95a4-ce76da25c4cd
This PR migrates the Style Engine PHP functions, classes and tests into Core for 6.1. It backports the original [WordPress/gutenberg#40260 PR #40260] from Gutenberg repository. Props ramonopoly, bernhard-reiter, costdev, azaozz, andrewserong, mukesh27, aristath. See #56467. Built from https://develop.svn.wordpress.org/trunk@54156 git-svn-id: https://core.svn.wordpress.org/trunk@53715 1a063a9b-81f0-0310-95a4-ce76da25c4cd
This commit backports the original PRs from Gutenberg repository: * [WordPress/gutenberg#40260 #40260 Add support for button elements to theme.json] * [WordPress/gutenberg#40889 #40889 Theme Json: Don't output double selectors for elements inside blocks] * [WordPress/gutenberg#41140 #41140 Global Styles: Add support for caption elements] * [WordPress/gutenberg#41160 #41160 Global Styles: Load block CSS conditionally] * [WordPress/gutenberg#41240 #41240 Global Styles: Button Element: update button element selector] * [WordPress/gutenberg#41335 #41335 Duotone: Fix CSS Selectors rendered by theme.json duotone/filter settings for blocks on public pages] * [WordPress/gutenberg#41446 #41446 Block styles: Account for style block nodes that have no name] * [WordPress/gutenberg#41696 #41696 Global Styles: Allow references to values in other locations in the tree] * [WordPress/gutenberg#41753 #41753 Elements: Add an API make it easier to get class names] * [WordPress/gutenberg#41786 #41786 Support pseudo selectors on elements in theme json] * [WordPress/gutenberg#41822 #41822 Elements: Button - Fix element selectors] * [WordPress/gutenberg#41981 #41981 Global Styles: Add support for heading elements] * [WordPress/gutenberg#42072 #42072 Fix link element hover bleeding into button element default styles] * [WordPress/gutenberg#42096 #42096 Add visited to link element allowed pseudo selector list] * [WordPress/gutenberg#42669 #42669 Link elements: Add a :where selector to the :not to lower specificity] * [WordPress/gutenberg#42776 #42776 Theme JSON: Add a static $blocks_metadata data definition to the Gutenberg instance of WP_Theme_JSON] * [WordPress/gutenberg#43088 #43088 Pseudo elements supports on button elements] * [WordPress/gutenberg#43167 #43167 Theme_JSON: Use existing append_to_selector for pseudo elements] * [WordPress/gutenberg#43988 #43988 Styles API: Fixed selectors for nested elements] Props onemaggie, bernhard-reiter, cbravobernal, mmaattiiaass, scruffian, andraganescu, dpcalhoun, get_dave, Mamaduka, SergeyBiryukov. See #56467. Built from https://develop.svn.wordpress.org/trunk@54118
This PR migrates the Style Engine PHP functions, classes and tests into Core for 6.1. It backports the original [WordPress/gutenberg#40260 PR #40260] from Gutenberg repository. Props ramonopoly, bernhard-reiter, costdev, azaozz, andrewserong, mukesh27, aristath. See #56467. Built from https://develop.svn.wordpress.org/trunk@54156
This commit backports the original PRs from Gutenberg repository: * [WordPress/gutenberg#40260 #40260 Add support for button elements to theme.json] * [WordPress/gutenberg#40889 #40889 Theme Json: Don't output double selectors for elements inside blocks] * [WordPress/gutenberg#41140 #41140 Global Styles: Add support for caption elements] * [WordPress/gutenberg#41160 #41160 Global Styles: Load block CSS conditionally] * [WordPress/gutenberg#41240 #41240 Global Styles: Button Element: update button element selector] * [WordPress/gutenberg#41335 #41335 Duotone: Fix CSS Selectors rendered by theme.json duotone/filter settings for blocks on public pages] * [WordPress/gutenberg#41446 #41446 Block styles: Account for style block nodes that have no name] * [WordPress/gutenberg#41696 #41696 Global Styles: Allow references to values in other locations in the tree] * [WordPress/gutenberg#41753 #41753 Elements: Add an API make it easier to get class names] * [WordPress/gutenberg#41786 #41786 Support pseudo selectors on elements in theme json] * [WordPress/gutenberg#41822 #41822 Elements: Button - Fix element selectors] * [WordPress/gutenberg#41981 #41981 Global Styles: Add support for heading elements] * [WordPress/gutenberg#42072 #42072 Fix link element hover bleeding into button element default styles] * [WordPress/gutenberg#42096 #42096 Add visited to link element allowed pseudo selector list] * [WordPress/gutenberg#42669 #42669 Link elements: Add a :where selector to the :not to lower specificity] * [WordPress/gutenberg#42776 #42776 Theme JSON: Add a static $blocks_metadata data definition to the Gutenberg instance of WP_Theme_JSON] * [WordPress/gutenberg#43088 #43088 Pseudo elements supports on button elements] * [WordPress/gutenberg#43167 #43167 Theme_JSON: Use existing append_to_selector for pseudo elements] * [WordPress/gutenberg#43988 #43988 Styles API: Fixed selectors for nested elements] Props onemaggie, bernhard-reiter, cbravobernal, mmaattiiaass, scruffian, andraganescu, dpcalhoun, get_dave, Mamaduka, SergeyBiryukov. See #56467. git-svn-id: https://develop.svn.wordpress.org/trunk@54118 602fd350-edb4-49c9-b593-d223f7449a82
This PR migrates the Style Engine PHP functions, classes and tests into Core for 6.1. It backports the original [WordPress/gutenberg#40260 PR #40260] from Gutenberg repository. Props ramonopoly, bernhard-reiter, costdev, azaozz, andrewserong, mukesh27, aristath. See #56467. git-svn-id: https://develop.svn.wordpress.org/trunk@54156 602fd350-edb4-49c9-b593-d223f7449a82
What?
Adds support for button elements to theme.json. This means themes can now add styles to their buttons using theme.json like this:
Why?
We need a consistent way to style buttons across blocks. The search block is the most obvious example here, but there will also be third party blocks that will benefit from this change.
How?
Adds a new class,
wp-element-button
to buttons, so that they can share the same styles. The search block requires a server side implementation. Right now we don't share the class name between client and server.Testing Instructions
Screenshots or screencast
Frontend:
Editor:
cc @WordPress/block-themers