Skip to content

Allow using a button element for button blocks #54206

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.

Sign up for GitHub

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

Merged
aristath merged 3 commits into from
Sep 7, 2023
Merged

Allow using a button element for button blocks #54206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ab48f46
Allow using a button element for button blocks
aristath Sep 6, 2023
a6d4ccd
update tests / fixtures
aristath Sep 6, 2023
00debe3
Add "type" attribute for buttons
aristath Sep 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/reference-guides/core-blocks.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ Prompt visitors to take action with a button-style link. ([Source](https://githu
- **Category:** design
- **Parent:** core/buttons
- **Supports:** anchor, color (background, gradients, text), shadow, spacing (padding), typography (fontSize, lineHeight), ~~alignWide~~, ~~align~~, ~~reusable~~
- **Attributes:** backgroundColor, gradient, linkTarget, placeholder, rel, text, textAlign, textColor, title, url, width
- **Attributes:** backgroundColor, gradient, linkTarget, placeholder, rel, tagName, text, textAlign, textColor, title, type, url, width

## Buttons

Expand Down
12 changes: 10 additions & 2 deletions packages/block-library/src/button/block.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@
"keywords": [ "link" ],
"textdomain": "default",
"attributes": {
"tagName": {
"type": "string",
"default": "a"
},
Comment on lines +12 to +15

@Mamaduka Mamaduka Sep 13, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be an enum as we only want to allow a and button elements.

@aristath aristath Sep 13, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we can actually use enum... At least not without changing the schema for block.json 🤔

@Mamaduka Mamaduka Sep 13, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

"direction": {
"type": "string",
"enum": [ "ltr", "rtl" ]
}

@aristath aristath Sep 13, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah OK!! That makes sense now... I thought you were talking about making the type an enum 😅

Comment thread
Mamaduka marked this conversation as resolved.
"type": {
"type": "string",
"default": "button"
},
"textAlign": {
"type": "string"
},
Expand All @@ -22,14 +30,14 @@
"title": {
"type": "string",
"source": "attribute",
"selector": "a",
"selector": "a,button",
"attribute": "title",
"__experimentalRole": "content"
},
"text": {
"type": "string",
"source": "html",
"selector": "a",
"selector": "a,button",
"__experimentalRole": "content"
},
"linkTarget": {
Expand Down
37 changes: 26 additions & 11 deletions packages/block-library/src/button/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,19 @@ function ButtonEdit( props ) {
onReplace,
mergeBlocks,
} = props;
const { textAlign, linkTarget, placeholder, rel, style, text, url, width } =
attributes;
const {
tagName,
textAlign,
linkTarget,
placeholder,
rel,
style,
text,
url,
width,
} = attributes;

const TagName = tagName || 'a';

function onToggleOpenInNewTab( value ) {
const newLinkTarget = value ? '_blank' : undefined;
Expand Down Expand Up @@ -209,7 +220,7 @@ function ButtonEdit( props ) {
setAttributes( { textAlign: nextAlign } );
} }
/>
{ ! isURLSet && (
{ ! isURLSet && 'a' === TagName && (

@Mamaduka Mamaduka Sep 13, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The 'a' === TagName check could be extracted into isLinkTag and reused in code.

<ToolbarButton
name="link"
icon={ link }
Expand All @@ -218,7 +229,7 @@ function ButtonEdit( props ) {
onClick={ startEditing }
/>
) }
{ isURLSet && (
{ isURLSet && 'a' === TagName && (
<ToolbarButton
name="link"
icon={ linkOff }
Expand All @@ -229,7 +240,7 @@ function ButtonEdit( props ) {
/>
) }
</BlockControls>
{ isSelected && ( isEditingURL || isURLSet ) && (
{ 'a' === TagName && isSelected && ( isEditingURL || isURLSet ) && (
<Popover
placement="bottom"
onClose={ () => {
Expand Down Expand Up @@ -268,12 +279,16 @@ function ButtonEdit( props ) {
/>
</InspectorControls>
<InspectorControls group="advanced">
<TextControl
__nextHasNoMarginBottom
label={ __( 'Link rel' ) }
value={ rel || '' }
onChange={ ( newRel ) => setAttributes( { rel: newRel } ) }
/>
{ 'a' === TagName && (
<TextControl
__nextHasNoMarginBottom
label={ __( 'Link rel' ) }
value={ rel || '' }
onChange={ ( newRel ) =>
setAttributes( { rel: newRel } )
}
/>
) }
</InspectorControls>
</>
);
Expand Down
13 changes: 9 additions & 4 deletions packages/block-library/src/button/save.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import {

export default function save( { attributes, className } ) {
const {
tagName,
type,
textAlign,
fontSize,
linkTarget,
Expand All @@ -32,6 +34,8 @@ export default function save( { attributes, className } ) {
return null;
}

const TagName = tagName || 'a';
const buttonType = type || 'button';
const borderProps = getBorderClassesAndStyles( attributes );
const colorProps = getColorClassesAndStyles( attributes );
const spacingProps = getSpacingClassesAndStyles( attributes );
Expand Down Expand Up @@ -65,14 +69,15 @@ export default function save( { attributes, className } ) {
return (
<div { ...useBlockProps.save( { className: wrapperClasses } ) }>
<RichText.Content
tagName="a"
tagName={ TagName }
type={ 'button' === TagName ? buttonType : null }
className={ buttonClasses }
href={ url }
href={ 'button' === TagName ? null : url }

@Mamaduka Mamaduka Sep 13, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above - 'button' === TagName could be extracted into isButtonTag and reused.

title={ title }
style={ buttonStyle }
value={ text }
target={ linkTarget }
rel={ rel }
target={ 'button' === TagName ? null : linkTarget }
rel={ 'button' === TagName ? null : rel }
/>
</div>
);
Expand Down
2 changes: 2 additions & 0 deletions test/integration/fixtures/blocks/core__button__squared.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
"name": "core/button",
"isValid": true,
"attributes": {
"tagName": "a",
"type": "button",
"text": "My button",
"style": {
"border": {
Expand Down
8 changes: 8 additions & 0 deletions test/integration/fixtures/blocks/core__buttons.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,13 @@
<!-- wp:button -->
<div class="wp-block-button"><a class="wp-block-button__link wp-element-button">My button 2</a></div>
<!-- /wp:button -->

<!-- wp:button {"tagName":"a"} -->
<div class="wp-block-button"><a class="wp-block-button__link wp-element-button">My button 3</a></div>
<!-- /wp:button -->

<!-- wp:button {"tagName":"button"} -->
<div class="wp-block-button"><button type="button" class="wp-block-button__link wp-element-button">My button 4</button></div>
<!-- /wp:button -->
</div>
<!-- /wp:buttons -->
24 changes: 24 additions & 0 deletions test/integration/fixtures/blocks/core__buttons.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
"name": "core/button",
"isValid": true,
"attributes": {
"tagName": "a",
"type": "button",
"text": "My button 1"
},
"innerBlocks": []
Expand All @@ -22,9 +24,31 @@
"name": "core/button",
"isValid": true,
"attributes": {
"tagName": "a",
"type": "button",
"text": "My button 2"
},
"innerBlocks": []
},
{
"name": "core/button",
"isValid": true,
"attributes": {
"tagName": "a",
"type": "button",
"text": "My button 3"
},
"innerBlocks": []
},
{
"name": "core/button",
"isValid": true,
"attributes": {
"tagName": "button",
"type": "button",
"text": "My button 4"
},
"innerBlocks": []
}
]
}
Expand Down
28 changes: 27 additions & 1 deletion test/integration/fixtures/blocks/core__buttons.parsed.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,40 @@
"innerContent": [
"\n\t<div class=\"wp-block-button\"><a class=\"wp-block-button__link wp-element-button\">My button 2</a></div>\n\t"
]
},
{
"blockName": "core/button",
"attrs": {
"tagName": "a"
},
"innerBlocks": [],
"innerHTML": "\n\t<div class=\"wp-block-button\"><a class=\"wp-block-button__link wp-element-button\">My button 3</a></div>\n\t",
"innerContent": [
"\n\t<div class=\"wp-block-button\"><a class=\"wp-block-button__link wp-element-button\">My button 3</a></div>\n\t"
]
},
{
"blockName": "core/button",
"attrs": {
"tagName": "button"
},
"innerBlocks": [],
"innerHTML": "\n\t<div class=\"wp-block-button\"><button type=\"button\" class=\"wp-block-button__link wp-element-button\">My button 4</button></div>\n\t",
"innerContent": [
"\n\t<div class=\"wp-block-button\"><button type=\"button\" class=\"wp-block-button__link wp-element-button\">My button 4</button></div>\n\t"
]
}
],
"innerHTML": "\n<div class=\"wp-block-buttons alignwide\">\n\t\n\n\t\n</div>\n",
"innerHTML": "\n<div class=\"wp-block-buttons alignwide\">\n\t\n\n\t\n\n\t\n\n\t\n</div>\n",
"innerContent": [
"\n<div class=\"wp-block-buttons alignwide\">\n\t",
null,
"\n\n\t",
null,
"\n\n\t",
null,
"\n\n\t",
null,
"\n</div>\n"
]
}
Expand Down
8 changes: 8 additions & 0 deletions test/integration/fixtures/blocks/core__buttons.serialized.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,13 @@

<!-- wp:button -->
<div class="wp-block-button"><a class="wp-block-button__link wp-element-button">My button 2</a></div>
<!-- /wp:button -->

<!-- wp:button -->
<div class="wp-block-button"><a class="wp-block-button__link wp-element-button">My button 3</a></div>
<!-- /wp:button -->

<!-- wp:button {"tagName":"button"} -->
<div class="wp-block-button"><button type="button" class="wp-block-button__link wp-element-button">My button 4</button></div>
<!-- /wp:button --></div>
<!-- /wp:buttons -->