Allow using a button element for button blocks #54206
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aristath
requested review from
ajitbohra,
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
|
Size Change: +83 B (0%) Total Size: 1.52 MB
ℹ️ View Unchanged
|
aristath
force-pushed
the
add/button-block-button-element
branch
from
f2df198 to
a6d4ccd
Compare
aristath
added
[Type] Feature
|
Question 🤔 It is often recommended that any button element should have an explicit
I can see how this automatic contextual awareness could also be seen as a feature rather than an issue. But then we run into the issue that we have no way of adding additional buttons inside forms like a reset button or just a regular button. I would therefore say that we should add a <!-- wp:buttons -->
<div class="wp-block-buttons"><!-- wp:button {"tagName":"button","type":"button"} -->
<div class="wp-block-button"><button class="wp-block-button__link wp-element-button" type="button">Actual button</button></div>
<!-- /wp:button --></div>
<!-- /wp:buttons --></div> |
|
Thank you for the feedback @fabiankaegy!
Makes sense... Added, and updated the PR description 👍 |
fabiankaegy
approved these changes
Sep 7, 2023
Mamaduka
reviewed
Sep 13, 2023
Comment on lines
+12
to
+15
| "tagName": { | ||
| "type": "string", | ||
| "default": "a" | ||
| }, |
There was a problem hiding this comment.
This should probably be an enum as we only want to allow a and button elements.
There was a problem hiding this comment.
I don't think we can actually use enum... At least not without changing the schema for block.json 🤔
There was a problem hiding this comment.
What's the exact reason?
I thought we could use something like this:
gutenberg/packages/block-library/src/paragraph/block.json
Lines 29 to 32 in 6e9c1d3
There was a problem hiding this comment.
Ah OK!! That makes sense now... I thought you were talking about making the type an enum 😅
| @@ -209,7 +220,7 @@ function ButtonEdit( props ) { | |||
| setAttributes( { textAlign: nextAlign } ); | |||
| } } | |||
| /> | |||
| { ! isURLSet && ( | |||
| { ! isURLSet && 'a' === TagName && ( | |||
There was a problem hiding this comment.
Suggestion: The 'a' === TagName check could be extracted into isLinkTag and reused in code.
| className={ buttonClasses } | ||
| href={ url } | ||
| href={ 'button' === TagName ? null : url } |
There was a problem hiding this comment.
Same as above - 'button' === TagName could be extracted into isButtonTag and reused.
Merged
annezazu
added
the
[Focus] Accessibility (a11y)
|
Warning: Type of PR label error To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Allows using a
<button>element for button blocks.Why?
Because it would improve semantics (and therefore also a11y), and allow us to use the block in more scenarios where a link is not the right element to use.
Right now, the
core/buttonblocks prints a link. Though visually this is OK, semantically it can be limiting and in some cases incorrect. Currently, we have no way to add an actual<button>element, and the button block doesn't do what someone with some basic HTML knowledge would expect it to do.Additionally, a real
<button>element is something we could use in other blocks like the search block, comments block, as well as other future blocks.This came out of #44214
How?
Added a
tagNameattribute to thecore/buttonblock.<button>elements don't havehref, so when using a button HTML element, the URL input is hidden from the UI.UPDATE: After this comment, A
typeattribute was also added to allow defining the button type.Nothing changes for link buttons.
IMPORTANT:
This PR does not add a UI for selecting the element. It just makes it possible for users to manually change it via the Code Editor, without getting an error when they switch back to the visual editor.
Testing Instructions
Testing Instructions for Keyboard
Nothing to test