Tracking: Dimensions Design Tools Consistency

aaronrobertshaw commented 2 years ago

Overview

This issue details the current state of dimensions and spacing block support or design tool adoption across all blocks and tasks required to fill any gaps. Overall design tool consistency efforts are being tracked via the parent issue: https://github.com/WordPress/gutenberg/issues/43241.

Guidelines for Adopting Spacing Supports

We're generally happy to have padding & margin support for all blocks. (Exceptions possible)
We're leaning towards simple opt-in for margin support i.e. true rather than ["top", "bottom"]
Neither padding nor margin should be shown by default i.e. they're accessed via the ellipsis menu. (Exceptions possible)
All blocks (except special cases like Heading & Paragraph) should have box-sizing: border-box. This will be reviewed on a case-by-case basis. (Exceptions likely).

Legend

Value	Description
✅	Feature has been adopted and is displayed as a default control
✅ (Optional)	Feature has been adopted but is an optional control
⚠	There is a bug or issue with this block support feature's adoption
❌	Feature has been explicitly opted out of
<PR#>	Links to PR adopting the feature for this block
-	Feature has not explicitly been adopted/omitted
🛠	Implemented via an ad hoc / bespoke control
🚧	Work is in progress towards adopting this feature (no PR yet)

Block Support Adoption

Note: Deprecated blocks have been omitted from this table. e.g. Comment Author Avatar, Post Comment & Text Columns.

Block	Padding	Margin	Block Gap	Height	Width	Min Height	Aspect Ratio
Archives	✅ (Optional)	✅ (Optional)	-	-	-	-	-
Audio	✅ (Optional)	✅ (Optional)	-	-	-	-	-
Avatar	✅ (Optional)	✅ (Optional)	-	🛠	🛠	-	-
Button	✅ (Horizontal, Vertical)	-	-	-	🛠	-	-
Buttons	✅	✅ (Top, Bottom)	✅	-	-	-	-	-
Calendar	⚠ #43654 (Optional)	⚠ #43654 (Optional)	-	-	-	-	-
Categories	✅ (Optional)	✅ (Optional)	-	-	-	-	-
Code	✅ (Optional)	✅ (Optional / Top, Bottom)	-	-	-	-	-
Column	✅	-	✅	-	🛠	-	-
Columns	✅	✅ (Top, Bottom)	✅ (Horizontal, Vertical)	-	-	-	-
Comment Author Name	✅	✅	-	-	-	-	-
Comment Content	✅ (Horizontal, Vertical)	-	-	-	-	-	-
Comment Date	✅	✅	-	-	-	-
Comment Edit Link	✅	✅	-	-	-	-	-
Comment Reply Link	✅	✅	-	-	-	-	-
Comment Template	✅	✅	-	-	-	-	-
Comments	✅	✅	-	-	-	-	-
Comments Pagination	#43905	#43905	-	-	-	-	-
Comments Pagination Next	#43905	#43905	-	-	-	-	-
Comments Pagination Numbers	#43905	#43905	-	-	-	-	-
Comments Pagination Previous	#43905	#43905	-	-	-	-	-
Comments Title	✅	✅	-	-	-	-	-
Cover	✅	✅ (Top, Bottom)	✅	-	-	🛠	✅
Details	✅ (Optional)	✅ (Optional)	✅	-	-	-	-
Embed	-	✅	-	-	-	-	-
File	✅	✅	-	-	-	-	-
Footnotes	✅ (Optional)	✅ (Optional)	-	-	-	-	-
Gallery	✅ (Optional)	✅ (Optional)	✅(Horizontal, Vertical)	🛠	🛠	-	-
Group	✅	✅ (Top, Bottom)	✅	-	-	✅	-
Heading	✅ (Optional)	✅ (Optional)	-	-	-	-	-
Home Link - Navigation	-	-	-	-	-	-	-
HTML	-	-	-	-	-	-	-
Image	-	✅	-	🛠	🛠	-	🛠
Latest Comments	✅	✅	-	-	-	-	-
Latest Posts	✅	✅	-	-	-	-	-
List	✅ (Optional)	✅ (Optional)	-	-	-	-	-
List Item	✅ (Optional)	✅ (Optional)	-	-	-	-	-
Login/logout	✅ (Optional)	✅ (Optional)	-	-	-	-	-
Media & Text	✅	✅	-	-	-	-	-
More (Read More)	-	-	-	-	-	-	-
Navigation	https://github.com/WordPress/gutenberg/pull/66392/	https://github.com/WordPress/gutenberg/pull/66392/	✅	-	-	-	-
Navigation Link	-	-	-	-	-	-	-
Navigation Submenu	-	-	-	-	-	-	-
Next Page (Page Break)	-	-	-	-	-	-	-
Page List	https://github.com/WordPress/gutenberg/pull/66385	https://github.com/WordPress/gutenberg/pull/66385		-	-	-	-
Paragraph	✅ (Optional)	✅ (Optional)	-	-	-	-	-
Post Author	✅	✅	-	🛠	🛠	-	-
Post Author Biography	✅	✅	-	-	-	-	-
Post Author Name	✅	✅	-	-	-	-	-
Post Comments Count	✅	✅	-	-	-	-	-
Post Comments Form	✅	✅	-	-	-	-	-
Post Comments Link	✅	✅	-	-	-	-	-
Post Content	✅ (Optional)	https://github.com/WordPress/gutenberg/pull/66366	✅	-	-	✅	-
Post Date	✅	✅	-	-	-	-	-
Post Excerpt	✅	✅	-	-	-	-	-
Post Featured Image	✅	✅	-	🛠	🛠	-	🛠
Post Navigation Link	https://github.com/WordPress/gutenberg/pull/64258	https://github.com/WordPress/gutenberg/pull/64258	-	-	-	-	-
Post Template	-	-	✅	-	-	-	-
Post Terms	✅	✅	-	-	-	-	-
Post Title	✅	✅	-	-	-	-	-
Preformatted	✅	✅	-	-	-	-	-
Pullquote	✅	✅	-	-	-	✅ (Optional)	-
Query	🚫	🚫	🚫	🚫	🚫	🚫	-
Query No Results	https://github.com/WordPress/gutenberg/pull/64601	https://github.com/WordPress/gutenberg/pull/64601	-	-	-	-	-
Query Pagination	https://github.com/WordPress/gutenberg/pull/66398	https://github.com/WordPress/gutenberg/pull/66398	-	-	-	-	-
Query Pagination Next	https://github.com/WordPress/gutenberg/pull/66394	https://github.com/WordPress/gutenberg/pull/66394	-	-	-	-	-
Query Pagination Numbers	https://github.com/WordPress/gutenberg/pull/66401	https://github.com/WordPress/gutenberg/pull/66401	-	-	-	-	-
Query Pagination Previous	https://github.com/WordPress/gutenberg/pull/66393	https://github.com/WordPress/gutenberg/pull/66393	-	-	-	-	-
Query Title	✅	✅	-	-	-	-	-
Quote	✅	✅	✅	-	-	✅ (Optional)	-
Read More	✅	✅ (Top, Bottom)	-	-	-	-	-
RSS	https://github.com/WordPress/gutenberg/pull/66411	https://github.com/WordPress/gutenberg/pull/66411	-	-	-	-	-
Search	-	✅	-	-	🛠	-	-
Separator	-	✅ (Top, Bottom)	-	-	-	-	-
Shortcode	🚫	🚫	🚫	🚫	🚫	🚫	🚫
Site Logo	✅ (Optional)	✅ (Optional)	-	-	-	-	-
Site Tagline	✅ (Optional)	✅ (Optional)	-	-	-	-	-
Site Title	✅ (Optional)	✅ (Optional)	-	-	-	-	-
Social Link	#63782	#63782	-	-	-	-	-
Social Links	✅ (Optional)	✅	✅ (Horizontal, Vertical)	-	-	-	-
Spacer	✅	✅ (Top, Bottom)	-	🛠	🛠	-	-
Table	✅ (Optional)	✅ (Optional)	-	-	-	-	-
Table of Contents	✅	✅	-	-	-	-	-
Tag Cloud	✅	✅	-	-	-	-	-
Term Description	✅	✅	-	-	-	-	-
Time To Read	✅ (Optional)	✅ (Optional)	-	-	-	-	-
Verse	✅ (Optional)	✅ (Optional)	-	-	-	✅ (Optional)	-
Video	✅ (Optional)	✅ (Optional)	-	-	-	-	-

Known Issues

Several blocks adopting padding support will also probably need box-sizing: border-box added to their styles. A sweep should be completed before closing this issue.
Before the Post Template block can fully adopt spacing supports it may need a bit of a refactor: https://github.com/WordPress/gutenberg/issues/44557

Merged PRs

The following list details all the PRs merged as part of this effort to increase spacing support.

PRs with pending questions, discussions, or concerns

Possible Follow-Ups

t-hamano commented 2 years ago

I think we need to be careful about blocks that are server-side rendered. The following are blocks that are server-side rendered, some of which are already supported by Dimension.

core/calendar
core/tag-cloud: #43367
core/archive: #43350
core/latest-comments
core/rss

The problem is that the hooks (useBlockProps) and server-side rendering cause styles to be applied twice. The tag cloud example renders the following on the editor side:

serversiderender

Simply excluding attributes from the ServerSideRender component would make dynamic rendering based on attributes impossible.

To address this I would like to offer three solutions. Or, are there any other solutions that would be appropriate?

1. Skip serialization

{
    "supports": {
        "spacing": {
            "__experimentalSkipSerialization": true,
            "margin": true,
            "padding": true
        }
    }
}

In this case, inline padding/margin styles are not automatically generated, so they must be generated explicitly in a callback function, as the search block does.

2. Omit padding / margin from attributes of `ServerSideRender`

const serverSideAttributes = {
    ...attributes,
    style: omit( attributes.style, [ 'spacing' ] ),
};

I think this would be a good approach to prevent duplication of custom CSS classes, as discussed in #43653.

3. Refactor without the `ServiersideRender` component

While this may be the ideal approach, it may be a bit more difficult with respect to the calendar block, since it uses the core get_calendar function internally. However, for the rest of the blocks, I believe it should be possible to rewrite them using the API.

If this approach is ideal for us, I would be happy to help.

andrewserong commented 2 years ago

Thanks for flagging the issue and laying out the different options @t-hamano!

Another potential option is that for blocks that are using the ServerSideRender component in its edit view, would be to try to strip the style key from ...useBlockProps() in the rendering of the wrapper. That way for blocks that use the server rendering in the edit view, we can defer to that server rendered version for all style output without needing to use the skip serialization option.

Do you think something like that might work?

t-hamano commented 2 years ago

Another potential option is that for blocks that are using the ServerSideRender component in its edit view, would be to try to strip the style key from ...useBlockProps() in the rendering of the wrapper. That way for blocks that use the server rendering in the edit view, we can defer to that server rendered version for all style output without needing to use the skip serialization option.

Yes. I checked with you about #43497 that you mentioned in this comment, and I think that approach makes sense. However, I think only the margins should be applied to the block wrapper (useBlockProps), not rendered on the server side.

I think the block support margin is expected to override the block wrapper margin, so they would not work correctly when rendered server-side.

Here is my approach as I see it:

// Use only margin support in the block wrapper style.
const blockStyles = {
    ...pickBy( blockProps.style, ( value, key ) => key.match( /^margin/ ) ),
};

// Don't pass margin attribute to ServerSideRender component.
const serverSideRenderAttributes = {
    ...attributes,
    style: {
        ...attributes.style,
        spacing: { ...omit( attributes.style.spacing, [ 'margin' ] ) },
    },
};

return (
    <div { ...blockProps } style={ blockStyles }>
        <ServerSideRender
            key="tag-cloud"
            block="core/tag-cloud"
            attributes={ serverSideRenderAttributes }
        />
    </div>
);

I believe this approach has two advantages:

No need to define skip serialization in block.json
The ServerSideRender component receives all attributes except margin. Therefore, even block supports are added, no update to edit.js is required, only an update to the callback function.

~~The only concern is that it is complicated to handle the various styles individually in the callback functions 😅~~ This may have been misleading. Margins and padding styles are applied as inline-style, so get_block_wrapper_attributes will take care of them automatically, except when they are needed to be apply to elements inside of the wrapper div.

andrewserong commented 2 years ago

Very interesting thoughts @t-hamano! This is a tricky one. Ideally to best represent what happens on the site frontend, I suppose all the inline styles would be applied to the same element that exists on the site frontend. For example, in the editor, the Tag Cloud block is rendered as a p tag within a wrapper div:

However, I think only the margins should be applied to the block wrapper (useBlockProps), not rendered on the server side.

But, that's a very good point. In order to work properly with overriding layout styles, it does seem that we'd need to have margin be output on the wrapper div in the editor, since it doesn't look like it'd be easy to remove the extra wrapper.

From my perspective, I think including some extra logic in the block's edit.js to ensure proper rendering is an acceptable trade-off of all the options. It does mean that instead of a single one-size-fits-all approach, we'll need to assess the impact on a case-by-case basis for each block that uses ServerSideRender, since as you mention, some blocks might use margins and paddings in unique ways. But because ServerSideRender is already an odd sort of workaround for rendering blocks within the editor, I think the extra code in your example sounds like a promising direction to explore.

t-hamano commented 2 years ago

It does mean that instead of a single one-size-fits-all approach, we'll need to assess the impact on a case-by-case basis for each block that uses ServerSideRender, since as you mention, some blocks might use margins and paddings in unique ways.

Yes, that makes sense. Let's deal with them individually, along with blocks that have already been merged with block support.

Another thing to discuss is how to handle the issue of additional class(es) being rendered as duplicates, as @ndiego submitted in #43653.

I think we have three options:

Apply only to the wrapper div.
Apply only to server-side rendered elements.
Both. In other words, we don't care duplicate additional classes.

The second plan would be appropriate if we want to respect the match with the front end side. However, since the core class of blocks (wp-block-XXX) is already duplicated, we believe a third plan is also possible.

If these directions are solidified, I think it might be a good idea to first proceed with the review of #43653 👍

aaronrobertshaw commented 2 years ago

An overall update on progress towards design tools consistency has been added to the primary tracking issue, including our goals for WordPress 6.1.

carolinan commented 2 years ago

I suggest not adding spacing to the more block which uses the  tag and does not use any block props on the front.

andrewserong commented 2 years ago

Small update: as of https://github.com/WordPress/gutenberg/pull/45300 there is now a Min Height dimensions block support that we can opt into. That PR has opted in both the Group and Post Content blocks, and I've updated the table in the issue description. @jasmussen flagged on the PR (here: https://github.com/WordPress/gutenberg/pull/45300#issuecomment-1298252520) that when we use the Min Height block support, it usually shouldn't be a default control (that is, it should be optional and only exposed via the drop-down menu). There is also a follow-up tracking issue in https://github.com/WordPress/gutenberg/issues/45501 to look at tweaks and enhancements for the UI of the Min Height controls.

t-hamano commented 1 year ago

I have updated the table cell because I found #43243 which adds margins to the Embed block.

annezazu commented 1 year ago

Noting this heavily requested negative margins feature request as potentially that could be addressed as part of this wider work: https://github.com/WordPress/gutenberg/issues/32644

aaronrobertshaw commented 1 year ago

Thanks for flagging that issue @annezazu 👍

It is something that has been explored previously (https://github.com/WordPress/gutenberg/pull/40464), however, there were a few blockers at the time, and more recently, there was an issue with layout support styles overriding the margin block support styles. That last issue was a blocker to adopting the design tools here as well.

Supporting negative margins will likely require some design work to ensure we come up with a nice UX. So while I'm 100% on board with us supporting negative margins in the near future. I think it might be better to keep it separate to the efforts adopting dimensions and spacing design tools as they have a knack for hitting enough roadblocks as is 🙂

@andrewserong's comment on the negative margin feature request issue goes into more detail on what we need to address and provides extra context.

t-hamano commented 1 year ago

Update: I have added the Footnotes and Details blocks to the list.

t-hamano commented 3 months ago

Update: I have updated the table with the latest specs.

At the same time, I also added the minHeight and units support column. We need to also consider whether we can add this support, except for blocks that already have it and blocks that clearly do not need it.

Something I noticed while working on this is that even if I don't explicitly define support in __experimentalDefaultControls, the control will be displayed by default.

For example, the following definition shows the padding, margin, block gap and min-height controls by default:

{
    "supports": {
        "dimensions": {
            "minHeight": true
        },
        "spacing": {
            "margin": true,
            "padding": true,
            "blockGap": true
        }
    }
}

This behavior is different from the Typography support. I don't know if this is an original specification or some kind of bug 🤔

aaronrobertshaw commented 3 months ago

Thanks again @t-hamano for all the effort in keeping these issues up-to-date.

I don't think the units support is intended for wide-spread adoption, or at least might be best dealt with separately to these efforts. Maybe remove it? What do you think?

This behavior is different from the Typography support. I don't know if this is an original specification or some kind of bug

These block supports pre-existed the ability to define which controls display by default. I believe the current behaviour is for backwards compatibility. Perhaps creating an issue to address and discuss this further would be a good idea?

P.S. Can you update the merged PRs list with any links for the blocks you updated?

t-hamano commented 3 months ago

I don't think the units support is intended for wide-spread adoption, or at least might be best dealt with separately to these efforts. Maybe remove it? What do you think?

I also have no strong opinion on adding units support to this PR, so Il remove the units column.

These block supports pre-existed the ability to define which controls display by default. I believe the current behaviour is for backwards compatibility. Perhaps creating an issue to address and discuss this further would be a good idea?

Yes, maybe it can be addressed after this issue is completed.

WordPress / gutenberg