Skip to content
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.

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

Block Support: Add gap block support feature #33991

Merged
merged 20 commits into from
Sep 2, 2021

Conversation

andrewserong
Copy link
Contributor

@andrewserong andrewserong commented Aug 11, 2021

Description

Related to: #32366
Fixes: #29098

Note: this PR supersedes #32571 — it is essentially the same PR, but now based off trunk

This PR is a follow-up to the gap CSS variable approach for the layout support added in #33812. It proposes adding in a Gap block support feature for blocks that could use a Gap spacing control. Potential candidates for using this include:

  • Buttons block, to control spacing between individual buttons
  • Navigation block to control spacing between nav items
  • Columns to control spacing between columns
  • Longer-term: possibly the Gallery block

To simplify things for this PR, we generate a single CSS variable (--wp--style--block-gap) for consistency with #33812. The idea at the moment is that we introduce a simple uniform gap control, and if we need to split out horizontal/vertical gap spacing, we can look into that further down the track / in follow-ups.

The CSS variable is generated via server-rendering and injecting the style inline into rendered blocks. The reason for this is because arbitrary CSS variables are not able to be updated in post content by users that don't have the unfiltered_html capability (e.g. Author and Contributor roles). This is a similar kind of approach as to the Layout support. Additionally, styles are rendered inline instead of via a separate style element so that the style will be rendered correctly in the context of post-content blocks in FSE.

To-dos

How has this been tested?

PHP tests:

npm run test-unit-php /var/www/html/wp-content/plugins/gutenberg/phpunit/block-supports/spacing-test.php

Switch on the block gap support

  1. Using TT1-Blocks theme, In your theme.json file, under settings.spacing add the "blockGap": true property. E.g. this is how my spacing settings look:
		"spacing": {
			"blockGap": true,
			"customPadding": true,
			"units": [
				"px",
				"em",
				"rem",
				"vh",
				"vw"
			]
		},
  1. In packages/block-library/src/buttons/block.json add the gap "spacing" support. In my testing, the "supports" key of this file looks like this:
	"supports": {
		"anchor": true,
		"align": [ "wide", "full" ],
		"spacing": {
			"blockGap": true,
			"__experimentalDefaultControls": {
				"blockGap": true
			}
		}
	},
  1. In packages/block-library/src/buttons/style.scss at line 8 add the following to set the gap styles:
	column-gap: var( --wp--style--block-gap, $blocks-block__margin );
	row-gap: var( --wp--style--block-gap, $blocks-block__margin );
  1. Insert a Buttons block and a few buttons into a post, and try out the Gap control as in the screenshots below.

Testing applying styles via theme.json

In a theme.json file you can specify gap styles in the following way (in this example, applied to the buttons block):

Using the blockGap property

In the theme.json file, use:

	"styles": {
		...
		"blocks": {
			...
			"core/buttons": {
				"spacing": {
					"blockGap": "4em"
				}
			},
		}
	}

This should result in the following rendered styles on the front-end of the site:

.wp-block-buttons {
	--wp--style--block-gap: 4em;
}

Screenshots

This PR only adds the gap block support feature, and doesn't switch it on for any blocks by default, but the following screenshots are how it looks if applied to the Buttons block:

block-gap-support-sml

In Global styles:

block-gap-support-global-styles-sml

Note that due to the Layout support also using the same CSS variable, in the above global styles screenshot, we can see the changed value affects the block's margin, too. This could be something for us to look into when applying the support to individual blocks.

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • 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 (please manually search all *.native.js files for terms that need renaming or removal).

@andrewserong andrewserong added the [Status] In Progress Tracking issues with work in progress label Aug 11, 2021
@andrewserong andrewserong self-assigned this Aug 11, 2021
@andrewserong andrewserong changed the title Add gap block support feature based on #32571 Block Support: Add gap block support feature Aug 11, 2021
@github-actions
Copy link

github-actions bot commented Aug 11, 2021

Size Change: +611 B (0%)

Total Size: 1.04 MB

Filename Size Change
build/block-editor/index.min.js 119 kB +156 B (0%)
build/block-library/blocks/navigation-link/editor-rtl.css 473 B +3 B (+1%)
build/block-library/blocks/navigation-link/editor.css 473 B +3 B (+1%)
build/block-library/blocks/navigation/editor-rtl.css 1.7 kB +3 B (0%)
build/block-library/blocks/navigation/editor.css 1.71 kB +5 B (0%)
build/block-library/editor-rtl.css 9.94 kB +9 B (0%)
build/block-library/editor.css 9.92 kB +8 B (0%)
build/block-library/index.min.js 150 kB +230 B (0%)
build/blocks/index.min.js 47 kB +23 B (0%)
build/components/index.min.js 209 kB +12 B (0%)
build/core-data/index.min.js 12.4 kB -28 B (0%)
build/edit-navigation/index.min.js 13.6 kB +44 B (0%)
build/edit-navigation/style-rtl.css 3.14 kB -1 B (0%)
build/edit-navigation/style.css 3.14 kB -2 B (0%)
build/edit-site/index.min.js 26.4 kB +146 B (+1%)
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 931 B
build/admin-manifest/index.min.js 1.09 kB
build/annotations/index.min.js 2.7 kB
build/api-fetch/index.min.js 2.19 kB
build/autop/index.min.js 2.08 kB
build/blob/index.min.js 459 B
build/block-directory/index.min.js 6.2 kB
build/block-directory/style-rtl.css 1.01 kB
build/block-directory/style.css 1.01 kB
build/block-editor/style-rtl.css 13.8 kB
build/block-editor/style.css 13.8 kB
build/block-library/blocks/archives/editor-rtl.css 61 B
build/block-library/blocks/archives/editor.css 60 B
build/block-library/blocks/archives/style-rtl.css 65 B
build/block-library/blocks/archives/style.css 65 B
build/block-library/blocks/audio/editor-rtl.css 58 B
build/block-library/blocks/audio/editor.css 58 B
build/block-library/blocks/audio/style-rtl.css 111 B
build/block-library/blocks/audio/style.css 111 B
build/block-library/blocks/audio/theme-rtl.css 125 B
build/block-library/blocks/audio/theme.css 125 B
build/block-library/blocks/block/editor-rtl.css 161 B
build/block-library/blocks/block/editor.css 161 B
build/block-library/blocks/button/editor-rtl.css 474 B
build/block-library/blocks/button/editor.css 474 B
build/block-library/blocks/button/style-rtl.css 605 B
build/block-library/blocks/button/style.css 604 B
build/block-library/blocks/buttons/editor-rtl.css 315 B
build/block-library/blocks/buttons/editor.css 315 B
build/block-library/blocks/buttons/style-rtl.css 370 B
build/block-library/blocks/buttons/style.css 370 B
build/block-library/blocks/calendar/style-rtl.css 207 B
build/block-library/blocks/calendar/style.css 207 B
build/block-library/blocks/categories/editor-rtl.css 84 B
build/block-library/blocks/categories/editor.css 83 B
build/block-library/blocks/categories/style-rtl.css 79 B
build/block-library/blocks/categories/style.css 79 B
build/block-library/blocks/code/style-rtl.css 90 B
build/block-library/blocks/code/style.css 90 B
build/block-library/blocks/code/theme-rtl.css 131 B
build/block-library/blocks/code/theme.css 131 B
build/block-library/blocks/columns/editor-rtl.css 194 B
build/block-library/blocks/columns/editor.css 193 B
build/block-library/blocks/columns/style-rtl.css 474 B
build/block-library/blocks/columns/style.css 475 B
build/block-library/blocks/cover/editor-rtl.css 666 B
build/block-library/blocks/cover/editor.css 670 B
build/block-library/blocks/cover/style-rtl.css 1.23 kB
build/block-library/blocks/cover/style.css 1.23 kB
build/block-library/blocks/embed/editor-rtl.css 488 B
build/block-library/blocks/embed/editor.css 488 B
build/block-library/blocks/embed/style-rtl.css 400 B
build/block-library/blocks/embed/style.css 400 B
build/block-library/blocks/embed/theme-rtl.css 124 B
build/block-library/blocks/embed/theme.css 124 B
build/block-library/blocks/file/editor-rtl.css 300 B
build/block-library/blocks/file/editor.css 300 B
build/block-library/blocks/file/style-rtl.css 255 B
build/block-library/blocks/file/style.css 255 B
build/block-library/blocks/file/view.min.js 322 B
build/block-library/blocks/freeform/editor-rtl.css 2.44 kB
build/block-library/blocks/freeform/editor.css 2.44 kB
build/block-library/blocks/gallery/editor-rtl.css 879 B
build/block-library/blocks/gallery/editor.css 876 B
build/block-library/blocks/gallery/style-rtl.css 1.7 kB
build/block-library/blocks/gallery/style.css 1.7 kB
build/block-library/blocks/gallery/theme-rtl.css 122 B
build/block-library/blocks/gallery/theme.css 122 B
build/block-library/blocks/group/editor-rtl.css 159 B
build/block-library/blocks/group/editor.css 159 B
build/block-library/blocks/group/style-rtl.css 57 B
build/block-library/blocks/group/style.css 57 B
build/block-library/blocks/group/theme-rtl.css 70 B
build/block-library/blocks/group/theme.css 70 B
build/block-library/blocks/heading/editor-rtl.css 113 B
build/block-library/blocks/heading/editor.css 113 B
build/block-library/blocks/heading/style-rtl.css 76 B
build/block-library/blocks/heading/style.css 76 B
build/block-library/blocks/home-link/style-rtl.css 247 B
build/block-library/blocks/home-link/style.css 247 B
build/block-library/blocks/html/editor-rtl.css 283 B
build/block-library/blocks/html/editor.css 284 B
build/block-library/blocks/image/editor-rtl.css 728 B
build/block-library/blocks/image/editor.css 728 B
build/block-library/blocks/image/style-rtl.css 482 B
build/block-library/blocks/image/style.css 487 B
build/block-library/blocks/image/theme-rtl.css 124 B
build/block-library/blocks/image/theme.css 124 B
build/block-library/blocks/latest-comments/style-rtl.css 284 B
build/block-library/blocks/latest-comments/style.css 284 B
build/block-library/blocks/latest-posts/editor-rtl.css 137 B
build/block-library/blocks/latest-posts/editor.css 137 B
build/block-library/blocks/latest-posts/style-rtl.css 528 B
build/block-library/blocks/latest-posts/style.css 527 B
build/block-library/blocks/list/style-rtl.css 63 B
build/block-library/blocks/list/style.css 63 B
build/block-library/blocks/media-text/editor-rtl.css 266 B
build/block-library/blocks/media-text/editor.css 263 B
build/block-library/blocks/media-text/style-rtl.css 488 B
build/block-library/blocks/media-text/style.css 485 B
build/block-library/blocks/more/editor-rtl.css 431 B
build/block-library/blocks/more/editor.css 431 B
build/block-library/blocks/navigation-link/style-rtl.css 94 B
build/block-library/blocks/navigation-link/style.css 94 B
build/block-library/blocks/navigation/style-rtl.css 1.44 kB
build/block-library/blocks/navigation/style.css 1.44 kB
build/block-library/blocks/navigation/view.min.js 2.52 kB
build/block-library/blocks/nextpage/editor-rtl.css 395 B
build/block-library/blocks/nextpage/editor.css 395 B
build/block-library/blocks/page-list/editor-rtl.css 310 B
build/block-library/blocks/page-list/editor.css 310 B
build/block-library/blocks/page-list/style-rtl.css 241 B
build/block-library/blocks/page-list/style.css 241 B
build/block-library/blocks/paragraph/editor-rtl.css 157 B
build/block-library/blocks/paragraph/editor.css 157 B
build/block-library/blocks/paragraph/style-rtl.css 248 B
build/block-library/blocks/paragraph/style.css 248 B
build/block-library/blocks/post-author/editor-rtl.css 210 B
build/block-library/blocks/post-author/editor.css 210 B
build/block-library/blocks/post-author/style-rtl.css 182 B
build/block-library/blocks/post-author/style.css 181 B
build/block-library/blocks/post-comments-form/style-rtl.css 140 B
build/block-library/blocks/post-comments-form/style.css 140 B
build/block-library/blocks/post-comments/style-rtl.css 360 B
build/block-library/blocks/post-comments/style.css 359 B
build/block-library/blocks/post-content/editor-rtl.css 138 B
build/block-library/blocks/post-content/editor.css 138 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B
build/block-library/blocks/post-excerpt/editor.css 73 B
build/block-library/blocks/post-excerpt/style-rtl.css 69 B
build/block-library/blocks/post-excerpt/style.css 69 B
build/block-library/blocks/post-featured-image/editor-rtl.css 398 B
build/block-library/blocks/post-featured-image/editor.css 398 B
build/block-library/blocks/post-featured-image/style-rtl.css 143 B
build/block-library/blocks/post-featured-image/style.css 143 B
build/block-library/blocks/post-template/editor-rtl.css 99 B
build/block-library/blocks/post-template/editor.css 98 B
build/block-library/blocks/post-template/style-rtl.css 378 B
build/block-library/blocks/post-template/style.css 379 B
build/block-library/blocks/post-terms/style-rtl.css 73 B
build/block-library/blocks/post-terms/style.css 73 B
build/block-library/blocks/post-title/style-rtl.css 60 B
build/block-library/blocks/post-title/style.css 60 B
build/block-library/blocks/preformatted/style-rtl.css 103 B
build/block-library/blocks/preformatted/style.css 103 B
build/block-library/blocks/pullquote/editor-rtl.css 198 B
build/block-library/blocks/pullquote/editor.css 198 B
build/block-library/blocks/pullquote/style-rtl.css 361 B
build/block-library/blocks/pullquote/style.css 360 B
build/block-library/blocks/pullquote/theme-rtl.css 167 B
build/block-library/blocks/pullquote/theme.css 167 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B
build/block-library/blocks/query-pagination/editor-rtl.css 270 B
build/block-library/blocks/query-pagination/editor.css 262 B
build/block-library/blocks/query-pagination/style-rtl.css 168 B
build/block-library/blocks/query-pagination/style.css 168 B
build/block-library/blocks/query-title/editor-rtl.css 85 B
build/block-library/blocks/query-title/editor.css 85 B
build/block-library/blocks/query/editor-rtl.css 131 B
build/block-library/blocks/query/editor.css 132 B
build/block-library/blocks/quote/style-rtl.css 169 B
build/block-library/blocks/quote/style.css 169 B
build/block-library/blocks/quote/theme-rtl.css 220 B
build/block-library/blocks/quote/theme.css 222 B
build/block-library/blocks/rss/editor-rtl.css 202 B
build/block-library/blocks/rss/editor.css 204 B
build/block-library/blocks/rss/style-rtl.css 289 B
build/block-library/blocks/rss/style.css 288 B
build/block-library/blocks/search/editor-rtl.css 165 B
build/block-library/blocks/search/editor.css 165 B
build/block-library/blocks/search/style-rtl.css 374 B
build/block-library/blocks/search/style.css 375 B
build/block-library/blocks/search/theme-rtl.css 64 B
build/block-library/blocks/search/theme.css 64 B
build/block-library/blocks/separator/editor-rtl.css 99 B
build/block-library/blocks/separator/editor.css 99 B
build/block-library/blocks/separator/style-rtl.css 250 B
build/block-library/blocks/separator/style.css 250 B
build/block-library/blocks/separator/theme-rtl.css 172 B
build/block-library/blocks/separator/theme.css 172 B
build/block-library/blocks/shortcode/editor-rtl.css 474 B
build/block-library/blocks/shortcode/editor.css 474 B
build/block-library/blocks/site-logo/editor-rtl.css 462 B
build/block-library/blocks/site-logo/editor.css 464 B
build/block-library/blocks/site-logo/style-rtl.css 153 B
build/block-library/blocks/site-logo/style.css 153 B
build/block-library/blocks/site-tagline/editor-rtl.css 86 B
build/block-library/blocks/site-tagline/editor.css 86 B
build/block-library/blocks/site-title/editor-rtl.css 84 B
build/block-library/blocks/site-title/editor.css 84 B
build/block-library/blocks/social-link/editor-rtl.css 165 B
build/block-library/blocks/social-link/editor.css 165 B
build/block-library/blocks/social-links/editor-rtl.css 812 B
build/block-library/blocks/social-links/editor.css 811 B
build/block-library/blocks/social-links/style-rtl.css 1.33 kB
build/block-library/blocks/social-links/style.css 1.33 kB
build/block-library/blocks/spacer/editor-rtl.css 307 B
build/block-library/blocks/spacer/editor.css 307 B
build/block-library/blocks/spacer/style-rtl.css 48 B
build/block-library/blocks/spacer/style.css 48 B
build/block-library/blocks/table/editor-rtl.css 471 B
build/block-library/blocks/table/editor.css 472 B
build/block-library/blocks/table/style-rtl.css 481 B
build/block-library/blocks/table/style.css 481 B
build/block-library/blocks/table/theme-rtl.css 188 B
build/block-library/blocks/table/theme.css 188 B
build/block-library/blocks/tag-cloud/style-rtl.css 146 B
build/block-library/blocks/tag-cloud/style.css 146 B
build/block-library/blocks/template-part/editor-rtl.css 636 B
build/block-library/blocks/template-part/editor.css 635 B
build/block-library/blocks/template-part/theme-rtl.css 101 B
build/block-library/blocks/template-part/theme.css 101 B
build/block-library/blocks/term-description/editor-rtl.css 90 B
build/block-library/blocks/term-description/editor.css 90 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B
build/block-library/blocks/text-columns/editor.css 95 B
build/block-library/blocks/text-columns/style-rtl.css 166 B
build/block-library/blocks/text-columns/style.css 166 B
build/block-library/blocks/verse/style-rtl.css 87 B
build/block-library/blocks/verse/style.css 87 B
build/block-library/blocks/video/editor-rtl.css 571 B
build/block-library/blocks/video/editor.css 572 B
build/block-library/blocks/video/style-rtl.css 173 B
build/block-library/blocks/video/style.css 173 B
build/block-library/blocks/video/theme-rtl.css 124 B
build/block-library/blocks/video/theme.css 124 B
build/block-library/common-rtl.css 1.29 kB
build/block-library/common.css 1.29 kB
build/block-library/reset-rtl.css 527 B
build/block-library/reset.css 527 B
build/block-library/style-rtl.css 10.7 kB
build/block-library/style.css 10.7 kB
build/block-library/theme-rtl.css 658 B
build/block-library/theme.css 663 B
build/block-serialization-default-parser/index.min.js 1.09 kB
build/block-serialization-spec-parser/index.min.js 2.79 kB
build/components/style-rtl.css 15.8 kB
build/components/style.css 15.8 kB
build/compose/index.min.js 10.2 kB
build/customize-widgets/index.min.js 11.1 kB
build/customize-widgets/style-rtl.css 1.5 kB
build/customize-widgets/style.css 1.49 kB
build/data-controls/index.min.js 614 B
build/data/index.min.js 7.1 kB
build/date/index.min.js 31.5 kB
build/deprecated/index.min.js 428 B
build/dom-ready/index.min.js 304 B
build/dom/index.min.js 4.53 kB
build/edit-post/classic-rtl.css 492 B
build/edit-post/classic.css 494 B
build/edit-post/index.min.js 28.8 kB
build/edit-post/style-rtl.css 7.2 kB
build/edit-post/style.css 7.19 kB
build/edit-site/style-rtl.css 5.07 kB
build/edit-site/style.css 5.07 kB
build/edit-widgets/index.min.js 16 kB
build/edit-widgets/style-rtl.css 4.06 kB
build/edit-widgets/style.css 4.06 kB
build/editor/index.min.js 37.6 kB
build/editor/style-rtl.css 3.74 kB
build/editor/style.css 3.73 kB
build/element/index.min.js 3.17 kB
build/escape-html/index.min.js 517 B
build/format-library/index.min.js 5.36 kB
build/format-library/style-rtl.css 668 B
build/format-library/style.css 669 B
build/hooks/index.min.js 1.55 kB
build/html-entities/index.min.js 424 B
build/i18n/index.min.js 3.59 kB
build/is-shallow-equal/index.min.js 501 B
build/keyboard-shortcuts/index.min.js 1.49 kB
build/keycodes/index.min.js 1.25 kB
build/list-reusable-blocks/index.min.js 1.85 kB
build/list-reusable-blocks/style-rtl.css 838 B
build/list-reusable-blocks/style.css 838 B
build/media-utils/index.min.js 2.88 kB
build/notices/index.min.js 845 B
build/nux/index.min.js 2.03 kB
build/nux/style-rtl.css 747 B
build/nux/style.css 743 B
build/plugins/index.min.js 1.83 kB
build/primitives/index.min.js 921 B
build/priority-queue/index.min.js 582 B
build/react-i18n/index.min.js 671 B
build/redux-routine/index.min.js 2.63 kB
build/reusable-blocks/index.min.js 2.28 kB
build/reusable-blocks/style-rtl.css 256 B
build/reusable-blocks/style.css 256 B
build/rich-text/index.min.js 10.6 kB
build/server-side-render/index.min.js 1.32 kB
build/shortcode/index.min.js 1.48 kB
build/token-list/index.min.js 562 B
build/url/index.min.js 1.74 kB
build/viewport/index.min.js 1.02 kB
build/warning/index.min.js 248 B
build/widgets/index.min.js 6.27 kB
build/widgets/style-rtl.css 1.05 kB
build/widgets/style.css 1.05 kB
build/wordcount/index.min.js 1.04 kB

compressed-size-action

@andrewserong andrewserong changed the title Block Support: Add gap block support feature [WIP] Block Support: Add gap block support feature Aug 11, 2021
@andrewserong
Copy link
Contributor Author

Based on feedback in #33812 (comment), I've refactored this to use a single CSS variable (--wp--style--block-gap) and removed the split horizontal / vertical values and controls (we can always explore this further in follow-ups). I've also updated the naming of the support to blockGap and customBlockGap for consistency with that PR.

Next up, I'll look into server-rendering the value instead of storing in inline styles. Just a note here that this is still in progress, I'll update the PR description to match the current state of things later on today.

@andrewserong
Copy link
Contributor Author

Added in server-rendering in 3fea1eb along with skipping the serialization of the blockGap value in post content, which seems like it's going to work to get around issues with storing CSS variables in post content. So it seems promising so far! There are some outstanding issues with the approach I've gone with, which I'll look at cleaning up tomorrow along with adding more tests.

@andrewserong andrewserong force-pushed the add/gap-block-support-feature branch 2 times, most recently from 474e5fc to c3f2081 Compare August 16, 2021 02:48
@andrewserong
Copy link
Contributor Author

Just noticed another bug to do with the difference between rendering within the editor and rendering server-side: it appears that the Edit component's inline styles aren't being rendered in the site editor. I'll take a look at what's going on.

@andrewserong
Copy link
Contributor Author

inline styles aren't being rendered in the site editor.

This issue is to do with rendering the gap support styles within the post-content block, which takes the rendered markup from the API response for the post, and renders it in a RawHTML component. In this context, the styles generated and rendered in wp_footer are never included, so the block-level styling isn't preserved:

Gap setting while editing post The post rendered in a post-content block
image image

An option to explore is to render the style tag in situ, instead of in the footer. I'll take a look at that tomorrow.

@andrewserong
Copy link
Contributor Author

I've had a go at server-rendering an inline style in 842bbfe to resolve the issue of rendering in the post-content block, and it seems to simplify things a bit, too. It seems promising to me so far, but since it's regex based, it might do with some testing. I'll keep chipping away tomorrow.

@andrewserong
Copy link
Contributor Author

I've updated this PR to tweak the regex slightly and add in some additional PHP tests. I've also removed gap from the function that applies the spacing support for dynamic blocks, as this gets handled anyway by the render_block callback for all blocks.

I think once #33812 lands, this PR should be in a good place for reviews / testing. I'll continue to update this PR if and when that PR lands to remove the duplication that gets this PR working.

@andrewserong andrewserong changed the title [WIP] Block Support: Add gap block support feature Block Support: Add gap block support feature Aug 18, 2021
Copy link
Contributor

@stacimc stacimc left a comment

Choose a reason for hiding this comment

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

I haven't tested this on top of trunk yet, but this is looking great!

  • When configured, Gap settings appear by default in the Dimensions panel
  • Tested changing the Gap value from the Dimensions panel
  • Tested setting a Gap through theme.json
  • Tested setting a Gap through global styles
  • Gap values set per-block override global styles, which override theme.json
  • Gap styles reflected correctly on the frontend, site editor, and block editor
  • Tested that Gap works with different alignment and items justification values

The only slightly odd things I noticed are just Buttons-specific stuff and would be handled when adding the support to that block (the existing top/bottom margin you pointed out, and a styling regression on trunk). This is looking really nice!

@andrewserong
Copy link
Contributor Author

Thanks for testing @stacimc! I've raised a PR to look into the styling regression in #34189. Once that's resolved, I'll give this a rebase and we can see about getting a final test/review.

The only slightly odd things I noticed are just Buttons-specific stuff and would be handled when adding the support to that block

I imagine there'll be a few wrinkles for implementing the gap support for particular blocks, but hopefully with the CSS variable approach, this will free us up to address the concerns of the individual blocks on their own PRs, like you suggest 🙂

@ramonjd
Copy link
Member

ramonjd commented Aug 20, 2021

Also tested according to instructions. Aside from the extra margins on buttons, which @stacimc already highlighted, it's working well for me. NICE!

I also tried it out on other blocks as well. Here's the gallery block:

gappy

It's going to be very useful for such patterns. Thank you!

@andrewserong
Copy link
Contributor Author

andrewserong commented Sep 2, 2021

Was there a specific thought process behind making the name be blockGap versus customBlockGap like the other related spacing properties? It seems to make the most sense to keep the naming convention as streamlined as possible to avoid confusion.

Thanks for the question @fwazeter, it looks like you've probably already read the linked issue, but for completeness sake — according to the theme.json v2 issue the current long-term plan appears to be to remove the custom prefix from those properties where the word "custom" doesn't have semantic meaning (including for customPadding and customMargin). By ensuring that the blockGap property starts out without the prefix, we can avoid having to migrate it further down the track. So hopefully we'll be able to get that consistency in there across the properties by WP 5.9.

@youknowriad
Copy link
Contributor

It appears that Safari doesn't re-render / recalculate grid or flex layouts when the gap value changes on its own. It needs another CSS property to change as well in order to trigger the layout to be updated.

Indeed, I was testing on Safari, what a weird bug. Curious to know if they're aware of it :)
I agree that it'd be good if we find a hack for it temporarily. I don't consider it a blocker to get this moving forward though.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

LGTM

@andrewserong
Copy link
Contributor Author

Thanks @youknowriad, I appreciate the review and feedback! Since this PR adds in the block support without switching it on anywhere, I'll merge it in now and I can look at enhancements and a fix for Safari in follow-ups. 🙇

@andrewserong andrewserong merged commit f0ee759 into trunk Sep 2, 2021
@andrewserong andrewserong deleted the add/gap-block-support-feature branch September 2, 2021 23:35
@github-actions github-actions bot added this to the Gutenberg 11.5 milestone Sep 2, 2021
@andrewserong andrewserong added [Type] Feature New feature to highlight in changelogs. and removed [Status] In Progress Tracking issues with work in progress labels Sep 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Design Tools Tools that impact the appearance of blocks both to expand the number of tools and improve the experi [Type] Feature New feature to highlight in changelogs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Buttons Block: Add a way to control vertical space between stacked buttons
6 participants