Add: Buttons block by jorgefilipecosta · Pull Request #17352 · WordPress/gutenberg

jorgefilipecosta · 2019-09-05T21:56:43Z

Description

Closes: #16480
This PR adds a buttons block. A container for buttons. It is starting point right now it is not much more than a container but we can follow up from this and discussi new features to add.

How has this been tested?

I tested the buttons and button block work as expected by doing some smoke tests.

Screenshots

mapk · 2019-09-12T15:29:20Z

While I love the idea of adding multiple buttons to a block, I keep thinking, should this just be part of the Button block? For example, with the addition of the Social block, I don't have a Social Block AND a Socials block. Whether it's one or many, it's all in one block. This, for me, seems how the Button block should work.

It appears there wasn't a strong preference in the issue #16480, and I'm completely open to being convinced otherwise, but I feel pretty strongly toward including this in the existing Button block instead of creating another block.

jorgefilipecosta · 2019-09-12T18:56:57Z

Hi @mapk, technically it is possible to follow this path and have a button block that can contain multiple buttons I don't see any blocker to this.
In the issue @mtias mentioned calling this block "Buttons" any thoughts on the unification of Buttons and Button @mtias?

paaljoachim · 2019-09-18T09:36:45Z

It seems really weird and confusing to have a button block and a buttons block. It would be better to improve the existing button block and also rename it to buttons.

mtias · 2019-09-22T18:21:18Z

Button is expected to become a child block of Buttons, so the inserter would only expose Buttons by default. Just need to ensure nothing weird happens if you use Button directly in a template or existing content.

jorgefilipecosta · 2019-09-23T11:41:40Z

Button is expected to become a child block of Buttons, so the inserter would only expose Buttons by default. Just need to ensure nothing weird happens if you use Button directly in a template or existing content.

Hi @mtias, this PR was updated and now follows this logic. Only buttons block is available, button can still be used in existing content and existing templates.

scruffian · 2019-09-24T11:58:51Z

I tested this and it looks great 👏

paaljoachim · 2019-09-24T12:21:20Z

Is this being backported to WP 5.3?

mtias · 2019-09-24T13:01:10Z

Is this being backported to WP 5.3?

No, for next release. Too many changes already.

mtias · 2019-09-24T13:02:31Z

+
+
+.wp-block-buttons {
+	// 1. Reset margins on immediate innerblocks container.


Can we extract all of this to InnerBlocks horizontalDisplay?

I extracted this CSS logic into generic InnerBlocks flags.

jorgefilipecosta · 2019-11-01T12:42:11Z

I performed some updates on this PR, following some suggestions by @mtias. I moved the movers into the block toolbar as an experiment, and now the standard block UI is hidden using generic inner blocks flags added in #18173.

youknowriad · 2020-01-03T08:24:55Z

+				if ( setting === 'opensInNewTab' ) {
+					onToggleOpenInNewTab( value );
+				}
+			} }


The API is a bit weird to me. That's feedback that is not meant directly to this PR but more about the LinkControl component. cc @getdave
I think ideally, the current link prop is just another setting and we should have a single onChange handler. Any particulra reason to separate both?
I also feel we should separate the definition of the settings (id, title) from the value (checked).
Should we also absorb handleLinkControlOnKeyDown inside the component itself?

I took a stab at this here #19396

youknowriad

Nice work here 👍

… part of InnerBlocks), removed toolbar movers (they are now part of InnerBlocks)

aduth · 2020-01-06T20:07:24Z

I've been seeing quite a few intermittent build failures recently which appear to stem from the test file introduced in this pull request.

Example: https://travis-ci.com/WordPress/gutenberg/jobs/272683577

ellatrix · 2020-01-14T08:16:04Z

The alignment buttons don't work. See #19616.

aduth · 2020-01-14T19:16:29Z

+const handleLinkControlOnKeyPress = ( event ) => {
+	event.stopPropagation();
+};


What purpose does this serve? How would a future maintainer be expected to interpret this requirement to avoid a regression?

A few suggestions:

Event#stopPropagation is a code smell, and we should avoid it as much as possible

When we have no alternative, we should document exhaustively why we're stopping propagation. Something like an inline comment or, in this case, even just naming the function in such a way which communicates the intention.

cinghaman · 2020-01-15T14:53:07Z

@aduth do you know if this is going to be added to the core gutenberg anytime soon?

aduth · 2020-01-15T18:27:52Z

@cinghaman This is already available in the latest version of the plugin.

See: https://make.wordpress.org/core/2020/01/09/whats-new-in-gutenberg-8-january/

Based on the regular development schedule, it would be expected this should be available in the next major release of WordPress (5.4).

jorgefilipecosta requested review from Soean, ajitbohra, ellatrix, gziolo, nerrad, ntwb, talldan and youknowriad as code owners September 5, 2019 21:56

jorgefilipecosta added the New Block Suggestion for a new block label Sep 5, 2019

jorgefilipecosta force-pushed the add/buttons-block branch from 61fbfd6 to 50429bf Compare September 6, 2019 11:58

scruffian mentioned this pull request Sep 23, 2019

Add a new buttons block #17523

Closed

5 tasks

jorgefilipecosta force-pushed the add/buttons-block branch from 50429bf to 578c1f2 Compare September 23, 2019 11:31

jorgefilipecosta force-pushed the add/buttons-block branch from 578c1f2 to ca64d9a Compare September 23, 2019 20:54

mtias reviewed Sep 24, 2019

View reviewed changes

jorgefilipecosta force-pushed the add/buttons-block branch 2 times, most recently from b279dda to 90c1f46 Compare October 29, 2019 20:49

jorgefilipecosta mentioned this pull request Oct 29, 2019

Add functionality on inner blocks to not include parts of the UI. #18173

Merged

jorgefilipecosta force-pushed the add/buttons-block branch 2 times, most recently from c398b47 to e364932 Compare November 1, 2019 12:17

jasmussen reviewed Nov 1, 2019

View reviewed changes

Comment thread packages/block-library/src/editor.scss Outdated

jasmussen reviewed Nov 1, 2019

View reviewed changes

Comment thread packages/edit-post/src/components/visual-editor/style.scss Outdated

youknowriad reviewed Jan 3, 2020

View reviewed changes

youknowriad approved these changes Jan 3, 2020

View reviewed changes

jorgefilipecosta added 5 commits January 3, 2020 11:14

Add: Buttons block

5543da9

Used LinkControl, Removed logic to insert block right away (it is not…

483c7c1

… part of InnerBlocks), removed toolbar movers (they are now part of InnerBlocks)

Make UX closer to navigation block

d03c51c

Update tests

6d0cfbc

Use jsx string for property

9b44f75

jorgefilipecosta force-pushed the add/buttons-block branch from b9afda0 to 9b44f75 Compare January 3, 2020 11:14

jorgefilipecosta merged commit a74921d into master Jan 3, 2020

jorgefilipecosta deleted the add/buttons-block branch January 3, 2020 11:38

youknowriad added this to the Gutenberg 7.2 milestone Jan 6, 2020

mmtr mentioned this pull request Jan 6, 2020

ISNBAT use the CoBlocks buttons block in Gutenberg 7.2.0 godaddy-wordpress/coblocks#1235

Closed

4 tasks

georgeolaru mentioned this pull request Jan 9, 2020

Add Support for Buttons Group (7.2+) pixelgrade/anima#64

Closed

2 tasks

aduth reviewed Jan 14, 2020

View reviewed changes

aduth added the [Block] Buttons label Jan 15, 2020

youknowriad added [Block] Buttons Affects the Buttons Block and removed [Block] Buttons labels Jan 16, 2020

aduth mentioned this pull request Jan 17, 2020

Block Library: Button: Remove title attribute handling #19735

Merged

pinarol mentioned this pull request Jan 21, 2020

Add Buttons block wordpress-mobile/gutenberg-mobile#1790

Closed

aduth mentioned this pull request Jan 31, 2020

Block Library: Navigation: Remove title attribute from navigation link #19990

Merged

jorgefilipecosta added the Needs Dev Note Requires a developer note for a major WordPress release cycle label Feb 11, 2020

jorgefilipecosta mentioned this pull request Feb 12, 2020

Block Editor Dev Notes 5.4 #20185

Closed

23 tasks

marekhrabe mentioned this pull request Feb 25, 2020

Remove !important override from Buttons block #20433

Merged

1 task

strarsis mentioned this pull request Mar 5, 2020

[Design] buttons container block? woocommerce/woocommerce-blocks#1886

Closed

youknowriad removed the Needs Dev Note Requires a developer note for a major WordPress release cycle label Apr 8, 2020

0aveRyan mentioned this pull request Apr 10, 2020

Button Update Doesn't Make Centering and Moving of Block Obvious/Intuitive #21509

Closed



		.wp-block-buttons {
		// 1. Reset margins on immediate innerblocks container.

Conversation

jorgefilipecosta commented Sep 5, 2019

Description

How has this been tested?

Screenshots

Uh oh!

mapk commented Sep 12, 2019

Uh oh!

jorgefilipecosta commented Sep 12, 2019

Uh oh!

paaljoachim commented Sep 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtias commented Sep 22, 2019

Uh oh!

jorgefilipecosta commented Sep 23, 2019

Uh oh!

scruffian commented Sep 24, 2019

Uh oh!

paaljoachim commented Sep 24, 2019

Uh oh!

mtias commented Sep 24, 2019

Uh oh!

mtias Sep 24, 2019

Choose a reason for hiding this comment

Uh oh!

jorgefilipecosta Nov 1, 2019

Choose a reason for hiding this comment

Uh oh!

jorgefilipecosta commented Nov 1, 2019

Uh oh!

Uh oh!

Uh oh!

youknowriad Jan 3, 2020

Choose a reason for hiding this comment

Uh oh!

youknowriad Jan 3, 2020

Choose a reason for hiding this comment

Uh oh!

youknowriad left a comment

Choose a reason for hiding this comment

Uh oh!

aduth commented Jan 6, 2020

Uh oh!

ellatrix commented Jan 14, 2020

Uh oh!

aduth Jan 14, 2020

Choose a reason for hiding this comment

Uh oh!

cinghaman commented Jan 15, 2020

Uh oh!

aduth commented Jan 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

paaljoachim commented Sep 18, 2019 •

edited

Loading