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 styles: remove box-sizing rule on Post Editor container to achieve editor/frontend parity #37902

Merged
merged 2 commits into from
Jan 19, 2022

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Jan 12, 2022

Description

For editor blocks, there's a blanket CSS rule for box sizing:border-box based on a reset mixin.

No such blanket rule exists on the frontend or in the Site Editor.

So you might see the following:

Editor Frontend
Screen Shot 2022-01-12 at 2 05 14 pm Screen Shot 2022-01-12 at 2 05 27 pm

Many blocks, for example, Group, Column, Cover, already define their own box sizing:border-box rule.

As you can see from the above screenshots, Heading and List do not.

This PR proposes to remove the box-sizing rule for elements in the Post Editor (visual editor) to ensure the Post Editor ❤️ Site Editor ❤️ Frontend parity.

This change could affect existing blocks and patterns with the assumption that some users may expect the editor and frontend to differ in these respects.

The arguments for this PR are:

  • The Post Editor does not reflect the frontend for this specific case
  • Removing the anomaly (the Post Editor box-sizing) rather than adding box-sizing to the frontend might avoid unexpected side-effects. At the very least, the box-sizing will be more consistent event if it's more desirable for blocks to define their own box sizing:border-box rule.

For context, see discussion over at:

#37818 (review)

How has this been tested?

Add patterns, blocks (make sure to liberally add Heading and List blocks) to a new post.

Apply borders, padding and margins where the block supports them.

Test in various core themes.

In all scenarios, check that the box sizing:border-box is consistent across the frontend and Site/Post editors.

Types of changes

Removing a CSS rule from the post editor

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).
  • I've updated related schemas if appropriate.

…-box in the editor is reflected on the frontend.
@github-actions
Copy link

github-actions bot commented Jan 12, 2022

Size Change: +1.18 kB (0%)

Total Size: 1.13 MB

Filename Size Change
build/block-editor/index.min.js 140 kB +1 B (0%)
build/block-editor/style-rtl.css 14.6 kB +1 B (0%)
build/block-editor/style.css 14.6 kB +1 B (0%)
build/block-library/blocks/navigation/style-rtl.css 1.83 kB +9 B (0%)
build/block-library/blocks/navigation/style.css 1.82 kB +9 B (0%)
build/block-library/index.min.js 165 kB +297 B (0%)
build/block-library/style-rtl.css 10.8 kB +11 B (0%)
build/block-library/style.css 10.8 kB +11 B (0%)
build/components/index.min.js 215 kB +85 B (0%)
build/components/style-rtl.css 15.5 kB -8 B (0%)
build/components/style.css 15.5 kB -8 B (0%)
build/core-data/index.min.js 13.3 kB +8 B (0%)
build/edit-post/index.min.js 29.6 kB +6 B (0%)
build/edit-post/style-rtl.css 7.15 kB -14 B (0%)
build/edit-post/style.css 7.14 kB -15 B (0%)
build/edit-site/index.min.js 37.8 kB +538 B (+1%)
build/edit-site/style-rtl.css 6.85 kB +19 B (0%)
build/edit-site/style.css 6.84 kB +20 B (0%)
build/edit-widgets/index.min.js 16.5 kB +11 B (0%)
build/editor/index.min.js 38.4 kB +135 B (0%)
build/editor/style-rtl.css 3.71 kB +5 B (0%)
build/editor/style.css 3.71 kB +6 B (0%)
build/i18n/index.min.js 3.75 kB +43 B (+1%)
build/server-side-render/index.min.js 1.58 kB +10 B (+1%)
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 960 B
build/admin-manifest/index.min.js 1.1 kB
build/annotations/index.min.js 2.75 kB
build/api-fetch/index.min.js 2.21 kB
build/autop/index.min.js 2.12 kB
build/blob/index.min.js 459 B
build/block-directory/index.min.js 6.28 kB
build/block-directory/style-rtl.css 1.01 kB
build/block-directory/style.css 1.01 kB
build/block-editor/default-editor-styles-rtl.css 378 B
build/block-editor/default-editor-styles.css 378 B
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 150 B
build/block-library/blocks/audio/editor.css 150 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 470 B
build/block-library/blocks/button/editor.css 470 B
build/block-library/blocks/button/style-rtl.css 560 B
build/block-library/blocks/button/style.css 560 B
build/block-library/blocks/buttons/editor-rtl.css 292 B
build/block-library/blocks/buttons/editor.css 292 B
build/block-library/blocks/buttons/style-rtl.css 275 B
build/block-library/blocks/buttons/style.css 275 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 108 B
build/block-library/blocks/columns/editor.css 108 B
build/block-library/blocks/columns/style-rtl.css 406 B
build/block-library/blocks/columns/style.css 406 B
build/block-library/blocks/comment-template/style-rtl.css 127 B
build/block-library/blocks/comment-template/style.css 127 B
build/block-library/blocks/comments-pagination-numbers/editor-rtl.css 123 B
build/block-library/blocks/comments-pagination-numbers/editor.css 121 B
build/block-library/blocks/comments-pagination/editor-rtl.css 222 B
build/block-library/blocks/comments-pagination/editor.css 209 B
build/block-library/blocks/comments-pagination/style-rtl.css 235 B
build/block-library/blocks/comments-pagination/style.css 231 B
build/block-library/blocks/cover/editor-rtl.css 546 B
build/block-library/blocks/cover/editor.css 547 B
build/block-library/blocks/cover/style-rtl.css 1.22 kB
build/block-library/blocks/cover/style.css 1.22 kB
build/block-library/blocks/embed/editor-rtl.css 293 B
build/block-library/blocks/embed/editor.css 293 B
build/block-library/blocks/embed/style-rtl.css 417 B
build/block-library/blocks/embed/style.css 417 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 965 B
build/block-library/blocks/gallery/editor.css 967 B
build/block-library/blocks/gallery/style-rtl.css 1.6 kB
build/block-library/blocks/gallery/style.css 1.6 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 78 B
build/block-library/blocks/group/theme.css 78 B
build/block-library/blocks/heading/style-rtl.css 114 B
build/block-library/blocks/heading/style.css 114 B
build/block-library/blocks/html/editor-rtl.css 332 B
build/block-library/blocks/html/editor.css 333 B
build/block-library/blocks/image/editor-rtl.css 810 B
build/block-library/blocks/image/editor.css 809 B
build/block-library/blocks/image/style-rtl.css 507 B
build/block-library/blocks/image/style.css 511 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 94 B
build/block-library/blocks/list/style.css 94 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 493 B
build/block-library/blocks/media-text/style.css 490 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/editor-rtl.css 649 B
build/block-library/blocks/navigation-link/editor.css 650 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-submenu/editor-rtl.css 299 B
build/block-library/blocks/navigation-submenu/editor.css 299 B
build/block-library/blocks/navigation-submenu/view.min.js 343 B
build/block-library/blocks/navigation/editor-rtl.css 1.93 kB
build/block-library/blocks/navigation/editor.css 1.94 kB
build/block-library/blocks/navigation/view.min.js 2.82 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 401 B
build/block-library/blocks/page-list/editor.css 402 B
build/block-library/blocks/page-list/style-rtl.css 175 B
build/block-library/blocks/page-list/style.css 175 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 273 B
build/block-library/blocks/paragraph/style.css 273 B
build/block-library/blocks/post-author/style-rtl.css 175 B
build/block-library/blocks/post-author/style.css 176 B
build/block-library/blocks/post-comments-form/style-rtl.css 446 B
build/block-library/blocks/post-comments-form/style.css 446 B
build/block-library/blocks/post-comments/style-rtl.css 521 B
build/block-library/blocks/post-comments/style.css 521 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 721 B
build/block-library/blocks/post-featured-image/editor.css 721 B
build/block-library/blocks/post-featured-image/style-rtl.css 153 B
build/block-library/blocks/post-featured-image/style.css 153 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 305 B
build/block-library/blocks/post-template/style.css 305 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 80 B
build/block-library/blocks/post-title/style.css 80 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 389 B
build/block-library/blocks/pullquote/style.css 388 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 221 B
build/block-library/blocks/query-pagination/editor.css 211 B
build/block-library/blocks/query-pagination/style-rtl.css 234 B
build/block-library/blocks/query-pagination/style.css 231 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 187 B
build/block-library/blocks/quote/style.css 187 B
build/block-library/blocks/quote/theme-rtl.css 223 B
build/block-library/blocks/quote/theme.css 226 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 397 B
build/block-library/blocks/search/style.css 398 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 245 B
build/block-library/blocks/separator/style.css 245 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 744 B
build/block-library/blocks/site-logo/editor.css 744 B
build/block-library/blocks/site-logo/style-rtl.css 181 B
build/block-library/blocks/site-logo/style.css 181 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 177 B
build/block-library/blocks/social-link/editor.css 177 B
build/block-library/blocks/social-links/editor-rtl.css 670 B
build/block-library/blocks/social-links/editor.css 669 B
build/block-library/blocks/social-links/style-rtl.css 1.32 kB
build/block-library/blocks/social-links/style.css 1.32 kB
build/block-library/blocks/spacer/editor-rtl.css 332 B
build/block-library/blocks/spacer/editor.css 332 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 214 B
build/block-library/blocks/tag-cloud/style.css 215 B
build/block-library/blocks/template-part/editor-rtl.css 560 B
build/block-library/blocks/template-part/editor.css 559 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/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 908 B
build/block-library/common.css 905 B
build/block-library/editor-rtl.css 10 kB
build/block-library/editor.css 10 kB
build/block-library/reset-rtl.css 474 B
build/block-library/reset.css 474 B
build/block-library/theme-rtl.css 672 B
build/block-library/theme.css 676 B
build/block-serialization-default-parser/index.min.js 1.09 kB
build/block-serialization-spec-parser/index.min.js 2.79 kB
build/blocks/index.min.js 46.3 kB
build/compose/index.min.js 11.2 kB
build/customize-widgets/index.min.js 11.4 kB
build/customize-widgets/style-rtl.css 1.5 kB
build/customize-widgets/style.css 1.49 kB
build/data-controls/index.min.js 631 B
build/data/index.min.js 7.49 kB
build/date/index.min.js 31.9 kB
build/deprecated/index.min.js 485 B
build/dom-ready/index.min.js 304 B
build/dom/index.min.js 4.5 kB
build/edit-navigation/index.min.js 16 kB
build/edit-navigation/style-rtl.css 3.76 kB
build/edit-navigation/style.css 3.76 kB
build/edit-post/classic-rtl.css 546 B
build/edit-post/classic.css 547 B
build/edit-widgets/style-rtl.css 4.17 kB
build/edit-widgets/style.css 4.17 kB
build/element/index.min.js 3.29 kB
build/escape-html/index.min.js 517 B
build/format-library/index.min.js 6.58 kB
build/format-library/style-rtl.css 571 B
build/format-library/style.css 571 B
build/hooks/index.min.js 1.63 kB
build/html-entities/index.min.js 424 B
build/is-shallow-equal/index.min.js 501 B
build/keyboard-shortcuts/index.min.js 1.8 kB
build/keycodes/index.min.js 1.39 kB
build/list-reusable-blocks/index.min.js 1.72 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.92 kB
build/notices/index.min.js 925 B
build/nux/index.min.js 2.08 kB
build/nux/style-rtl.css 747 B
build/nux/style.css 743 B
build/plugins/index.min.js 1.84 kB
build/primitives/index.min.js 924 B
build/priority-queue/index.min.js 582 B
build/react-i18n/index.min.js 671 B
build/react-refresh-entry/index.min.js 8.44 kB
build/react-refresh-runtime/index.min.js 7.31 kB
build/redux-routine/index.min.js 2.65 kB
build/reusable-blocks/index.min.js 2.22 kB
build/reusable-blocks/style-rtl.css 256 B
build/reusable-blocks/style.css 256 B
build/rich-text/index.min.js 11 kB
build/shortcode/index.min.js 1.49 kB
build/token-list/index.min.js 639 B
build/url/index.min.js 1.9 kB
build/viewport/index.min.js 1.05 kB
build/warning/index.min.js 248 B
build/widgets/index.min.js 7.15 kB
build/widgets/style-rtl.css 1.16 kB
build/widgets/style.css 1.16 kB
build/wordcount/index.min.js 1.04 kB

compressed-size-action

@carolinan
Copy link
Contributor

Hi, can you share the block markup used in the images?

@jasmussen
Copy link
Contributor

Thanks for the draft PR! I think this rule has been discussed back and forth a few times, and there are many upsides. There are downsides too, such as the case where you want the padding or border to expand the width of the box, as opposed to squeezing it inwards. @jameskoster noted one example. There's a risk that for those edgecases there isn't a way to toggle the behavior off for a block, without adding new CSS to your theme for that case.

Given how many themes add this blanket rule by default and just keep it there, however, I'm personally in favor of this landing, given the more intuitive box model it brings with it. But it's something that could use many more thoughts: @kjellr @MaggieCabrera @richtabor @scruffian @Mamaduka to just ask a few additional folks at random.

If we do move forward, we can probably remove the editor from the previous reset, as common.scss should be loaded in the editor already. And if we don't move forward, an alternative would be to remove the box-sizing behavior from the reset, ro at least omit the editor from getting it, so at least the editor and frontend behave identically.

@ramonjd
Copy link
Member Author

ramonjd commented Jan 12, 2022

Hi, can you share the block markup used in the images?

You bet! Thanks for asking.

Example code

<!-- wp:group {"style":{"spacing":{"padding":{"top":"92px","right":"92px","bottom":"92px","left":"92px"},"blockGap":"0px"},"border":{"width":"61px","style":"solid"}},"backgroundColor":"luminous-vivid-orange"} -->
<div class="wp-block-group has-luminous-vivid-orange-background-color has-background" style="border-style:solid;border-width:61px;padding-top:92px;padding-right:92px;padding-bottom:92px;padding-left:92px"><!-- wp:heading -->
<h2 id="group-1">Group 1</h2>
<!-- /wp:heading --></div>
<!-- /wp:group -->

<!-- wp:group {"backgroundColor":"pale-pink"} -->
<div class="wp-block-group has-pale-pink-background-color has-background"><!-- wp:heading -->
<h2 id="group-2">Group 2</h2>
<!-- /wp:heading --></div>
<!-- /wp:group -->

<!-- wp:heading {"backgroundColor":"vivid-purple"} -->
<h2 class="has-vivid-purple-background-color has-background" id="heading">Heading</h2>
<!-- /wp:heading -->

<!-- wp:list {"backgroundColor":"secondary"} -->
<ul class="has-secondary-background-color has-background"><li>A list</li></ul>
<!-- /wp:list -->

<!-- wp:table {"style":{"color":{}},"className":"is-style-stripes"} -->
<figure class="wp-block-table is-style-stripes"><table><tbody><tr><td></td><td>Table</td></tr><tr><td></td><td>Table cell</td></tr></tbody></table><figcaption>Table caption</figcaption></figure>
<!-- /wp:table -->

<!-- wp:columns {"backgroundColor":"pale-cyan-blue"} -->
<div class="wp-block-columns has-pale-cyan-blue-background-color has-background"><!-- wp:column -->
<div class="wp-block-column"><!-- wp:heading -->
<h2 id="column">Column</h2>
<!-- /wp:heading --></div>
<!-- /wp:column --></div>
<!-- /wp:columns -->

<!-- wp:pullquote -->
<figure class="wp-block-pullquote"><blockquote><p>Pullquote</p></blockquote></figure>
<!-- /wp:pullquote -->

<!-- wp:cover {"overlayColor":"pale-cyan-blue","isDark":false} -->
<div class="wp-block-cover is-light"><span aria-hidden="true" class="has-pale-cyan-blue-background-color has-background-dim-100 wp-block-cover__gradient-background has-background-dim"></span><div class="wp-block-cover__inner-container"><!-- wp:paragraph {"align":"center","placeholder":"Write title…","fontSize":"large"} -->
<p class="has-text-align-center has-large-font-size">Cover block</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:cover -->

<!-- wp:heading {"level":3} -->
<h3 id="what-follows-is-a-gallery">What follows is a gallery</h3>
<!-- /wp:heading -->

<!-- wp:gallery {"linkTo":"none"} -->
<figure class="wp-block-gallery has-nested-images columns-default is-cropped"><!-- wp:image {"id":12,"sizeSlug":"large","linkDestination":"none","className":"is-style-default"} -->
<figure class="wp-block-image size-large is-style-default"><img src="http://localhost:4759/wp-content/uploads/2022/01/AdobeStock_172853026.jpg" alt="" class="wp-image-12"/></figure>
<!-- /wp:image -->

<!-- wp:image {"id":11,"sizeSlug":"large","linkDestination":"none","className":"is-style-default"} -->
<figure class="wp-block-image size-large is-style-default"><img src="http://localhost:4759/wp-content/uploads/2022/01/electric-guitar-4064538_960_720-1.jpg" alt="" class="wp-image-11"/></figure>
<!-- /wp:image --></figure>
<!-- /wp:gallery -->

<!-- wp:media-text {"align":"","mediaId":10,"mediaLink":"http://localhost:4759/?attachment_id=10","mediaType":"image"} -->
<div class="wp-block-media-text is-stacked-on-mobile"><figure class="wp-block-media-text__media"><img src="http://localhost:4759/wp-content/uploads/2022/01/Guild-Jetstar-Vintage-White-Electric-Guitar-Front-1.jpg" alt="" class="wp-image-10 size-full"/></figure><div class="wp-block-media-text__content"><!-- wp:paragraph {"placeholder":"Content…","fontSize":"large"} -->
<p class="has-large-font-size">Media block</p>
<!-- /wp:paragraph --></div></div>
<!-- /wp:media-text -->

@scruffian
Copy link
Contributor

I like this idea. My preference is to apply the rule to the frontend as I think that's a common reset that many themes would benefit from, but there may be cases where this breaks themes.

I agree with @jasmussen, if we don't move froward with this we should remove the reset from the editor so at least we have parity.

@ramonjd ramonjd self-assigned this Jan 14, 2022
@ramonjd
Copy link
Member Author

ramonjd commented Jan 17, 2022

I tested a bunch of core blocks in the site editor too. Without the changes in this PR, the Site Editor reflects the frontend, that is, no blanket box-sizing:

Screen Shot 2022-01-17 at 1 29 36 pm

I tested on a bunch of core themes (2017++) and the blocks that stand out are headings (including site title), which do not declare any box-sizing internally.

I think we're agreed that it should be consistent, the only question is what would be a low-friction approach.

if we don't move froward with this we should remove the reset from the editor so at least we have parity.

Maybe this is the right approach. We won't know the full effect of forcing box-sizing on the frontend and site editor. At least, if we treat the post editor as the outlier for now and bring it in line, people's sites won't break.

Here's what we're doing in the post editor:

.edit-post-header,
.edit-post-visual-editor,
.edit-post-text-editor,
.edit-post-sidebar,
.editor-post-publish-panel,
.components-popover,
.components-modal__frame,
.edit-post-editor__inserter-panel {
	box-sizing: border-box;

	*,
	*::before,
	*::after {
		box-sizing: inherit;
	}
}

It looks like we could remove .edit-post-visual-editor, to get most of the way there.

@jasmussen
Copy link
Contributor

Just looking at that screenshot it really feels like border and padding behaviors would benefit from the rule being applied. That said I do also share the notion that the most delicate step we can do at this point is to start by removing it since it's not necessarily there on the frontend 👍 👍

It looks like we could remove .edit-post-visual-editor, to get most of the way there.

We'll want to make sure no in-canvas blocks have come to accidentally rely on this behavior. I don't think this should be the case, but some complex blocks like Navigation or Cover would be worth sanity checking just to see that the nested UI for resizing and otherwise still behaves as intended.

Finally there are still block UI elements in the canvas that might need the rules, things like placeholders. In that light, we could potentially target .components-placeholder which should cover those.

…ere is no corresponding rule in the frontend or site editor.
@ramonjd
Copy link
Member Author

ramonjd commented Jan 18, 2022

I've flipped this PR to remove box-sizing from the visual editor (post editor)

don't think this should be the case, but some complex blocks like Navigation or Cover would be worth sanity checking just to see that the nested UI for resizing and otherwise still behaves as intended.

The Cover Block explicitly sets its own box-sizing: border-box.

The Navigation Block doesn't define box-sizing: border-box. If we opt into padding support (it doesn't by default), there will be overflow. It's consistent in the editor and frontend:

Screen Shot 2022-01-18 at 11 45 14 am

Screen Shot 2022-01-18 at 11 45 24 am

In that light, we could potentially target .components-placeholder which should cover those.

Placeholders also set their own box-sizing: border-box.

In the testing I've done it looks like they behave. 🤞

@ramonjd ramonjd added [Package] Edit Post /packages/edit-post CSS Styling Related to editor and front end styles, CSS-specific issues. labels Jan 18, 2022
@ramonjd ramonjd marked this pull request as ready for review January 18, 2022 01:30
Copy link
Contributor

@jasmussen jasmussen left a comment

Choose a reason for hiding this comment

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

I'd be happy to try this simply to add parity between editor and frontend. Whatever errors this parity might uncover in the editor should then at least be consistent with what's on the frontend, which in turn could let us revisit and add the blanket reset for themes on top of this.

I'd encourage more testing if you can get it. Thanks for working on this!

Copy link
Member

@aristath aristath left a comment

Choose a reason for hiding this comment

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

From my testing I didn't see any regressions. That being said, this is a seemingly small change but which can have significant impact that we can't see right now.
Long-term I agree with @jasmussen that it's worth trying this. If anything breaks, we can fix it on a per-use-case basis instead of adding a blanket rule - which would be better for editor/frontend parity in the future.

This has a +1 from me 👍

@ramonjd
Copy link
Member Author

ramonjd commented Jan 18, 2022

Thanks for the 👀 @jasmussen and @aristath 🙏

I'll leave it open for another round of testing before hitting the merge button.

If anything breaks, we can fix it on a per-use-case basis instead of adding a blanket rule - which would be better for editor/frontend parity in the future.

Agree. If bugs do arise, at least we'll know what the blanket rule was "concealing" and it might help us tighten up our rules.

@ramonjd ramonjd changed the title Block styles: Add a low specificity box-sizing rule to achieve editor/frontend parity Block styles: remove box-sizing rule on Post Editor container to achieve editor/frontend parity Jan 18, 2022
Copy link
Contributor

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

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

Thanks for this @ramonjd! I agree with the others, I think this will be a good one to land and it'll hopefully reveal places where there are gaps in individual blocks rather than obfuscating the issue within the post editor.

Interestingly, while testing this out (after creating the post content in Twenty Twenty Two), I noticed that setting a horizontal margin value on a heading block doesn't play nicely with some classic themes (e.g. Twenty Twenty One — the non-blocks version) where the centered view of the blocks on the front end use left/right margins to do so:

image

Fortunately those classic themes don't opt-in to the margin support it seems (so a user can't really wind up in this situation easily), but separately to this PR, I think it highlights again the challenge dealing with margin support (particularly horizontal margins) on blocks. But that discussion / consideration is of course very much separate to this PR.

LGTM!

@ramonjd ramonjd merged commit 21dfe87 into trunk Jan 19, 2022
@ramonjd ramonjd deleted the add/box-sizing-rule-to-frontend branch January 19, 2022 01:03
@github-actions github-actions bot added this to the Gutenberg 12.5 milestone Jan 19, 2022
@t-hamano t-hamano mentioned this pull request May 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Styling Related to editor and front end styles, CSS-specific issues. [Package] Edit Post /packages/edit-post
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants