Aligments: explore idea on image block #20656

ellatrix · 2020-03-05T13:48:58Z

Description

Explores ideas in #20650.

(As you can see in the screenshot above, the block outline and toolbar are positioned correctly without an exception to the rule.)

The idea:

Add an "alignment wrapper" around the block instead of inside the block content. This alignment wrapper can similarly be used to position floated images relative to the main column.

Here's what the editor DOM looks like:

As you can see it wraps around the block.

There are two ways to implement this:

Either we automatically add this wrapper to both the edit and save output when we detect block support for it.
Or the block add the wrapper by itself. A slight challenge here is that we automatically add wp-block-* classes to the top level element returned by save, which shouldn't be added to the wrapper.

Benefits:

Much more straightforward to implement by blocks, especially if the wrapper would be added internally to both edit and save output.
There's no exceptions needed to handle toolbar position and block selection styles.

How has this been tested?

Screenshots

Types of changes

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR.

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

github-actions · 2020-03-05T13:53:41Z

Size Change: -2.27 kB (0%)

Total Size: 863 kB

Filename	Size	Change
`build/annotations/index.js`	3.43 kB	-1 B
`build/api-fetch/index.js`	3.39 kB	-1 B
`build/block-editor/index.js`	104 kB	-473 B (0%)
`build/block-editor/style-rtl.css`	10.5 kB	-6 B (0%)
`build/block-editor/style.css`	10.5 kB	-9 B (0%)
`build/block-library/editor-rtl.css`	7.26 kB	-100 B (1%)
`build/block-library/editor.css`	7.26 kB	-100 B (1%)
`build/block-library/index.js`	115 kB	-862 B (0%)
`build/blocks/index.js`	57.7 kB	+50 B (0%)
`build/components/index.js`	191 kB	-126 B (0%)
`build/components/style-rtl.css`	15.5 kB	-18 B (0%)
`build/components/style.css`	15.5 kB	-12 B (0%)
`build/compose/index.js`	5.75 kB	-2 B (0%)
`build/date/index.js`	5.36 kB	-4 B (0%)
`build/dom-ready/index.js`	569 B	+1 B
`build/edit-post/index.js`	91.3 kB	-213 B (0%)
`build/edit-post/style-rtl.css`	8.8 kB	+137 B (1%)
`build/edit-post/style.css`	8.8 kB	+141 B (1%)
`build/edit-site/style-rtl.css`	2.48 kB	-28 B (1%)
`build/edit-site/style.css`	2.48 kB	-27 B (1%)
`build/edit-widgets/style-rtl.css`	2.59 kB	-2 B (0%)
`build/edit-widgets/style.css`	2.58 kB	-2 B (0%)
`build/editor/index.js`	43.8 kB	-117 B (0%)
`build/escape-html/index.js`	734 B	+1 B
`build/format-library/index.js`	7.11 kB	-492 B (6%)	✅
`build/hooks/index.js`	1.92 kB	+1 B
`build/keyboard-shortcuts/index.js`	2.3 kB	+1 B
`build/list-reusable-blocks/index.js`	2.99 kB	+1 B
`build/nux/index.js`	3.01 kB	-16 B (0%)
`build/plugins/index.js`	2.54 kB	-1 B
`build/redux-routine/index.js`	2.84 kB	+1 B
`build/rich-text/index.js`	14.3 kB	+1 B
`build/server-side-render/index.js`	2.55 kB	+6 B (0%)
`build/url/index.js`	4 kB	-1 B
`build/warning/index.js`	1.14 kB	-1 B

ℹ️ View Unchanged

Filename	Size	Change
`build/a11y/index.js`	1.01 kB	0 B
`build/autop/index.js`	2.58 kB	0 B
`build/blob/index.js`	620 B	0 B
`build/block-directory/index.js`	6.02 kB	0 B
`build/block-directory/style-rtl.css`	760 B	0 B
`build/block-directory/style.css`	760 B	0 B
`build/block-library/style-rtl.css`	7.5 kB	0 B
`build/block-library/style.css`	7.51 kB	0 B
`build/block-library/theme-rtl.css`	669 B	0 B
`build/block-library/theme.css`	671 B	0 B
`build/block-serialization-default-parser/index.js`	1.65 kB	0 B
`build/block-serialization-spec-parser/index.js`	3.1 kB	0 B
`build/core-data/index.js`	10.6 kB	0 B
`build/data-controls/index.js`	1.03 kB	0 B
`build/data/index.js`	8.22 kB	0 B
`build/deprecated/index.js`	772 B	0 B
`build/dom/index.js`	3.06 kB	0 B
`build/edit-site/index.js`	4.64 kB	0 B
`build/edit-widgets/index.js`	4.42 kB	0 B
`build/editor/editor-styles-rtl.css`	325 B	0 B
`build/editor/editor-styles.css`	327 B	0 B
`build/editor/style-rtl.css`	3.98 kB	0 B
`build/editor/style.css`	3.98 kB	0 B
`build/element/index.js`	4.45 kB	0 B
`build/format-library/style-rtl.css`	502 B	0 B
`build/format-library/style.css`	502 B	0 B
`build/html-entities/index.js`	622 B	0 B
`build/i18n/index.js`	3.48 kB	0 B
`build/is-shallow-equal/index.js`	710 B	0 B
`build/keycodes/index.js`	1.68 kB	0 B
`build/list-reusable-blocks/style-rtl.css`	226 B	0 B
`build/list-reusable-blocks/style.css`	226 B	0 B
`build/media-utils/index.js`	4.85 kB	0 B
`build/notices/index.js`	1.57 kB	0 B
`build/nux/style-rtl.css`	616 B	0 B
`build/nux/style.css`	613 B	0 B
`build/primitives/index.js`	1.49 kB	0 B
`build/priority-queue/index.js`	780 B	0 B
`build/shortcode/index.js`	1.7 kB	0 B
`build/token-list/index.js`	1.27 kB	0 B
`build/viewport/index.js`	1.61 kB	0 B
`build/wordcount/index.js`	1.18 kB	0 B

_{compressed-size-action}

youknowriad

For the className automatically added to save, I've been wanting to remove the automatic addition for some time now to match edit and server. So might we can tie the removal of the magic addition with the lighterBlockWrapper opt-in behavior?

ellatrix · 2020-03-05T13:56:49Z

@youknowriad Yes, that sounds like a good idea. :) I was thinking it might be useful to have some sort of Block.* variant that can be used in the save function. This component could then add the class by default, and it future proofs anything we might want to add in the future.

youknowriad · 2020-03-05T13:59:52Z

Something unrelated I'm wondering, why did you go with <Block.* instead of <Block tagName={"something"} as it's a pattern that is widely used in our codebase?

ellatrix · 2020-03-05T14:01:42Z

So that it could be passed to e.g. RichText: <RichText tagName={ Block.p } />. This is a bit inspired by react-spring.

ellatrix · 2020-03-05T14:02:10Z

They could be seen as enhanced tags. :)

youknowriad · 2020-03-05T15:39:01Z

packages/edit-post/src/style.scss

+	margin-left: auto;
+	margin-right: auto;
+
+	> .wp-block,


why do we need this?

Because the max-width should apply to the align wrapper, but not the nested block

youknowriad · 2020-03-05T15:40:45Z

packages/edit-post/src/style.scss

+	}
+
+	&.wp-align-center {
+		// Same as no alignment?


this needs display: flex and justify-content center for me.

you should be able to see the difference between "center" and "no" when you resize an image smaller than the $content-width

display: flex has the unfortunate side effect of not collapsing margins. :) I'll figure it out though.

youknowriad · 2020-03-05T15:44:18Z

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

 		// Disable reason: Each block can be selected by clicking on it
 		/* eslint-disable jsx-a11y/click-events-have-key-events */
 		return (
 			<>
 				{ controls }
-				<AlignmentWrapper>
-					<BlockContentWrapper className={ classes }>
+				<div className={ `wp-align-wrapper wp-align-${ align }` }>


we're having wp-align-undefined sometimes

youknowriad · 2020-03-05T15:46:06Z

In my testing, floats are not working properly because it seems the width of the figure is always $content-size even if the image is small.

(same for wide and full)

youknowriad · 2020-03-05T15:48:43Z

Seems like the breakage is because of these editor styles https://developer.wordpress.org/block-editor/developers/themes/theme-support/#changing-the-width-of-the-editor

We need to figure a new way for these? or appy the wp-block className differently

youknowriad · 2020-03-05T15:52:18Z

One potential solution is to extract the styles defining the widths from here https://github.com/WordPress/gutenberg/pull/20656/files#diff-556b1933de5650dc555c469c584accc2R116 and move them to editor-styles.css

another solution would be to rely on CSS variables (but maybe it's too soon for this)

ellatrix · 2020-03-05T17:24:50Z

For the className automatically added to save, I've been wanting to remove the automatic addition for some time now to match edit and server. So might we can tie the removal of the magic addition with the lighterBlockWrapper opt-in behavior?

What's the behaviour for server? What do you envision for save? I was thinking of mirroring Block.* in the save function as well, which would automatically have the classes and filtered props etc. What do you think?

youknowriad · 2020-03-06T07:02:19Z

I was thinking of mirroring Block.* in the save function as well

This would work, In the server we do nothing, we rely on the block author.

youknowriad · 2020-03-06T08:41:22Z

So I've been thinking about this more and we have two possibilities I think:

First approach

To have the following styles in editor styles. Instead of asking theme authors to add this to their editor styles, we could potentially have a CSS transform to generate it from https://developer.wordpress.org/block-editor/developers/themes/theme-support/#changing-the-width-of-the-editor

It's tricky though and there might be things we'll be missing.

.wp-align-wrapper {
	max-width: $content-width;
}

.wp-align-wrapper > .wp-block,
.wp-align-wrapper.wp-align-full {
	max-width: none;
}

.wp-align-wrapper.wp-align-wide {
	max-width: 1100px;
}

Second approach

Apply the wp-block class to the alignment wrapper in the editor. I think it makes sense if we consider that the main/only use-case for this class (why it exists) is to serve the purpose of assigning block widths in different alignment situations. Now, when we do this should we remove it from the block wrapper Block.* too (so it needs to know about the existence of the alignment support, I wonder if it could even be responsible for the alignment support: adding the wrapper...)

What's left is a CSS transform for editor styles to match [data-align="full"] [data-align="wide"] to the new classNames, unless we also keep these data attribute for BC.

I'm leaning towards the second approach with Block.* responsible for the alignment wrapper addition. What do you think?

annezazu · 2021-08-25T16:45:48Z

Closing this PR out as the original issue referenced has closed:

#20650 (comment)

If that's incorrect, feel free to reopen!

Aligments: explore idea on image block

ffeb24d

ellatrix requested a review from youknowriad March 5, 2020 13:48

ellatrix requested review from ajitbohra, jorgefilipecosta, Soean and talldan as code owners March 5, 2020 13:48

youknowriad reviewed Mar 5, 2020

View reviewed changes

packages/block-library/src/image/edit.js Outdated Show resolved Hide resolved

youknowriad reviewed Mar 5, 2020

View reviewed changes

Add editor styles

68816aa

youknowriad reviewed Mar 5, 2020

View reviewed changes

This was referenced Mar 11, 2020

Try G2 #19344

Merged

Lighter InnerBlocks: allow blocks to render own wrappers #19910

Merged

ellatrix requested a review from ajlende as a code owner October 1, 2020 09:39

Base automatically changed from master to trunk March 1, 2021 15:43

annezazu closed this Aug 25, 2021

annezazu added the [Feature] Block API API that allows to express the block paradigm. label Aug 25, 2021

annezazu added the [Feature] Blocks Overall functionality of blocks label Aug 25, 2021

youknowriad deleted the try/image-improved-alignment branch August 25, 2021 18:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Aligments: explore idea on image block #20656

Aligments: explore idea on image block #20656

ellatrix commented Mar 5, 2020

github-actions bot commented Mar 5, 2020 •

edited

Loading

youknowriad left a comment

ellatrix commented Mar 5, 2020

youknowriad commented Mar 5, 2020

ellatrix commented Mar 5, 2020

ellatrix commented Mar 5, 2020 •

edited

Loading

youknowriad Mar 5, 2020

ellatrix Mar 5, 2020

youknowriad Mar 5, 2020

youknowriad Mar 5, 2020

ellatrix Mar 5, 2020

youknowriad Mar 5, 2020

youknowriad commented Mar 5, 2020 •

edited

Loading

youknowriad commented Mar 5, 2020 •

edited

Loading

youknowriad commented Mar 5, 2020

ellatrix commented Mar 5, 2020

youknowriad commented Mar 6, 2020

youknowriad commented Mar 6, 2020

annezazu commented Aug 25, 2021

Aligments: explore idea on image block #20656

Aligments: explore idea on image block #20656

Conversation

ellatrix commented Mar 5, 2020

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

github-actions bot commented Mar 5, 2020 • edited Loading

youknowriad left a comment

Choose a reason for hiding this comment

ellatrix commented Mar 5, 2020

youknowriad commented Mar 5, 2020

ellatrix commented Mar 5, 2020

ellatrix commented Mar 5, 2020 • edited Loading

youknowriad Mar 5, 2020

Choose a reason for hiding this comment

ellatrix Mar 5, 2020

Choose a reason for hiding this comment

youknowriad Mar 5, 2020

Choose a reason for hiding this comment

youknowriad Mar 5, 2020

Choose a reason for hiding this comment

ellatrix Mar 5, 2020

Choose a reason for hiding this comment

youknowriad Mar 5, 2020

Choose a reason for hiding this comment

youknowriad commented Mar 5, 2020 • edited Loading

youknowriad commented Mar 5, 2020 • edited Loading

youknowriad commented Mar 5, 2020

ellatrix commented Mar 5, 2020

youknowriad commented Mar 6, 2020

youknowriad commented Mar 6, 2020

annezazu commented Aug 25, 2021

github-actions bot commented Mar 5, 2020 •

edited

Loading

ellatrix commented Mar 5, 2020 •

edited

Loading

youknowriad commented Mar 5, 2020 •

edited

Loading

youknowriad commented Mar 5, 2020 •

edited

Loading