Block Library: Add width attribute for resizable Column blocks #15499

aduth · 2019-05-08T12:53:17Z

This pull request seeks to implement an initial exploration of resizable column blocks. It should serve as some foundation upon which to improve the usefulness of the Columns block.

In this first iteration:

An empty Column block displays with the "Button" appender introduced in Adds Block Appender as placeholder to empty InnerBlocks #14241, similar to the Group block (Use button block appender on group block #14943). The thinking is that this allows for a simpler gauge of columns width before adding content.
An individual column block can be assigned with a width attribute which overrides the default equal distribution of column widths in a Columns block. Widths of all adjacent Column blocks are updated automatically, and widths redistributed when a Column is added or removed from the Columns block.

Not yet included, but planned for future iterations:

A draggable resizer handle to adjust columns width. I have the bulk of the code for this complete already.
Quick-select "template" options for columns distribution when initially creating a Columns block.
Improved methods to select a Column and a Columns block to adjust settings.

Specific Behavior:

When changing the width of a Column, the amount of increase or decrease is respectively removed or added in equal distribution across the Column blocks after the block being changed, unless the Column block is the last in the set (in which case, the redistribution occurs in reverse).
When adding or removing a Column to a Columns block, the new Column is assigned a width as if there were equal distribution. For example, increasing Columns from 2 to 3 would assume a width of 33.3% for the new block (100 / 3). This width (or, in the case of removal, the width of the removed block) is removed from or added to other blocks in the Columns.
The default behavior of Columns remains unchanged in that width is only assigned or adjusted if a width value was assigned to a Column block in the set. This also helps ensure backwards-compatibility with previously-existing Column blocks.

Testing Instructions:

Verify that you can resize a Column block, and that in doing so, adjacent Column blocks are resized accordingly. Similarly, verify that adding or removing new Columns should redistribute widths to/from other columns.

Navigate to Posts > Add New
Insert a Columns block
Select the Column block (ArrowUp and ArrowDown are reliable here)
Adjust width value in the Inspector
Observe that the second column updates its width accordingly (to a sum of 100)
Select the Columns block
Increase / decrease Colujmsn count
Observe that the previous two columns update their widths accordingly (to a sum of 100)
Repeat steps 3 through 8 with a variation of Column combinations

Remaining tasks:

There is occasionally some issue in columns redistribution related to precision, where the sum total of Column widths may not equal 100 (but rather, e.g. 99.98%)

Soean · 2019-05-08T13:14:23Z

Looks very promising.
Maybe we should add a help text about the unit. It's percent %, not pixel px.

paulwilde · 2019-05-08T14:28:31Z

It would be ideal if the columns could snap into a grid of 12 as is common with most grid foundations such as Bootstrap, or at least if there's a preference to enable such functionality. Reason being this allows for a consistent grid on a website rather than having random widths on the page.

It would also allow for each column to use classes instead of inline styles which are quite troublesome when it comes to making these columns responsive, especially if the ability to set different widths on different breakpoints were to be added in the future.

youknowriad · 2019-05-08T15:42:58Z

packages/block-library/src/column/edit.js

+				renderAppender={ (
+					hasChildBlocks ?
+						undefined :
+						() => <InnerBlocks.ButtonBlockAppender />


I like this change :)

Do you why the sibling inserter is not shown when there's a single column with a single block? It feels like a separate issue but it's made more obvious with this change

Do you why the sibling inserter is not shown when there's a single column with a single block? It feels like a separate issue but it's made more obvious with this change

I think it's the same as the fact that there is no sibling inserter shown at the end of the block list, only the "default appender". The column is effectively the same as the top-level block list.

However, I don't necessary agree that the sibling inserter shouldn't be shown after the last block in a block list. I think there may have been an issue about this at some point?

youknowriad · 2019-05-08T15:46:18Z

packages/block-library/src/column/edit.js

+			updateAlignment( verticalAlignment ) {
+				const { clientId, setAttributes } = ownProps;
+				const { updateBlockAttributes } = dispatch( 'core/block-editor' );
+				const { getBlockRootClientId } = registry.select( 'core/block-editor' );


Nice little perf tweak

Nice little perf tweak

Thanks for reminding: I'd meant to reference some commentary at #13899 (comment) which had led to some of this general (but related/required) refactoring.

youknowriad · 2019-05-08T15:46:50Z

packages/block-library/src/column/edit.js

 		};
 	} ),
-	withDispatch( ( dispatch, { clientId, parentColumnsBlockClientId } ) => {
+	withDispatch( ( dispatch, ownProps, registry ) => {


Nit: Any reason you didn't destructure registry

Nit: Any reason you didn't destructure registry

I'm personally not a fan of destructuring in an arguments signature, since it loses context of what it is that is being destructured. To a lesser degree, I find it hard to visually distinguish between proper arguments, and properties destructured (attention to the { and }).

youknowriad · 2019-05-08T15:48:08Z

packages/block-library/src/column/edit.js

+				// Update own width.
+				setAttributes( { width } );
+
+				// Constrain or expand siblings to account for gain or loss of


I wonder if the computations here could be extracted to a unit-testable pure function.

I wonder if the computations here could be extracted to a unit-testable pure function.

Yeah, I expect there's a fair bit of clean-up which could be done here 😅 Perhaps even some reusability between redistribution logic between the individual Column and its parent Columns block.

I wonder if the computations here could be extracted to a unit-testable pure function.

Still generally more complex than I'd have liked, but improved refactoring and tests in 6880cfa.

youknowriad · 2019-05-08T15:49:36Z

packages/block-library/src/column/save.js

 	const wrapperClasses = classnames( {
 		[ `is-vertically-aligned-${ verticalAlignment }` ]: verticalAlignment,
 	} );

+	let style;
+	if ( Number.isFinite( width ) ) {
+		style = { flexBasis: width + '%' };


is this kses allowed (just to make sure we check as all inline style properties are not allowed)?

is this kses allowed (just to make sure we check as all inline style properties are not allowed)?

Good catch! It is not.

I will need to filter this set, and follow-up with a Trac ticket.

https://core.trac.wordpress.org/browser/trunk/src/wp-includes/kses.php?rev=45242#L2057

is this kses allowed (just to make sure we check as all inline style properties are not allowed)?

Added in 5bb641c

youknowriad · 2019-05-08T15:52:06Z

packages/block-library/src/columns/edit.js

+			return;
+		}
+
+		// Redistribute available width for existing inner blocks.


Same, I wonder if this could be unit testable

youknowriad

Overall, the PR seems great to me. It would open a lot more possibilities for follow-up improvements (drag and drop, setup state)

youknowriad · 2019-05-08T15:54:30Z

@Soean you're commenting from the future mate :) (Github bug)

mapk · 2019-05-08T23:00:08Z

I just tested today and immediately loved the new Button appenders in the columns! Fantastic indicators and visual reference for the columns.

I've got a few notes for some improvements before we merge this.

The column width slider defaults with no number in it. But it's set at 50%. Can we add the number there by default?
It would be great to add a % sign, or maybe just change the label to say Percentage width for the column width slider so that it's obvious we're setting percentage widths.
Can we also add a Reset button to return the columns back to the default setting?

Something like this:

mapk · 2019-05-08T23:01:34Z

Or maybe the reset button can be similar to the Color Settings for the paragraph block and say, clear instead? This way we follow a pattern.

draganescu · 2019-05-09T08:02:57Z

Hi @aduth I've tested and found that the width in certain cases (kinda extreme) make the columns flicker:

aduth · 2019-05-09T12:52:50Z

@draganescu I had seen that on an occasion as well. It's odd, because the only effective difference in markup generated is the application of a flex-basis style, which shouldn't seem like it ought to have this effect. I'd wondered if it was perhaps a browser bug. If you are able to reproduce it, could you try viewing it in another browser?

aduth · 2019-05-09T12:53:18Z

Thanks for the feedback, all. I'll plan to apply it today.

youknowriad

Makes sense to me. Let's work on these tests separately.

aduth · 2019-05-10T21:00:58Z

packages/block-library/src/columns/style.scss

@@ -26,11 +29,9 @@

 	@include break-small() {

-		// Beyond mobile, allow 2 columns.
-		flex-basis: calc(50% - #{$grid-size-large});


I'm assuming this wasn't strictly necessary, also by the fact that it (wrongly) assumes a Columns block would only ever have 2 columns. In some testing, the default width distribution of a columns without widths assigned (flex-basis: auto;) appears to be equal distribution.

I'm assuming this wasn't strictly necessary, also by the fact that it (wrongly) assumes a Columns block would only ever have 2 columns. In some testing, the default width distribution of a columns without widths assigned (flex-basis: auto;) appears to be equal distribution.

Actually, in some final testing, it does appear to make a difference. I'm not sure if it's intended that mid-range viewports show as two-column, but that's how it behaves in master, and I'd rather leave it as it was.

(Note: This applies only to Columns blocks where widths are not explicitly assigned)

Your assumptions sound correct here @aduth
The original design had 2 columns always showing at the medium breakpoint. You may not have noticed it cause It didn't actually work for the first couple versions of the editor. Got fixed here #12199

I don't think that two column breakpoint would be missed too much if removing it would help what you're doing here.

aduth · 2019-05-10T21:17:27Z

Hi @aduth I've tested and found that the width in certain cases (kinda extreme) make the columns flicker:

This seems to happen when columns contain images. In some cross-browser testing, I believe it's an issue specifically affecting Chrome. It's not a great experience surely, but based on my previous comment on how limited the DOM changes to a Column block is with this branch, I strongly suspect this is a browser rendering bug largely out of our control.

paaljoachim · 2019-05-11T08:18:30Z

This feels like one of multiple very useful steps/iterations that will affect width, height, padding, margin, drag, responsive, grid and more of multiple blocks (perhaps all blocks). It would be great with a writeup of this step and thoughts on future steps. So that others could also share feedback. Thanks!

mapk · 2019-05-14T23:02:46Z

🎉 Excellent work!

Will there be another PR soon that works toward the dragging resizing option that Joen mentioned above?

aduth · 2019-05-15T14:31:12Z

Will there be another PR soon that works toward the dragging resizing option that Joen mentioned above?

I'll create some follow-up issues today.

aduth · 2019-05-15T15:19:24Z

Follow-up issues:

Block Library: Column: Add draggable resize control (Block Library: Column: Add draggable resize control #15659)
Block Library: Column: Move resizing controls to Columns block (Block Library: Column: Move resizing controls to Columns block #15660)
Block Editor: Add predefined arrangements options support to InnerBlocks (Block Editor: Add predefined arrangements options support to InnerBlocks #15662)
Block Library: Column: Add predefined "layout" selections for new Columns block (Block Library: Column: Add predefined "layout" selections for new Columns block #15663)
Editor: KSES: Include Flex properties in CSS whitelist to support core Column block width (Trac#47281)

StaggerLeee · 2019-05-15T16:55:38Z

There are already Bootstrap Gutenberg blocks (for columns and much more) that you will never come close to. And you lose your time to make everything from the scratch, when there is almost perfect Bootstrap library to integrate.
I do not understand it.

Does Matt Mullenweg pay you per code lines ?

Ah OK, now continue to adapt them to responsive views.

SchneiderSam · 2019-05-15T16:59:34Z

If I enter 33 in one of my columns, the entire column layout will be destroyed. Since the page is online, I can't show it live. But somewhere there are still problems.

Login: rb
PW: buvexayu
URL: http://b320kx.myraidbox.de/

Normally it should look like this:

but in the live site it looks like this:

aduth · 2019-05-15T19:11:59Z

@StaggerLeee That's an awfully cynical stance to hold, and I think you could express your point without resorting to sarcasm or abuse. It's fine for a plugin and core functionality to coexist in the same space. The WordPress philosophy advocates design for the majority, simplicity, and out-of-the-box usefulness. A plugin can add specialized functionality, and simply absorbing one plugin's implementation isn't always the best option for the baseline offering; though it can certainly inform it, such as is already taking place in follow-up tasks (e.g. #15662 "Prior art"). If you have specific feedback, I'd encourage you to provide it (constructively) in one of the existing issues, a new issue, or to participate in one of the weekly meetings.

@SchneiderSam Could you create a new issue for this? It may be a specific incompatibility with your theme, but a dedicated issue would be the more appropriate place for debugging.

nerrad · 2019-05-15T19:24:37Z

@StaggerLeee please take some time to read the Code of Conduct for this repository. Your comments could be seen as breaching some of the principles of that code. Constructive feedback is always welcome here but repeated abuse of the code of conduct will not be tolerated.

karmatosed · 2019-05-15T19:43:26Z

@StaggerLeee whilst I understand you may have a strong opinion here, @nerrad is correct there is a code of conduct that applies to this repo. I would ask you to please be mindful of this in the future. Every person here is doing their best and individual humans that deserve respectful interactions. Let's focus on all creating the best product we can together.

Thank you @nerrad for your response on this issue 🙏

StaggerLeee · 2019-05-15T19:46:37Z

What a fuss, just for mentioning the Boss. My apologies.

karmatosed · 2019-05-15T19:51:13Z

Whilst again I understand your feelings here, the code of conduct is taken seriously within this repo and WordPress itself. Let's draw a line for now under this and move on with that clear understanding for future interactions.

m-e-h · 2019-05-15T21:15:04Z

@StaggerLeee have you checked out the Joomla CMS? Pretty sure it still includes the bootstrap framework in it's core. Of course there are challenges to such an approach but it sounds like it may be closer to what you're looking for.

I don't get paid anything for my code here 🤷‍♂️

…ress#15499) * Block Library: Render Column block with ButtonBlockAppender * Block Library: Add width attribute for resizable Column blocks * Block Library: Update Column width input label to clarify percentage * Block Library: Refactor Columns width redistribution to reusable utilities * Plugin: Filter safe CSS to allow column flex-basis CSS attribute * Block Library: Return undefined for non-finite column width precision * Components: Fix display of RangeControl reset button * Block Library: Allow column block width resetting * Block Library: Ensure Column width RangeControl treated as controlled input * Testing: Update E2E tests for button inserter columns block * Testing: Try to appease the E2E gods * Block Library: Restore flex-basis for mid-range viewports

aduth added 2 commits May 8, 2019

Block Library: Render Column block with ButtonBlockAppender

0442276

Block Library: Add width attribute for resizable Column blocks

67d2b75

aduth added [Feature] Blocks Needs Design Feedback Anything that needs general design feedback. [Block] Columns labels May 8, 2019

aduth requested review from ajitbohra, gziolo, jorgefilipecosta, noisysocks, notnownikki, Soean, talldan and youknowriad as code owners May 8, 2019

youknowriad reviewed May 8, 2019

View changes

jasmussen mentioned this pull request May 9, 2019

Selecting Parent Blocks: Try clickthrough #15537

Merged

allancole mentioned this pull request May 9, 2019

All Theme Variations: Add a 1/3 + 2/3 custom class for the Columns Blocks Automattic/themes#754

Closed

youknowriad approved these changes May 10, 2019

View changes

aduth reviewed May 10, 2019

View changes

Block Library: Restore flex-basis for mid-range viewports

1a9fa24

aduth merged commit 5494bc5 into master May 12, 2019
1 check passed

aduth deleted the update/columns-width branch May 12, 2019

youknowriad added this to the 5.7 (Gutenberg) milestone May 13, 2019

This was referenced May 15, 2019

Block Library: Column: Add draggable resize control #15659

Open

Block Library: Column: Move resizing controls to Columns block #15660

Closed

Block Library: Column: Add predefined "layout" selections for new Columns block #15663

Closed

SchneiderSam mentioned this pull request May 15, 2019

Problems with the new Gutenberg Columns tomusborne/generatepress#91

Closed

This was referenced May 21, 2019

Enables Block Grouping/UnGrouping using core/group #14908

Merged

Blocks: Consider a "passthrough" supports for nested block wrappers #7694

Open

aduth mentioned this pull request Jun 10, 2019

Block Library: Move Column resizing to Columns #16077

Closed

youknowriad mentioned this pull request Aug 19, 2019

Phase 2 Scope & Features #13113

Closed

22 tasks

LukaszJaro mentioned this pull request Oct 29, 2019

Allow for core block Inspector Controls to be extended and removed #6023

Closed

Block Library: Add width attribute for resizable Column blocks #15499

Block Library: Add width attribute for resizable Column blocks #15499

Conversation

aduth commented May 8, 2019 • edited

Soean commented May 8, 2019

paulwilde commented May 8, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowriad left a comment

youknowriad commented May 8, 2019

mapk commented May 8, 2019

mapk commented May 8, 2019

draganescu commented May 9, 2019

aduth commented May 9, 2019

aduth commented May 9, 2019

youknowriad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduth commented May 10, 2019

paaljoachim commented May 11, 2019

mapk commented May 14, 2019

aduth commented May 15, 2019

aduth commented May 15, 2019

StaggerLeee commented May 15, 2019 • edited

SchneiderSam commented May 15, 2019

aduth commented May 15, 2019

nerrad commented May 15, 2019 • edited

karmatosed commented May 15, 2019

StaggerLeee commented May 15, 2019

karmatosed commented May 15, 2019

m-e-h commented May 15, 2019

aduth commented May 8, 2019 •

edited

paulwilde commented May 8, 2019 •

edited

StaggerLeee commented May 15, 2019 •

edited

nerrad commented May 15, 2019 •

edited