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

Try: Make empty paragraphs take up the same space on the frontend, as in the editor #27995

Merged
merged 3 commits into from
Jan 14, 2021

Conversation

jasmussen
Copy link
Contributor

@jasmussen jasmussen commented Jan 5, 2021

Fixes #10051.

If you press Enter a bunch of times, you get this:

Screenshot 2021-01-05 at 13 14 29

Those are empty paragraphs, and seem to suggest you'd get some space there when you publish. But you don't, those paragraphs collapse to nothing, but are still present in the markup. Essentially tagsoup. This PR fixes that.

Before:

before

After:

after

Note, an ealier version of this PR also tweaked the behavior of paragraphs in the editor. I have removed that to focus on the frontend backend consistency.

@jasmussen jasmussen added the Needs Design Feedback Needs general design feedback. label Jan 5, 2021
@jasmussen jasmussen requested a review from kjellr January 5, 2021 12:26
@jasmussen jasmussen self-assigned this Jan 5, 2021
@github-actions
Copy link

github-actions bot commented Jan 5, 2021

Size Change: +108 B (0%)

Total Size: 1.28 MB

Filename Size Change
build/block-library/blocks/paragraph/style-rtl.css 390 B +39 B (+11%) ⚠️
build/block-library/blocks/paragraph/style.css 391 B +39 B (+11%) ⚠️
build/block-library/style-rtl.css 8.52 kB +15 B (0%)
build/block-library/style.css 8.52 kB +15 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.8 kB 0 B
build/api-fetch/index.js 3.42 kB 0 B
build/autop/index.js 2.83 kB 0 B
build/blob/index.js 665 B 0 B
build/block-directory/index.js 9.03 kB 0 B
build/block-directory/style-rtl.css 1.01 kB 0 B
build/block-directory/style.css 1.01 kB 0 B
build/block-editor/index.js 121 kB 0 B
build/block-editor/style-rtl.css 11.5 kB 0 B
build/block-editor/style.css 11.5 kB 0 B
build/block-library/blocks/archives/editor-rtl.css 196 B 0 B
build/block-library/blocks/archives/editor.css 196 B 0 B
build/block-library/blocks/audio/editor-rtl.css 194 B 0 B
build/block-library/blocks/audio/editor.css 194 B 0 B
build/block-library/blocks/audio/style-rtl.css 225 B 0 B
build/block-library/blocks/audio/style.css 225 B 0 B
build/block-library/blocks/block/editor-rtl.css 283 B 0 B
build/block-library/blocks/block/editor.css 283 B 0 B
build/block-library/blocks/button/editor-rtl.css 576 B 0 B
build/block-library/blocks/button/editor.css 577 B 0 B
build/block-library/blocks/button/style-rtl.css 552 B 0 B
build/block-library/blocks/button/style.css 552 B 0 B
build/block-library/blocks/buttons/editor-rtl.css 345 B 0 B
build/block-library/blocks/buttons/editor.css 346 B 0 B
build/block-library/blocks/buttons/style-rtl.css 419 B 0 B
build/block-library/blocks/buttons/style.css 419 B 0 B
build/block-library/blocks/calendar/style-rtl.css 319 B 0 B
build/block-library/blocks/calendar/style.css 319 B 0 B
build/block-library/blocks/categories/editor-rtl.css 210 B 0 B
build/block-library/blocks/categories/editor.css 209 B 0 B
build/block-library/blocks/categories/style-rtl.css 208 B 0 B
build/block-library/blocks/categories/style.css 208 B 0 B
build/block-library/blocks/code/style-rtl.css 216 B 0 B
build/block-library/blocks/code/style.css 216 B 0 B
build/block-library/blocks/columns/editor-rtl.css 300 B 0 B
build/block-library/blocks/columns/editor.css 299 B 0 B
build/block-library/blocks/columns/style-rtl.css 529 B 0 B
build/block-library/blocks/columns/style.css 528 B 0 B
build/block-library/blocks/cover/editor-rtl.css 524 B 0 B
build/block-library/blocks/cover/editor.css 522 B 0 B
build/block-library/blocks/cover/style-rtl.css 1.3 kB 0 B
build/block-library/blocks/cover/style.css 1.3 kB 0 B
build/block-library/blocks/embed/editor-rtl.css 594 B 0 B
build/block-library/blocks/embed/editor.css 595 B 0 B
build/block-library/blocks/embed/style-rtl.css 489 B 0 B
build/block-library/blocks/embed/style.css 489 B 0 B
build/block-library/blocks/file/editor-rtl.css 314 B 0 B
build/block-library/blocks/file/editor.css 313 B 0 B
build/block-library/blocks/file/style-rtl.css 352 B 0 B
build/block-library/blocks/file/style.css 352 B 0 B
build/block-library/blocks/freeform/editor-rtl.css 2.55 kB 0 B
build/block-library/blocks/freeform/editor.css 2.55 kB 0 B
build/block-library/blocks/gallery/editor-rtl.css 749 B 0 B
build/block-library/blocks/gallery/editor.css 750 B 0 B
build/block-library/blocks/gallery/style-rtl.css 1.17 kB 0 B
build/block-library/blocks/gallery/style.css 1.17 kB 0 B
build/block-library/blocks/group/editor-rtl.css 433 B 0 B
build/block-library/blocks/group/editor.css 432 B 0 B
build/block-library/blocks/group/style-rtl.css 190 B 0 B
build/block-library/blocks/group/style.css 190 B 0 B
build/block-library/blocks/heading/editor-rtl.css 248 B 0 B
build/block-library/blocks/heading/editor.css 248 B 0 B
build/block-library/blocks/heading/style-rtl.css 212 B 0 B
build/block-library/blocks/heading/style.css 212 B 0 B
build/block-library/blocks/html/editor-rtl.css 384 B 0 B
build/block-library/blocks/html/editor.css 385 B 0 B
build/block-library/blocks/image/editor-rtl.css 801 B 0 B
build/block-library/blocks/image/editor.css 800 B 0 B
build/block-library/blocks/image/style-rtl.css 569 B 0 B
build/block-library/blocks/image/style.css 570 B 0 B
build/block-library/blocks/latest-comments/editor-rtl.css 277 B 0 B
build/block-library/blocks/latest-comments/editor.css 275 B 0 B
build/block-library/blocks/latest-comments/style-rtl.css 382 B 0 B
build/block-library/blocks/latest-comments/style.css 382 B 0 B
build/block-library/blocks/latest-posts/editor-rtl.css 254 B 0 B
build/block-library/blocks/latest-posts/editor.css 254 B 0 B
build/block-library/blocks/latest-posts/style-rtl.css 634 B 0 B
build/block-library/blocks/latest-posts/style.css 634 B 0 B
build/block-library/blocks/list/editor-rtl.css 203 B 0 B
build/block-library/blocks/list/editor.css 203 B 0 B
build/block-library/blocks/list/style-rtl.css 201 B 0 B
build/block-library/blocks/list/style.css 201 B 0 B
build/block-library/blocks/media-text/editor-rtl.css 311 B 0 B
build/block-library/blocks/media-text/editor.css 311 B 0 B
build/block-library/blocks/media-text/style-rtl.css 642 B 0 B
build/block-library/blocks/media-text/style.css 640 B 0 B
build/block-library/blocks/more/editor-rtl.css 545 B 0 B
build/block-library/blocks/more/editor.css 545 B 0 B
build/block-library/blocks/navigation-link/editor-rtl.css 503 B 0 B
build/block-library/blocks/navigation-link/editor.css 504 B 0 B
build/block-library/blocks/navigation-link/style-rtl.css 805 B 0 B
build/block-library/blocks/navigation-link/style.css 803 B 0 B
build/block-library/blocks/navigation/editor-rtl.css 1.38 kB 0 B
build/block-library/blocks/navigation/editor.css 1.38 kB 0 B
build/block-library/blocks/navigation/style-rtl.css 274 B 0 B
build/block-library/blocks/navigation/style.css 274 B 0 B
build/block-library/blocks/nextpage/editor-rtl.css 507 B 0 B
build/block-library/blocks/nextpage/editor.css 507 B 0 B
build/block-library/blocks/paragraph/editor-rtl.css 236 B 0 B
build/block-library/blocks/paragraph/editor.css 236 B 0 B
build/block-library/blocks/post-author/editor-rtl.css 329 B 0 B
build/block-library/blocks/post-author/editor.css 329 B 0 B
build/block-library/blocks/post-author/style-rtl.css 303 B 0 B
build/block-library/blocks/post-author/style.css 303 B 0 B
build/block-library/blocks/post-comments-form/style-rtl.css 358 B 0 B
build/block-library/blocks/post-comments-form/style.css 358 B 0 B
build/block-library/blocks/post-content/editor-rtl.css 262 B 0 B
build/block-library/blocks/post-content/editor.css 262 B 0 B
build/block-library/blocks/post-excerpt/editor-rtl.css 206 B 0 B
build/block-library/blocks/post-excerpt/editor.css 206 B 0 B
build/block-library/blocks/post-featured-image/editor-rtl.css 453 B 0 B
build/block-library/blocks/post-featured-image/editor.css 453 B 0 B
build/block-library/blocks/post-featured-image/style-rtl.css 223 B 0 B
build/block-library/blocks/post-featured-image/style.css 223 B 0 B
build/block-library/blocks/preformatted/style-rtl.css 193 B 0 B
build/block-library/blocks/preformatted/style.css 193 B 0 B
build/block-library/blocks/pullquote/editor-rtl.css 304 B 0 B
build/block-library/blocks/pullquote/editor.css 304 B 0 B
build/block-library/blocks/pullquote/style-rtl.css 428 B 0 B
build/block-library/blocks/pullquote/style.css 428 B 0 B
build/block-library/blocks/query-loop/editor-rtl.css 217 B 0 B
build/block-library/blocks/query-loop/editor.css 216 B 0 B
build/block-library/blocks/query-loop/style-rtl.css 427 B 0 B
build/block-library/blocks/query-loop/style.css 429 B 0 B
build/block-library/blocks/query/editor-rtl.css 279 B 0 B
build/block-library/blocks/query/editor.css 279 B 0 B
build/block-library/blocks/quote/editor-rtl.css 195 B 0 B
build/block-library/blocks/quote/editor.css 195 B 0 B
build/block-library/blocks/quote/style-rtl.css 284 B 0 B
build/block-library/blocks/quote/style.css 285 B 0 B
build/block-library/blocks/rss/editor-rtl.css 307 B 0 B
build/block-library/blocks/rss/editor.css 309 B 0 B
build/block-library/blocks/rss/style-rtl.css 394 B 0 B
build/block-library/blocks/rss/style.css 393 B 0 B
build/block-library/blocks/search/editor-rtl.css 285 B 0 B
build/block-library/blocks/search/editor.css 285 B 0 B
build/block-library/blocks/search/style-rtl.css 454 B 0 B
build/block-library/blocks/search/style.css 456 B 0 B
build/block-library/blocks/separator/editor-rtl.css 229 B 0 B
build/block-library/blocks/separator/editor.css 229 B 0 B
build/block-library/blocks/separator/style-rtl.css 352 B 0 B
build/block-library/blocks/separator/style.css 352 B 0 B
build/block-library/blocks/shortcode/editor-rtl.css 603 B 0 B
build/block-library/blocks/shortcode/editor.css 603 B 0 B
build/block-library/blocks/site-logo/editor-rtl.css 321 B 0 B
build/block-library/blocks/site-logo/editor.css 321 B 0 B
build/block-library/blocks/site-logo/style-rtl.css 238 B 0 B
build/block-library/blocks/site-logo/style.css 238 B 0 B
build/block-library/blocks/social-link/editor-rtl.css 283 B 0 B
build/block-library/blocks/social-link/editor.css 283 B 0 B
build/block-library/blocks/social-links/editor-rtl.css 811 B 0 B
build/block-library/blocks/social-links/editor.css 810 B 0 B
build/block-library/blocks/social-links/style-rtl.css 1.44 kB 0 B
build/block-library/blocks/social-links/style.css 1.44 kB 0 B
build/block-library/blocks/spacer/editor-rtl.css 416 B 0 B
build/block-library/blocks/spacer/editor.css 416 B 0 B
build/block-library/blocks/spacer/style-rtl.css 184 B 0 B
build/block-library/blocks/spacer/style.css 184 B 0 B
build/block-library/blocks/subhead/editor-rtl.css 223 B 0 B
build/block-library/blocks/subhead/editor.css 223 B 0 B
build/block-library/blocks/subhead/style-rtl.css 210 B 0 B
build/block-library/blocks/subhead/style.css 210 B 0 B
build/block-library/blocks/table/editor-rtl.css 593 B 0 B
build/block-library/blocks/table/editor.css 593 B 0 B
build/block-library/blocks/table/style-rtl.css 501 B 0 B
build/block-library/blocks/table/style.css 501 B 0 B
build/block-library/blocks/tag-cloud/editor-rtl.css 237 B 0 B
build/block-library/blocks/tag-cloud/editor.css 235 B 0 B
build/block-library/blocks/tag-cloud/style-rtl.css 221 B 0 B
build/block-library/blocks/tag-cloud/style.css 221 B 0 B
build/block-library/blocks/template-part/editor-rtl.css 714 B 0 B
build/block-library/blocks/template-part/editor.css 714 B 0 B
build/block-library/blocks/text-columns/editor-rtl.css 220 B 0 B
build/block-library/blocks/text-columns/editor.css 220 B 0 B
build/block-library/blocks/text-columns/style-rtl.css 283 B 0 B
build/block-library/blocks/text-columns/style.css 283 B 0 B
build/block-library/blocks/verse/editor-rtl.css 194 B 0 B
build/block-library/blocks/verse/editor.css 194 B 0 B
build/block-library/blocks/verse/style-rtl.css 215 B 0 B
build/block-library/blocks/verse/style.css 215 B 0 B
build/block-library/blocks/video/editor-rtl.css 617 B 0 B
build/block-library/blocks/video/editor.css 617 B 0 B
build/block-library/blocks/video/style-rtl.css 303 B 0 B
build/block-library/blocks/video/style.css 304 B 0 B
build/block-library/common-rtl.css 1.01 kB 0 B
build/block-library/common.css 1.01 kB 0 B
build/block-library/editor-rtl.css 8.96 kB 0 B
build/block-library/editor.css 8.96 kB 0 B
build/block-library/index.js 142 kB 0 B
build/block-library/theme-rtl.css 860 B 0 B
build/block-library/theme.css 860 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.06 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/index.js 173 kB 0 B
build/components/style-rtl.css 15.5 kB 0 B
build/components/style.css 15.5 kB 0 B
build/compose/index.js 11.2 kB 0 B
build/core-data/index.js 15.2 kB 0 B
build/data-controls/index.js 829 B 0 B
build/data/index.js 8.97 kB 0 B
build/date/index.js 31.8 kB 0 B
build/deprecated/index.js 768 B 0 B
build/dom-ready/index.js 571 B 0 B
build/dom/index.js 4.95 kB 0 B
build/edit-navigation/index.js 11.1 kB 0 B
build/edit-navigation/style-rtl.css 938 B 0 B
build/edit-navigation/style.css 944 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.56 kB 0 B
build/edit-post/style.css 6.55 kB 0 B
build/edit-site/index.js 24.2 kB 0 B
build/edit-site/style-rtl.css 4 kB 0 B
build/edit-site/style.css 4 kB 0 B
build/edit-widgets/index.js 23.6 kB 0 B
build/edit-widgets/style-rtl.css 3.16 kB 0 B
build/edit-widgets/style.css 3.16 kB 0 B
build/editor/editor-styles-rtl.css 476 B 0 B
build/editor/editor-styles.css 478 B 0 B
build/editor/index.js 42.3 kB 0 B
build/editor/style-rtl.css 3.89 kB 0 B
build/editor/style.css 3.89 kB 0 B
build/element/index.js 4.62 kB 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/index.js 6.75 kB 0 B
build/format-library/style-rtl.css 620 B 0 B
build/format-library/style.css 621 B 0 B
build/hooks/index.js 2.27 kB 0 B
build/html-entities/index.js 623 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 698 B 0 B
build/keyboard-shortcuts/index.js 2.54 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.15 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/media-utils/index.js 5.31 kB 0 B
build/notices/index.js 1.86 kB 0 B
build/nux/index.js 3.42 kB 0 B
build/nux/style-rtl.css 731 B 0 B
build/nux/style.css 727 B 0 B
build/plugins/index.js 2.54 kB 0 B
build/primitives/index.js 1.43 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/reusable-blocks/index.js 2.91 kB 0 B
build/rich-text/index.js 13.5 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 3.01 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

@kjellr
Copy link
Contributor

kjellr commented Jan 5, 2021

Thanks for taking on this long-standing issue!

The block is there, even if it's empty.

While this definitely does remove visual clutter, I am concerned that at a glance it implies that there's space there — without a placeholder, all those empty paragraphs look just like a deselected spacer block. But they won't render that way on the front end (Related: #10051).

This PR adds an attribute to the paragraph block saying whether it's empty or not (is there a better way to do this?)

It's a little unfortunate, but yeah I think an attribute is the best technical solution. :empty seems like it should work, but since that is only relevant for the nested [data-rich-text-placeholder] selector, it doesn't actually work here.

@jasmussen
Copy link
Contributor Author

Thank you Kjell, and happy new year by the way!

But they won't render that way on the front end

Delicious point. That seems like a problem to solve, honestly!

I took a stab and even included your :empty idea, which we can't use in the editor, but can on the frontend. Editor:

Screenshot 2021-01-05 at 15 14 36

Frontend:

Screenshot 2021-01-05 at 15 14 15

This is the code that does it:

p:empty::before {
	content: "\a0";
}

Basically that outputs a non-breaking space in an empty paragraph, making the empty paragraph technically not be empty, but having the exact same height as if it had text content. Behold:

Screenshot 2021-01-05 at 15 11 55

Screenshot 2021-01-05 at 15 12 07

Fixes #10051

This obviously needs a total sanity check by you and others. CC: @jffng @scruffian @MaggieCabrera and anyone else who might have an opinion, because it basically adds an unscoped rule to the empty paragraph. But the question is: should you ever have an empty paragraph that wasn't the height of a non-empty single line paragraph?

@kjellr
Copy link
Contributor

kjellr commented Jan 5, 2021

I like that. 😄 But I also wonder if rolling it out at this stage will cause a bunch of headaches for existing sites?

@mkaz
Copy link
Member

mkaz commented Jan 6, 2021

I think rolling out the height of an empty paragraph would require updating themes, so it would only be available in themes that support it. But also it is counter to how HTML defaults work, an empty paragraph tag has no height by default, Altering this might have unintended consequences with people's content.

A question I have for this ticket, is it to reduce noise in the editor? Or provide a way for a user to create space around content? Both? Would a dismissable tooltip work? Something that introduces them to the Spacer block, maybe triggered after 3 new empty paragraph tags?

@jasmussen
Copy link
Contributor Author

I like that. 😄 But I also wonder if rolling it out at this stage will cause a bunch of headaches for existing sites?

We ran a quick anonymous data sampling on WordPress.com websites, and found that about 1.6% of published posts/pages contained <p></p>. I personally think that's a small enough number that this is a change worth making. And although it's already existed for a while, it'd be good to get this out before full site editing, IMO.

A question I have for this ticket, is it to reduce noise in the editor? Or provide a way for a user to create space around content? Both?

It's to further the consistency and WYSIWYG between the editor and the frontend. If you make and see a linebreak in the editor, that linebreak should also be present on the frontend. From #10051, this is the current behavior:

I'd think the most important aspect of this PR, is to make left and right look mostly the same.

@DaisyOlsen
Copy link
Contributor

My Opinion, some of this has already been touched on in previous comments.

Because the empty paragraphs are collapsed on the front end, it's likely that most of those empty paragraph blocks are there by mistake from hitting enter or clicking into the insertion point and then not adding anything. I have done it a million times, and it's a pet peeve of mine.

The problem I think would be better to solve here is to make it less likely that empty blocks will be created. Adding space to paragraphs added by accident will create more problems than it will solve. Also, I definitely wouldn't want to encourage content creators to use empty paragraph blocks as spacers.

@jasmussen
Copy link
Contributor Author

@sirreal I know in the past you have been very observant on side effects from pull requests like this one. If you have time (thank you, and not urgent), and given the context in #27995 (comment), I would very much value your thoughts as well.

@jasmussen
Copy link
Contributor Author

I have refocused this PR to be only about fixing #10051. The other experiment can be revisited, but did not set hearts on fire and also needed extra work.

@jasmussen jasmussen changed the title Try: Hide adjacent empty paragraphs. Try: Make empty paragraphs take up the same space on the frontend, as in the editor Jan 14, 2021
@jasmussen
Copy link
Contributor Author

I have updated the PR to reflect the focus on just the frontend backend consistency. Would appreciate thoughts. As a reminder, we got some numbers in this comment on how many posts or pages might have empty paragraphs.

@jasmussen jasmussen requested a review from a team January 14, 2021 11:50
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.

Simple PR, improves consistency between frontend and backend and I can't see any drawbacks 👍

@jasmussen
Copy link
Contributor Author

Thank you for the review, Ari!

The potential downside is that people might have accidentally created a bunch of empty paragraphs without knowing it. For It's easy to do in blocks that feature nesting, for example. And when we then roll this out, suddenly those empty paragraphs would take up space.

I happen to agree this is worth doing, but I'll leave it up unmerged to gather some additional opinions.

@aristath
Copy link
Member

aristath commented Jan 14, 2021

The potential downside is that people might have accidentally created a bunch of empty paragraphs without knowing it. For It's easy to do in blocks that feature nesting, for example. And when we then roll this out, suddenly those empty paragraphs would take up space.

Yeah, I agree... But that would actually be a "bug" in their content. This PR will make it more apparent and they can then fix their content - something they could not do before this PR since they didn't even know it was an issue.
So while it might cause some temporary inconvenience in these cases, long-term I believe it would be worth it ❤️

@scruffian
Copy link
Contributor

I think user's expectations when working with an editor like Gutenberg are probably heavily influenced by their experience of other text editors and word processors. In these system it is very expected that an empty paragraph should take up space, and removing that probably is a source of frustration to many users. I agree that we would rather users were inserting spacer blocks rather than empty paragraphs, but I don't think that collapsing paragraph blocks is the way to do that, as it simply causes confusion - why do things look one way in the editor and another way in the front end. All of which is to say I support this change!

I also share the concern that this might cause layouts to break from people accidentally inserting empty paragraphs, but it is very hard to know the extent of the problem. My feeling is that its worth trying to make this change and if we get a lot of feedback from people using trunk that it's a problem then we revert before we get to the next release.

@draganescu draganescu merged commit 479a01e into master Jan 14, 2021
@draganescu draganescu deleted the try/clean-empty-paragraphs branch January 14, 2021 15:43
@github-actions github-actions bot added this to the Gutenberg 9.9 milestone Jan 14, 2021
Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

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

@sirreal I know in the past you have been very observant on side effects from pull requests like this one. If you have time (thank you, and not urgent), and given the context in #27995 (comment), I would very much value your thoughts as well.

Thanks! I've been sensitive to structural changes to the HTML, but this solution doesn't include any which is quite nice!

This will visibly change site content on the frontend, where collapsed space is now expanded. Is that correct? That can be disruptive for users, but it should be more intuitive for content to match between the editor and the frontend. Let's make sure to call out the possible visual/layout changes in release notes.


// Prevent an empty P tag from collapsing, so it matches the backend.
p:empty::before {
content: "\a0";
Copy link
Member

Choose a reason for hiding this comment

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

This is NO-BREAK SPACE, the same as the &nbsp; pretty common in HTML. I'm not sure how screen readers handle this or whether there are considerations in the accessibility space to take into account.

There are some other characters that we could consider to ensure the p elements don't collapse, ZERO WIDTH SPACE \200B seems like a viable alternative. The advantage is that it's… zero width. It doesn't take up space.

zero-width.mov

Copy link
Member

@sirreal sirreal Jan 14, 2021

Choose a reason for hiding this comment

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

Naturally I got that wrong in the demo. The zero width space is aboove and the no-break space is below in the example. Zero width is the one that has… 0 width 🤦

@youknowriad youknowriad added the Needs Dev Note Requires a developer note for a major WordPress release cycle label Jan 14, 2021
@youknowriad
Copy link
Contributor

I wonder if it's time to remove the placeholders from empty paragraphs 🤷‍♂️

@jasmussen
Copy link
Contributor Author

Awesome, thank you all for the sanity checks.

Some legitimate concerns have been voiced. I will be sure to write a dev note, call it out in the theme update notes, and follow up with any PRs necessary. That's the good thing, this is a very very small change, and it's trivial to revert if unforeseen problems arise from this, and if that happens, at least we'll have tried it.

I will also follow up with a PR to address this good point.

@jasmussen
Copy link
Contributor Author

I wonder if it's time to remove the placeholders from empty paragraphs 🤷‍♂️

This PR actually started out as an effort to do just that, but the code I wrote for it was a little ugly, and I pared it back to focus things up: b8bd5b1

Here's how that looked:

I'm happy to revisit, notably #13599 and #17366 are still open, and could benefit from some thinking here. But this felt like a good start.

@jasmussen
Copy link
Contributor Author

Suggested dev note:

In 27995 the default behavior of a published empty paragraph (<p></p>) changed. Before, the tag would collapse to zero width and zero height, and be inconsistent with what users saw in the editor. Now an invisible space is output in empty paragraphs, ensuring linebreaks in the editor correspond to linebreaks on the frontend.
There is a chance of empty paragraphs accidentally published, which will now take up space. If that is the case, it is easily fixed by deleting the empty paragraphs.

@paaljoachim
Copy link
Contributor

paaljoachim commented Jan 18, 2021

Hey I am reading through this PR and I am not fully certain of what will happen, and how it works when I test the next version of the Gutenberg plugin.

I am very hesitant.
I do believe the easiest method to actually add some space in backend and frontend is to after clicking 3 or more enter times have it automatically converted to a spacer. The user will have a spacer block to relate to in the backend. This can then be adjusted in size by the user. Creating a more precise control. #17366

  1. Why click enter multiple times? I know that I usually do this just to create some air/space.
  2. Handling empty paragraphs would be easiest to handle in one spacer block where one can easily adjust the height.
  3. As blocks gradually will have padding and margins. There will be less need to click enter a few times just to create some space.

NB! Bottom line is. I believe we just need to test it out and see how it works.

@aristath
Copy link
Member

Why click enter multiple times? I know that I usually do this just to create some air/space.

The answer is in the 2nd part of that sentence... Sometimes people add empty lines in order to add some breathing room to their sentences. The problem whith that however is that there was an inconsistency between the editor and the frontend. This PR just makes the frontend look as the editor does, so any whitespace added on the editor side will now be visible on the frontend.

Handling empty paragraphs would be easiest to handle in one spacer block where one can easily adjust the height.

True, but people used to working with any text editor just instinctively double-hit enter when they want/need some extra space.

As blocks gradually will have padding and margins. There will be less need to click enter a few times just to create some space.

Agreed. However, adding paddings/margins etc takes a lot more time... You need to stop typing, get the mouse, go to a block's settings, and edit the paddings/margins etc. Double-tapping the enter key is something that comes naturally to a lot of people so we should take that into account (which is what this PR does) 👍

@noisysocks noisysocks mentioned this pull request Jan 28, 2021
9 tasks
jasmussen added a commit that referenced this pull request Feb 8, 2021
jasmussen added a commit that referenced this pull request Mar 8, 2021
jasmussen added a commit that referenced this pull request Mar 8, 2021
* Try: Hide tip from multiple paragraphs, version 1.

This one revisits work from #27995 (comment).

* Remove hover.
@joyously
Copy link

@jasmussen
There have been several forum topics complaining about legacy content having extra spacing, with this in place in WP 5.7.

My opinion is that the browser has always condensed white space, and this CSS change makes that inconsistent. I think it should be removed.

The Trac ticket is https://core.trac.wordpress.org/ticket/52764

@jasmussen
Copy link
Contributor Author

Thanks for the ping.

It's sad that shortcodes end up causing this editor/frontend de-sync, so I'll ping a few people for thoughts, and in the meantime I'll prepare a revert PR. CC: @kjellr @scruffian @MaggieCabrera @aristath @jameskoster

@StuartKent17
Copy link

As a possible alternate option, would allowing empty Paragraphs to be transformed into Vertical Spacers work? If spacers defaulted to 1em then can be overridden?

@youknowriad youknowriad removed the Needs Dev Note Requires a developer note for a major WordPress release cycle label Jun 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Feedback Needs general design feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggestion: Make empty paragraph blocks behave consistently on the front and backend