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

Add a layout config to the group and theme.json and make alignments declarative #29335

Merged
merged 24 commits into from
Mar 18, 2021

Conversation

youknowriad
Copy link
Contributor

@youknowriad youknowriad commented Feb 25, 2021

The way alignments work in Gutenberg right now is very optimized for traditional themes that provide their own alignment styles and it's also built to be backward compatible with themes that can or can't support wide alignments. This approach while it served us well in phase1, it can't scale properly to FSE because it's not really WYSWYG since the themes has to provide all the custom styles for all the alignments to work properly and it's also not possible to define different kind of alignments/widths in different contexts/templates/sidebars....

The current PR is a proof of concept of how we could make alignments (and layout in general) more declarative. It's based on the following principles:

  • An empty theme with no custom stylesheets should work properly and have the backend resemble the frontend without any custom CSS. (This PR will focus on alignments here but this principle is also broken for other reasons now)
  • Alignments and layout are context specific, which means there's a global layout but each container could potentially define its own internal layout for its children.
  • Based on the above, it becomes clear that the container blocks should have a way to define/override layouts.
  • Embrace the defaults from the browser: by default inserting "divs" just show take the whole width available, blocks should behave the same way unless the parent layout says otherwise.
  • Supporting wide/full widths and floats (left/right/center) at the same time consistently has proven to be a difficult task. The only way to achieve it properly is to actually alter the markup of blocks and add an extra wrapper. The idea in this PR is that block markup shouldn't be altered because of a properly which means this PR takes the opposite approach, instead of trying to hack these features together in the same container, it proposes that a container can authorize its children to have wide/full alignments or left/right ones but not both at the same time.

Notes

  • In terms of UI, the PR essentially adds controls to the sidebar of the group block, don't mind the ugly UI though, it's there only to show the capabilities, but it can be rethought in various ways including: having presets for layouts (or just a single default layout like the PR is doing right now), hide/show the custom properties based on theme configs...
  • The PR removes a bunch of CSS. Of course we can't just remove that CSS for backward compatibility reasons, so to move forward, we'd have to find a way to only limit the CSS to the existing containers and not to the one relying on the declarative approach.

Testing the PR

  • Current themes (both block based and traditional) provide styles for different alignments... which means there will be conflicts, for this reason, I'm attaching an "empty" theme which almost nothing in its stylesheet for you to play with.
    emptytheme.zip

  • Install the theme above

  • Use the site editor (I didn't update the post editor to remove the opinionated styles for alignments/widths yet)

  • Try modifying the templates by adding different group blocks, changing their widths, and aligning their inner blocks and see how things behave, if it feels natural and simple.

Todo list

  • Move the style generation to the server
  • Potentially remove the extra div in the group block (this div doesn't seem useful to me at all in this approach)
  • Check if we can build this as a support flag
  • Provide a way to inherit the layout of core/post-content block in the post editor
  • Think about the backward compatibility story, how can we introduce this behavior only to block based themes and ensure other themes keeping using the current behavior of the group block.
  • Polish theme.json integration
  • Design work to present this in the best way possible for container blocks.
  • Tests
  • Documentation
  • Follow-up: Update existing block-based themes as this PR might break some of them.

@github-actions
Copy link

github-actions bot commented Feb 25, 2021

Size Change: +3.13 kB (0%)

Total Size: 1.4 MB

Filename Size Change
build/annotations/index.js 3.78 kB +1 B (0%)
build/blob/index.js 665 B +1 B (0%)
build/block-directory/index.js 8.63 kB -4 B (0%)
build/block-editor/index.js 126 kB +1.22 kB (+1%)
build/block-editor/style-rtl.css 12.4 kB +84 B (+1%)
build/block-editor/style.css 12.4 kB +86 B (+1%)
build/block-library/blocks/cover/editor-rtl.css 605 B +6 B (+1%)
build/block-library/blocks/cover/editor.css 605 B +6 B (+1%)
build/block-library/blocks/freeform/editor-rtl.css 2.46 kB +7 B (0%)
build/block-library/blocks/freeform/editor.css 2.46 kB +7 B (0%)
build/block-library/blocks/gallery/editor-rtl.css 696 B +7 B (+1%)
build/block-library/blocks/gallery/editor.css 696 B +6 B (+1%)
build/block-library/blocks/group/editor-rtl.css 160 B -158 B (-50%) 🏆
build/block-library/blocks/group/editor.css 160 B -157 B (-50%) 🏆
build/block-library/blocks/navigation-link/editor-rtl.css 626 B +6 B (+1%)
build/block-library/blocks/navigation-link/editor.css 627 B +6 B (+1%)
build/block-library/blocks/navigation/editor-rtl.css 1.09 kB +6 B (+1%)
build/block-library/blocks/navigation/editor.css 1.1 kB +7 B (+1%)
build/block-library/blocks/query/editor-rtl.css 820 B +6 B (+1%)
build/block-library/blocks/query/editor.css 819 B +7 B (+1%)
build/block-library/blocks/shortcode/editor-rtl.css 512 B +8 B (+2%)
build/block-library/blocks/shortcode/editor.css 512 B +8 B (+2%)
build/block-library/blocks/social-links/editor-rtl.css 776 B +7 B (+1%)
build/block-library/blocks/social-links/editor.css 776 B +7 B (+1%)
build/block-library/blocks/social-links/style-rtl.css 1.32 kB +8 B (+1%)
build/block-library/blocks/social-links/style.css 1.33 kB +8 B (+1%)
build/block-library/blocks/template-part/editor-rtl.css 552 B -5 B (-1%)
build/block-library/blocks/template-part/editor.css 551 B -5 B (-1%)
build/block-library/editor-rtl.css 9.5 kB -77 B (-1%)
build/block-library/editor.css 9.5 kB -78 B (-1%)
build/block-library/index.js 147 kB +193 B (0%)
build/block-library/style-rtl.css 8.86 kB +7 B (0%)
build/block-library/style.css 8.86 kB +7 B (0%)
build/blocks/index.js 48.3 kB +21 B (0%)
build/components/index.js 284 kB +414 B (0%)
build/components/style-rtl.css 16.2 kB +39 B (0%)
build/components/style.css 16.2 kB +42 B (0%)
build/compose/index.js 11.2 kB +90 B (+1%)
build/core-data/index.js 16.7 kB +3 B (0%)
build/customize-widgets/index.js 3.95 kB -1 B (0%)
build/data-controls/index.js 830 B +1 B (0%)
build/data/index.js 8.87 kB -2 B (0%)
build/date/index.js 31.8 kB +1 B (0%)
build/deprecated/index.js 768 B -1 B (0%)
build/dom/index.js 4.96 kB +33 B (+1%)
build/edit-navigation/index.js 11.9 kB +18 B (0%)
build/edit-post/index.js 307 kB +712 B (0%)
build/edit-post/style-rtl.css 7.06 kB +193 B (+3%)
build/edit-post/style.css 7.05 kB +192 B (+3%)
build/edit-site/index.js 27.2 kB +72 B (0%)
build/edit-site/style-rtl.css 4.46 kB -36 B (-1%)
build/edit-site/style.css 4.46 kB -35 B (-1%)
build/edit-widgets/index.js 20.2 kB +31 B (0%)
build/editor/index.js 41.9 kB +79 B (0%)
build/editor/style-rtl.css 3.9 kB +8 B (0%)
build/editor/style.css 3.9 kB +8 B (0%)
build/element/index.js 4.61 kB +1 B (0%)
build/format-library/index.js 6.75 kB -3 B (0%)
build/keyboard-shortcuts/index.js 2.53 kB +4 B (0%)
build/media-utils/index.js 5.34 kB +1 B (0%)
build/notices/index.js 1.85 kB +3 B (0%)
build/nux/index.js 3.41 kB -1 B (0%)
build/primitives/index.js 1.42 kB +1 B (0%)
build/react-i18n/index.js 1.46 kB +2 B (0%)
build/redux-routine/index.js 2.84 kB +1 B (0%)
build/rich-text/index.js 13.4 kB +1 B (0%)
build/shortcode/index.js 1.7 kB -1 B (0%)
build/url/index.js 3.02 kB +1 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/api-fetch/index.js 3.4 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/block-directory/style-rtl.css 1 kB 0 B
build/block-directory/style.css 1.01 kB 0 B
build/block-library/blocks/archives/editor-rtl.css 61 B 0 B
build/block-library/blocks/archives/editor.css 60 B 0 B
build/block-library/blocks/audio/editor-rtl.css 58 B 0 B
build/block-library/blocks/audio/editor.css 58 B 0 B
build/block-library/blocks/audio/style-rtl.css 112 B 0 B
build/block-library/blocks/audio/style.css 112 B 0 B
build/block-library/blocks/block/editor-rtl.css 161 B 0 B
build/block-library/blocks/block/editor.css 161 B 0 B
build/block-library/blocks/button/editor-rtl.css 475 B 0 B
build/block-library/blocks/button/editor.css 474 B 0 B
build/block-library/blocks/button/style-rtl.css 479 B 0 B
build/block-library/blocks/button/style.css 479 B 0 B
build/block-library/blocks/buttons/editor-rtl.css 315 B 0 B
build/block-library/blocks/buttons/editor.css 315 B 0 B
build/block-library/blocks/buttons/style-rtl.css 364 B 0 B
build/block-library/blocks/buttons/style.css 363 B 0 B
build/block-library/blocks/calendar/style-rtl.css 208 B 0 B
build/block-library/blocks/calendar/style.css 208 B 0 B
build/block-library/blocks/categories/editor-rtl.css 84 B 0 B
build/block-library/blocks/categories/editor.css 83 B 0 B
build/block-library/blocks/categories/style-rtl.css 79 B 0 B
build/block-library/blocks/categories/style.css 79 B 0 B
build/block-library/blocks/code/style-rtl.css 90 B 0 B
build/block-library/blocks/code/style.css 90 B 0 B
build/block-library/blocks/columns/editor-rtl.css 190 B 0 B
build/block-library/blocks/columns/editor.css 190 B 0 B
build/block-library/blocks/columns/style-rtl.css 421 B 0 B
build/block-library/blocks/columns/style.css 421 B 0 B
build/block-library/blocks/cover/style-rtl.css 1.24 kB 0 B
build/block-library/blocks/cover/style.css 1.24 kB 0 B
build/block-library/blocks/embed/editor-rtl.css 486 B 0 B
build/block-library/blocks/embed/editor.css 486 B 0 B
build/block-library/blocks/embed/style-rtl.css 401 B 0 B
build/block-library/blocks/embed/style.css 400 B 0 B
build/block-library/blocks/file/editor-rtl.css 199 B 0 B
build/block-library/blocks/file/editor.css 198 B 0 B
build/block-library/blocks/file/style-rtl.css 248 B 0 B
build/block-library/blocks/file/style.css 248 B 0 B
build/block-library/blocks/gallery/style-rtl.css 1.11 kB 0 B
build/block-library/blocks/gallery/style.css 1.1 kB 0 B
build/block-library/blocks/group/style-rtl.css 57 B 0 B
build/block-library/blocks/group/style.css 57 B 0 B
build/block-library/blocks/heading/editor-rtl.css 129 B 0 B
build/block-library/blocks/heading/editor.css 129 B 0 B
build/block-library/blocks/heading/style-rtl.css 76 B 0 B
build/block-library/blocks/heading/style.css 76 B 0 B
build/block-library/blocks/html/editor-rtl.css 281 B 0 B
build/block-library/blocks/html/editor.css 281 B 0 B
build/block-library/blocks/image/editor-rtl.css 717 B 0 B
build/block-library/blocks/image/editor.css 716 B 0 B
build/block-library/blocks/image/style-rtl.css 476 B 0 B
build/block-library/blocks/image/style.css 478 B 0 B
build/block-library/blocks/latest-comments/editor-rtl.css 159 B 0 B
build/block-library/blocks/latest-comments/editor.css 158 B 0 B
build/block-library/blocks/latest-comments/style-rtl.css 269 B 0 B
build/block-library/blocks/latest-comments/style.css 269 B 0 B
build/block-library/blocks/latest-posts/editor-rtl.css 137 B 0 B
build/block-library/blocks/latest-posts/editor.css 137 B 0 B
build/block-library/blocks/latest-posts/style-rtl.css 523 B 0 B
build/block-library/blocks/latest-posts/style.css 522 B 0 B
build/block-library/blocks/list/editor-rtl.css 65 B 0 B
build/block-library/blocks/list/editor.css 65 B 0 B
build/block-library/blocks/list/style-rtl.css 63 B 0 B
build/block-library/blocks/list/style.css 63 B 0 B
build/block-library/blocks/media-text/editor-rtl.css 191 B 0 B
build/block-library/blocks/media-text/editor.css 191 B 0 B
build/block-library/blocks/media-text/style-rtl.css 535 B 0 B
build/block-library/blocks/media-text/style.css 532 B 0 B
build/block-library/blocks/more/editor-rtl.css 434 B 0 B
build/block-library/blocks/more/editor.css 434 B 0 B
build/block-library/blocks/navigation-link/style-rtl.css 671 B 0 B
build/block-library/blocks/navigation-link/style.css 668 B 0 B
build/block-library/blocks/navigation/style-rtl.css 204 B 0 B
build/block-library/blocks/navigation/style.css 205 B 0 B
build/block-library/blocks/nextpage/editor-rtl.css 395 B 0 B
build/block-library/blocks/nextpage/editor.css 395 B 0 B
build/block-library/blocks/page-list/editor-rtl.css 215 B 0 B
build/block-library/blocks/page-list/editor.css 215 B 0 B
build/block-library/blocks/page-list/style-rtl.css 527 B 0 B
build/block-library/blocks/page-list/style.css 526 B 0 B
build/block-library/blocks/paragraph/editor-rtl.css 157 B 0 B
build/block-library/blocks/paragraph/editor.css 157 B 0 B
build/block-library/blocks/paragraph/style-rtl.css 247 B 0 B
build/block-library/blocks/paragraph/style.css 248 B 0 B
build/block-library/blocks/post-author/editor-rtl.css 209 B 0 B
build/block-library/blocks/post-author/editor.css 209 B 0 B
build/block-library/blocks/post-author/style-rtl.css 183 B 0 B
build/block-library/blocks/post-author/style.css 184 B 0 B
build/block-library/blocks/post-comments-form/style-rtl.css 250 B 0 B
build/block-library/blocks/post-comments-form/style.css 250 B 0 B
build/block-library/blocks/post-content/editor-rtl.css 139 B 0 B
build/block-library/blocks/post-content/editor.css 139 B 0 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B 0 B
build/block-library/blocks/post-excerpt/editor.css 73 B 0 B
build/block-library/blocks/post-featured-image/editor-rtl.css 338 B 0 B
build/block-library/blocks/post-featured-image/editor.css 338 B 0 B
build/block-library/blocks/post-featured-image/style-rtl.css 100 B 0 B
build/block-library/blocks/post-featured-image/style.css 100 B 0 B
build/block-library/blocks/preformatted/style-rtl.css 63 B 0 B
build/block-library/blocks/preformatted/style.css 63 B 0 B
build/block-library/blocks/pullquote/editor-rtl.css 183 B 0 B
build/block-library/blocks/pullquote/editor.css 183 B 0 B
build/block-library/blocks/pullquote/style-rtl.css 318 B 0 B
build/block-library/blocks/pullquote/style.css 318 B 0 B
build/block-library/blocks/query-loop/editor-rtl.css 90 B 0 B
build/block-library/blocks/query-loop/editor.css 89 B 0 B
build/block-library/blocks/query-loop/style-rtl.css 315 B 0 B
build/block-library/blocks/query-loop/style.css 317 B 0 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B 0 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B 0 B
build/block-library/blocks/query-pagination/editor-rtl.css 270 B 0 B
build/block-library/blocks/query-pagination/editor.css 262 B 0 B
build/block-library/blocks/query-pagination/style-rtl.css 168 B 0 B
build/block-library/blocks/query-pagination/style.css 168 B 0 B
build/block-library/blocks/query-title/editor-rtl.css 86 B 0 B
build/block-library/blocks/query-title/editor.css 86 B 0 B
build/block-library/blocks/quote/editor-rtl.css 61 B 0 B
build/block-library/blocks/quote/editor.css 61 B 0 B
build/block-library/blocks/quote/style-rtl.css 169 B 0 B
build/block-library/blocks/quote/style.css 169 B 0 B
build/block-library/blocks/rss/editor-rtl.css 201 B 0 B
build/block-library/blocks/rss/editor.css 202 B 0 B
build/block-library/blocks/rss/style-rtl.css 290 B 0 B
build/block-library/blocks/rss/style.css 290 B 0 B
build/block-library/blocks/search/editor-rtl.css 165 B 0 B
build/block-library/blocks/search/editor.css 165 B 0 B
build/block-library/blocks/search/style-rtl.css 342 B 0 B
build/block-library/blocks/search/style.css 344 B 0 B
build/block-library/blocks/separator/editor-rtl.css 99 B 0 B
build/block-library/blocks/separator/editor.css 99 B 0 B
build/block-library/blocks/separator/style-rtl.css 236 B 0 B
build/block-library/blocks/separator/style.css 236 B 0 B
build/block-library/blocks/site-logo/editor-rtl.css 201 B 0 B
build/block-library/blocks/site-logo/editor.css 201 B 0 B
build/block-library/blocks/site-logo/style-rtl.css 115 B 0 B
build/block-library/blocks/site-logo/style.css 115 B 0 B
build/block-library/blocks/social-link/editor-rtl.css 164 B 0 B
build/block-library/blocks/social-link/editor.css 165 B 0 B
build/block-library/blocks/spacer/editor-rtl.css 317 B 0 B
build/block-library/blocks/spacer/editor.css 317 B 0 B
build/block-library/blocks/spacer/style-rtl.css 48 B 0 B
build/block-library/blocks/spacer/style.css 48 B 0 B
build/block-library/blocks/table/editor-rtl.css 478 B 0 B
build/block-library/blocks/table/editor.css 478 B 0 B
build/block-library/blocks/table/style-rtl.css 402 B 0 B
build/block-library/blocks/table/style.css 402 B 0 B
build/block-library/blocks/tag-cloud/editor-rtl.css 118 B 0 B
build/block-library/blocks/tag-cloud/editor.css 118 B 0 B
build/block-library/blocks/tag-cloud/style-rtl.css 94 B 0 B
build/block-library/blocks/tag-cloud/style.css 94 B 0 B
build/block-library/blocks/term-description/editor-rtl.css 90 B 0 B
build/block-library/blocks/term-description/editor.css 90 B 0 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B 0 B
build/block-library/blocks/text-columns/editor.css 95 B 0 B
build/block-library/blocks/text-columns/style-rtl.css 166 B 0 B
build/block-library/blocks/text-columns/style.css 166 B 0 B
build/block-library/blocks/verse/editor-rtl.css 50 B 0 B
build/block-library/blocks/verse/editor.css 50 B 0 B
build/block-library/blocks/verse/style-rtl.css 87 B 0 B
build/block-library/blocks/verse/style.css 87 B 0 B
build/block-library/blocks/video/editor-rtl.css 504 B 0 B
build/block-library/blocks/video/editor.css 503 B 0 B
build/block-library/blocks/video/style-rtl.css 187 B 0 B
build/block-library/blocks/video/style.css 187 B 0 B
build/block-library/common-rtl.css 1.1 kB 0 B
build/block-library/common.css 1.1 kB 0 B
build/block-library/theme-rtl.css 700 B 0 B
build/block-library/theme.css 701 B 0 B
build/block-serialization-default-parser/index.js 1.87 kB 0 B
build/block-serialization-spec-parser/index.js 3.06 kB 0 B
build/customize-widgets/style-rtl.css 168 B 0 B
build/customize-widgets/style.css 168 B 0 B
build/dom-ready/index.js 576 B 0 B
build/edit-navigation/style-rtl.css 1.31 kB 0 B
build/edit-navigation/style.css 1.31 kB 0 B
build/edit-widgets/style-rtl.css 3.2 kB 0 B
build/edit-widgets/style.css 3.2 kB 0 B
build/editor/editor-styles-rtl.css 347 B 0 B
build/editor/editor-styles.css 347 B 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/style-rtl.css 637 B 0 B
build/format-library/style.css 639 B 0 B
build/hooks/index.js 2.28 kB 0 B
build/html-entities/index.js 623 B 0 B
build/i18n/index.js 4.01 kB 0 B
build/is-shallow-equal/index.js 698 B 0 B
build/keycodes/index.js 1.95 kB 0 B
build/list-reusable-blocks/index.js 3.14 kB 0 B
build/list-reusable-blocks/style-rtl.css 629 B 0 B
build/list-reusable-blocks/style.css 628 B 0 B
build/nux/style-rtl.css 731 B 0 B
build/nux/style.css 727 B 0 B
build/plugins/index.js 2.89 kB 0 B
build/priority-queue/index.js 791 B 0 B
build/reusable-blocks/index.js 3.78 kB 0 B
build/reusable-blocks/style-rtl.css 225 B 0 B
build/reusable-blocks/style.css 225 B 0 B
build/server-side-render/index.js 2.58 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/viewport/index.js 1.86 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

@jasmussen
Copy link
Contributor

jasmussen commented Feb 25, 2021

This is what I see:

alignments

This is cool. I imagine:

  • Post content would come with defaults that mimic what post content does today, only all the vinegar of enabling wide and fullwide are taken out of it, and much more configurable.
  • Blocks inserted in other areas, like the Header, do not have to enable "full wide" to avoid being in a centered column. Unless that's what you want, in which case you can change the layout properties of the Header.

I'm sure there will be implementation headaches, but the principles here all seem like they solve profoundly more problems than they introduce.

As for supporting old and new themes, could the trigger be is it a block-based theme? Or could it involve the add_theme_support( 'align-wide' ); somehow?

Nice work!

Edit: Oh, and "full wide" should also have a configurable content width. While it's mean to imply edge to edge, you could imagine it being "edge to edge minus a padding".

@ellatrix
Copy link
Member

it proposes that a container can authorize its children to have wide/full alignments or left/right ones but not both at the same time

What do you mean by this? Can different blocks in the post content have a wide/full width and be left/right floated?

@youknowriad
Copy link
Contributor Author

it proposes that a container can authorize its children to have wide/full alignments or left/right ones but not both at the same time

I mean, inside a single container, you can say the available alignments for the children are

  • wide, full
  • left, right, center

but not wide, full, left, right, center.

@youknowriad
Copy link
Contributor Author

As for supporting old and new themes, could the trigger be is it a block-based theme? Or could it involve the add_theme_support( 'align-wide' ); somehow?

I'm not sure yet :) I do believe block-based theme should only have that behavior yes, and that the align-wide theme support is ignored here. (deprecated)

But for classic themes, we could potentially have a way to opt-in into this as well somehow.

@ellatrix
Copy link
Member

but not wide, full, left, right, center

So we wouldn't have all those options for an image block?

@youknowriad
Copy link
Contributor Author

So we wouldn't have all those options for an image block?

yes, unless you add an intermediary group block. If you think about it, it makes sense, it's that "alignment wrapper" we used to add statically in block markup for some blocks but not others.

@jasmussen
Copy link
Contributor

yes, unless you add an intermediary group block. If you think about it, it makes sense, it's that "alignment wrapper" we used to add statically in block markup for some blocks but not others.

This would also be the way to enable floats inside CSS grid based contexts.

Potentially a multi select to pattern transform could help make this feel more natural.

@ellatrix
Copy link
Member

Why not allow floats/alignments in any of these cases and separate alignment from layout (break outs)? Floating works regardless of the context you're in.

@youknowriad
Copy link
Contributor Author

Why not allow floats/alignments in any of these cases and separate alignment from layout (break outs)? Floating works regardless of the context you're in.

oh we definitely can, but for me it's more a user expectation (not a technical blocker): in a context where the "content area" is centered by default, you don't expect "align left" (users don't understand float) to break out of the content area while adding a group block, setting it to "wide" and adding a left aligned thing inside it seems more natural to me.

@mtias
Copy link
Member

mtias commented Feb 25, 2021

Awesome to see this! I agree alignments are one of the most painful parts to support on a new theme and removign all that handling would be excellent.

Post content would come with defaults that mimic what post content does today, only all the vinegar of enabling wide and fullwide are taken out of it, and much more configurable.

Yes, I think current post-content should be emulated as:

  • Invisible wrapper (you don't see the content boundaries).
  • Is full by default with inner content set to $content_width and wide to whatever the theme has instructed.

Oh, and "full wide" should also have a configurable content width. While it's mean to imply edge to edge, you could imagine it being "edge to edge minus a padding".

I think this should be the actual padding control that the Group block has.

it proposes that a container can authorize its children to have wide/full alignments or left/right ones but not both at the same time

Left / Right / Center are valuable in containers like "Cover", where we use the matrix alignments. With grid it can also mimic layouts that now require columns to be used. However, I wouldn't couple Left / Right with floats, which are a very particular and quirky subclass. I think we need to support left / right as the default position of the inner content (which is what Cover does with Matrix control).

Matrix should also be available for Group, for example.

yes, unless you add an intermediary group block. If you think about it, it makes sense, it's that "alignment wrapper" we used to add statically in block markup for some blocks but not others.

This makes sense structurally, but it should be surfaced more transparently to users in the toolbar. For example, clicking on "float left" would transform to group, or allow you to say how many elements you want to include in the group. The UX could be tricky since floating requires other blocks to float around so it's more like a multi-block transform.

@aristath
Copy link
Member

aristath commented Feb 25, 2021

I like this a lot!! It was one of the major pain points for blocks styling & FSE...

we could consider CSS variables, the problem is that I don't want them to cascade and CSS variables do cascade.

What is the problem with cascading? Having them cascade makes perfect sense to me... If I set something on a group I expect it to apply to all inner group blocks by default - unless I override it in one of the children. If it doesn't cascade I'll have to make the same tweaks for all inner groups 🤔

@youknowriad
Copy link
Contributor Author

What is the problem with cascading? Having them cascade makes perfect sense to me...

No, what we want is to apply the "widths" to the inner blocks of the current container but not the grand children blocks of another container which means we'd have to "reset" these variables somehow regardless of whether a layout has been defined in the inner container block. This makes the CSS variables less compelling to me here.

@ntsekouras
Copy link
Contributor

This is really interesting Riad! 👏

yes, unless you add an intermediary group block. If you think about it, it makes sense, it's that "alignment wrapper" we used to add statically in block markup for some blocks but not others.

I played around with this and in a group I added another group with an Image inside but the alignment options were only wide, full. By the above comment I would expect them to also show left etc.., no?

I know that this is a POC now so I'm asking about what it should show.

@youknowriad
Copy link
Contributor Author

By the above comment I would expect them to also show left etc.., no?

yes, it should show left/right and not wide/full, make sure that intermediary group block doesn't have any "content size" or "wide size" attributes. (empty)

@kjellr kjellr added the [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. label Feb 25, 2021
@youknowriad youknowriad mentioned this pull request Feb 26, 2021
6 tasks
@jasmussen
Copy link
Contributor

jasmussen commented Feb 26, 2021

Oh, and "full wide" should also have a configurable content width. While it's mean to imply edge to edge, you could imagine it being "edge to edge minus a padding".

I think this should be the actual padding control that the Group block has.

Excellent point.

@youknowriad
Copy link
Contributor Author

I updated the PR description and added a todo list of items that needs to happen in order to ship this. It's a lot and I'd love to find ways to ship this iteratively but can't think of what pieces I can extract from this. Also some of these items are actually design work, so I'd love some help there.

@kjellr
Copy link
Contributor

kjellr commented Feb 26, 2021

I gave this a test this morning, and I really like the direction it's headed in. It works nicely out of the box, which is exactly what I've been looking for in terms of Gutenberg's alignment rules. 😄

But for classic themes, we could potentially have a way to opt-in into this as well somehow.

Just wanted to chime in to say that this would be great — I imagine quite a few existing classic themes could decide to rely on this and jettison quite a bit of complicated CSS in the process.

1
);

return $content . '<style>' . $style . '</style>';
Copy link
Member

Choose a reason for hiding this comment

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

@youknowriad Is there currently any API in PHP that we can use to combine all these styles at the start or end of the content?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can potentially use wp_enqueue_style but I believe we do inline style tags already (lazy loading of styles), so it's not the only place we do that.

aristath added a commit to aristath/q that referenced this pull request Mar 19, 2021
@youknowriad youknowriad mentioned this pull request Jun 1, 2021
15 tasks
@youknowriad youknowriad removed the Needs Dev Note Requires a developer note for a major WordPress release cycle label Jul 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Group Affects the Group Block [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Experimental Experimental feature or API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.