[Gutenberg 7.7.1]: Fix editor header layout issues on mobile & tablet view. #40361

yansern · 2020-03-23T10:49:04Z

Changes proposed in this Pull Request

The following changes were made to fix header layout issues mobile & tablet view on Gutenberg 7.7.

Fix visible whitespace in mobile & tablet view. Consider making Fullscreen Mode effects visible only on larger screens WordPress/gutenberg#13425).
Hide Back/Close Button in mobile & tablet view (as how it behaved in previous version).
Hide Table of Contents Button in mobile view (as how it behaved in previous version).
Ensure our custom Block Inserter Button on page editor has a blue background (as seen on newer Gutenberg)

Fixes #40335

Testing instructions

Build @automattic/full-site-editing and sync to wpcom
Build @automattic/wpcom-block-editor and sync to wpcom

Test the editor against the following scenarios:

WP-Calypso Page Editor FSE
https://wordpress.com/block-editor/page/YOUR_FSE_SITE_HERE.wordpress.com
WP-Calypso Page Editor Non-FSE
https://wordpress.com/block-editor/page/YOUR_NON_FSE_SITE_HERE.wordpress.com
WP-Admin Page Editor FSE
https://YOUR_FSE_SITE_HERE.wordpress.com/wp-admin/post-new.php?post_type=page
WP-Admin Page Editor Non-FSE
https://YOUR_NON_FSE_SITE_HERE.wordpress.com/wp-admin/post-new.php?post_type=page
WP-Calypso Post Editor
https://wordpress.com/block-editor/post/YOUR_SITE_HERE.wordpress.com
WP-Admin Post Editor
https://YOUR_SITE_HERE.wordpress.com/wp-admin/post-new.php

Spot for any possible regressions by testing the above scenarios again on another site running non gutenberg-edge version. This is for backwards compatiblity with older Atomic sites.

Note:

You'll need to build and sync over both full-site-editing-plugin and wpcom-block-editor scopes to your sandbox.

Known Issues / Remaining Work

Hamburger Button is overlapping Block Inserter Button on WP-Admin version of the editor. Also take note that this issue already exist in live production so we don't necessarily have to fix it in this PR.
(Optional) The layout may break on fringe viewport widths. I'm limiting the scope to just some of the standard viewport widths as seen in the screenshot below.

Screenshots

WP-Calypso FSE Page Editor

WP-Calypso Non-FSE Page Editor (Has Issues)

WP-Admin FSE Page Editor (Has Issues)

WP-Admin Non-FSE Page Editor (Has Issues)

WP-Calypso Post Editor

WP-Admin Post Editor

matticbot · 2020-03-23T10:49:06Z

Test live: https://calypso.live/?branch=fix/gutenberg-7.7-mobile-header-spacing

matticbot · 2020-03-23T10:52:16Z

Caution: This PR has changes that must be merged to WordPress.com
Hello yansern! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D40677-code before merging this PR. Thank you!

matticbot · 2020-03-23T10:55:47Z

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

simison · 2020-03-23T13:57:10Z

apps/full-site-editing/full-site-editing-plugin/dotcom-fse/editor/style.scss

@@ -35,6 +35,45 @@
 		}
 	}

+	// Taken from styles/starter-page-templates-editor.scss


Could also say taken from https://github.com/WordPress/gutenberg/blob/9f0471e36aaf4a7172cc567494a8b9e4561861db/packages/base-styles/_breakpoints.scss ;-)

We can also use https://github.com/WordPress/gutenberg/blob/9f0471e36aaf4a7172cc567494a8b9e4561861db/packages/base-styles/_mixins.scss

But for now for this fix this is fine. 👍

Thanks. These are the list of breakpoints I was looking for:
https://github.com/WordPress/gutenberg/blob/9f0471e36aaf4a7172cc567494a8b9e4561861db/packages/base-styles/_breakpoints.scss

I didn't want to just make up my own numbers. :)

Addison-Stavlo

I think there are a couple issues here:

First Issue I notice is that when loading the editor from wp-admin (non-iframe), there are some screen widths where the toolbar gets hidden underneath the wp-admin toolbar:
Do all of these changes make sense to do from the dotcom FSE plugin? Hiding the dotcom FSE back button and adding padding to the inserter due to the custom button make sense, but the other pieces may need to be handled outside of the dotcom FSE plugin as I believe these issues may exist elsewhere?
Changes to the custom back buttons visibility (and probably style changes it requires to happen to other things as the inserter padding) should probably be handled in dotcom-fse/plugins/close-button-override/style.scss

Addison-Stavlo · 2020-03-23T18:40:22Z

apps/full-site-editing/full-site-editing-plugin/dotcom-fse/editor/style.scss

+
+	@media screen and ( max-width: $breakpoint-tablet ) {
+		// Removes whitespace above header in mobile & tablet view.
+		.block-editor-editor-skeleton {


Perhaps this is the culprit for why we get hidden under the wp-admin bar on this PR?

Thanks! Fixed in this commit: a7295c3

Reasoning for the top whitespace can be traced back to: WordPress/gutenberg#13425

Addison-Stavlo · 2020-03-23T18:41:33Z

apps/full-site-editing/full-site-editing-plugin/dotcom-fse/editor/style.scss

+
+		.edit-post-header__toolbar {
+			// Back button is not displayed in mobile & tablet view.
+			.edit-post-fullscreen-mode-close__toolbar__override {


If we go to dotcom-fse/plugins/close-button-override, there is a scss file in there where changes to this buttons visibility may make more sense.

Thanks! Fixes in this commit. e6d5287

noahtallen · 2020-03-24T02:03:29Z

definitely a +1 to what Addie said. i wanted to add that the code here only resolves issues if dotcom-fse is active, so it wouldn't affect most sites. I think #40335 mentions that there are similar issues with non-fse sites

yansern · 2020-03-24T11:20:18Z

@Addison-Stavlo Thanks for your review & feedback. I've taken care of point 1 and 3 (with consideration of what you have said in point 2).

However, I'm changing the status of this PR from Needs Review to In Progress. This is because I reverted some of the fixes I made and reworking them. I'm going more investigative work on what's the right CSS fix and which file they should land on (based on point 2).

apps/wpcom-block-editor/src/calypso/features/iframe-bridge-server.scss

yansern · 2020-03-24T13:23:15Z

PR updated new descriptions, test instructions, known/remaining issues and new screenshots (which also show some of the errors remaining).

vindl · 2020-03-24T15:34:07Z

Here are a couple of notes from my testing:

Hide Back/Close Button in mobile & tablet view (as how it behaved in previous version).

It sounds like this was the case in previous versions too, but now there is no way to exit the editor on mobile and tablet views. It's probably not a blocker given the previous state of things, but could we at least file a follow up issues for this and add it to the maintenance board?

Hamburger Button is overlapping Block Inserter Button on WP-Admin version of the editor. Also take note that this issue already exist in live production so we don't necessarily have to fix it in this PR.

This could be bundled in the above issue too. It's better if we can come up with a unified strategy for closing the editor, instead of having different behavior in wp-admin and Calypso.

The layout may break on fringe viewport widths. I'm limiting the scope to just some of the standard viewport widths as seen in the screenshot below.

For the record I've noticed this on widths between 600px and 800px while testing on already published posts with English (might be different depending translation length):

Spot for any possible regressions by testing the above scenarios again on another site running non gutenberg-edge version. This is for backwards compatiblity with older Atomic sites.

I noticed a small issue on dotcom-FSE simple sites while testing without the sticker (the Page Layout has no left margin):

Taking into account the number of affected users and wighting this against the other fixes and benefits that the update will bring, I think we shouldn't block on any of these, but it would be good to record the items that haven't been addressed in follow-up issues.

vindl

This PR now needs a rebase.

Be more specific with fixes. Ensure our custom inserter button on page editor also has a blue background just like in newer Gutenberg. Proper overrides for close button. Proper fix for whitespace above mobile header. Revert old overrides which I didn't fully understand the context. Should be top: 0. Balance out side padding. Hide close button label >= 960px to make room for others, hide toc button, apply padding balancing to all editor scenarios. Fix regression on header whitespace.

simison · 2020-03-24T16:05:15Z

Rebased (and squashed to one commit to make rebasing easier)

stale

Addison-Stavlo

Thanks for the updates! This is looking better and the wp-admin top bar seems fixed.

One thing I am finding now (perhaps as a result of the recent rebase), is a large amount of extra space at the top of the bar in the iframed/calypso tablet view ( <~780px )

simison · 2020-03-24T17:38:30Z

Thanks for the updates! This is looking better and the wp-admin top bar seems fixed.

One thing I am finding now (perhaps as a result of the recent rebase), is a large amount of extra space at the top of the bar in the iframed/calypso tablet view ( <~780px )

This seemed to be due needing to build and sync @automattic/wpcom-block-editor :-)

simison

Tested all scenarios pretty thoroughly. (FSE/non-FSE,calypso iframe/directly in wp-admin/different screen sizes/while drinking coffee/tee).

I agree with your notion to leave other issues to follow-ups. :-)

Addison-Stavlo

This seemed to be due needing to build and sync @automattic/wpcom-block-editor :-)

Ah yes! Well that makes sense. After syncing the bridge server build that goes away.
After syncing that as well I do now notice a padding issue in the iframed version that does not exist in the wp-admin version:

But if that is truly an issue past my environment it should be added to the list of follow ups, and not hold this up any further.

noahtallen · 2020-03-24T17:56:08Z

@Addison-Stavlo I wonder if the padding is fixed in #40329. I think there was some work towards it

Addison-Stavlo · 2020-03-24T18:33:29Z

I think this was rebased with those changes. Maybe that rebase gunked it up here? (Or maybe im the only one that sees it 😆 )

yansern · 2020-03-25T05:09:55Z

@noahtallen @Addison-Stavlo
Padding issue is related to conflict of opinion on how to handle the margins for the close button.

The implementation by @razvanpapadopol here https://github.com/Automattic/wp-calypso/pull/40329/files#diff-6ef79f4807f019b9b2b7cd070541b6ddR55-R60 and this is mine https://github.com/Automattic/wp-calypso/pull/40361/files#diff-6ef79f4807f019b9b2b7cd070541b6ddL18-R23.

There's no right or wrong, just which approach to proceed with. Of course, I vote mine, lol :P

My argument is that overrides should be less disruptive, that's why if an offset of margin-left: -24px is needed on an injected custom element, its disruption should be restored with margin-right: 24px so that the adjacent elements would behave as if there were no modifications in place.

I will review what's the current state of editors now that all is merged to master... and add the additional fixes in new PRs.

yansern · 2020-03-25T07:38:47Z

This PR #40425 fixes:

Padding/margin issue on block inserter button as reported by @Addison-Stavlo & @noahtallen.

After syncing that as well I do now notice a padding issue in the iframed version that does not exist in the wp-admin version:

Regressions on the close button margin-left issue on non-gutenberg-edge dotcomfse page editor as reported by @vindl.

I noticed a small issue on dotcom-FSE simple sites while testing without the sticker (the Page Layout has no left margin):

yansern · 2020-03-25T07:57:20Z

@vindl Here are the new follow-up issues created.

Hide Back/Close Button in mobile & tablet view (as how it behaved in previous version).

It sounds like this was the case in previous versions too, but now there is no way to exit the editor on mobile and tablet views. It's probably not a blocker given the previous state of things, but could we at least file a follow up issues for this and add it to the maintenance board?

Hamburger Button is overlapping Block Inserter Button on WP-Admin version of the editor. Also take note that this issue already exist in live production so we don't necessarily have to fix it in this PR.

This could be bundled in the above issue too. It's better if we can come up with a unified strategy for closing the editor, instead of having different behavior in wp-admin and Calypso.

Hamburger button overlapping + no way to exit editor in mobile & tablet view.
#40426

The layout may break on fringe viewport widths. I'm limiting the scope to just some of the standard viewport widths as seen in the screenshot below.

For the record I've noticed this on widths between 600px and 800px while testing on already published posts with English (might be different depending translation length):

Editor header layout break in certain viewport width.
#40427

vindl · 2020-03-25T17:19:11Z

@vindl Here are the new follow-up issues created.

Great, thanks for creating these! I added them to Create Focus maintenance board so we don't lose track of them.

jasmussen · 2020-04-22T10:11:22Z

Hi @yansern 👋

I'd like to know more about the change made in dfe7550#diff-9dbcb1d2dc7c57c8d265da14297e9223R57. The left padding was intentionally designed to be 24px because it's a blue button next to a dark square, whereas the button on the right, the ellipsis/more menu, is a thin vertical icon. In having different margins, the end result is optically balanced:

I would suggest that it is more productive to disagree in the block editor repository where if we find a new consensus we can change the code at the source. By creating CSS overrides like this, you are likely to both create technical debt as the structure and markup of the core editor changes, and it also works against the concerted design effort that takes place there.

Happy to review your PRs in the public repository, you can ping me at @jasmussen and I'll help shepard them along.

simison · 2020-04-22T10:30:02Z

Yep, that can be removed. @alshakero @andrewserong could you add this to the list of maintainance tasks? Thanks!

andrewserong · 2020-04-23T00:36:54Z

Thanks for the ping, I've added it to the list!

yansern added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Goal] Gutenberg Working towards full integration with Gutenberg [Goal] Full Site Editing labels Mar 23, 2020

yansern self-assigned this Mar 23, 2020

yansern requested a review from a team March 23, 2020 10:49

simison reviewed Mar 23, 2020

View reviewed changes

simison requested a review from a team March 23, 2020 13:58

simison changed the title ~~[Gutenberg 7.7.1]: Fix editor header layout spacing issues on mobile & tablet.~~ [Gutenberg 7.7.1]: Fix FSE editor header layout spacing issues on mobile & tablet. Mar 23, 2020

Addison-Stavlo previously requested changes Mar 23, 2020

View reviewed changes

yansern requested a review from a team as a code owner March 24, 2020 11:13

yansern added the [Status] In Progress label Mar 24, 2020

yansern changed the title ~~[Gutenberg 7.7.1]: Fix FSE editor header layout spacing issues on mobile & tablet.~~ [Gutenberg 7.7.1]: Fix editor header layout issues on mobile & tablet view. Mar 24, 2020

yansern commented Mar 24, 2020

View reviewed changes

apps/wpcom-block-editor/src/calypso/features/iframe-bridge-server.scss Outdated Show resolved Hide resolved

vindl added the [Status] Needs Rebase label Mar 24, 2020

vindl reviewed Mar 24, 2020

View reviewed changes

simison force-pushed the fix/gutenberg-7.7-mobile-header-spacing branch from 89a93a8 to 9036d3b Compare March 24, 2020 16:05

simison removed the request for review from a team March 24, 2020 16:05

Addison-Stavlo reviewed Mar 24, 2020

View reviewed changes

simison approved these changes Mar 24, 2020

View reviewed changes

simison merged commit dfe7550 into master Mar 24, 2020

simison deleted the fix/gutenberg-7.7-mobile-header-spacing branch March 24, 2020 17:46

simison removed [Status] In Progress [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] Needs Rebase labels Mar 24, 2020

Addison-Stavlo approved these changes Mar 24, 2020

View reviewed changes

ramonjd mentioned this pull request Mar 25, 2020

Plugin updates: Gutenberg 7.7.1 #40078

Closed

23 tasks

andrewserong mentioned this pull request Apr 20, 2020

Plugin updates: Gutenberg v7.9.1 #41265

Closed

11 tasks

andrewserong mentioned this pull request Apr 23, 2020

Remove header padding override introduced in #40361 #41386

Closed

andrewserong mentioned this pull request Apr 23, 2020

Remove margin-left override on block-editor-inserter__toggle in wpcom-block-editor #41392

Merged

razvanpapadopol mentioned this pull request Sep 1, 2021

Editor: button to exit editor hidden on mobile screens #51162

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Gutenberg 7.7.1]: Fix editor header layout issues on mobile & tablet view. #40361

[Gutenberg 7.7.1]: Fix editor header layout issues on mobile & tablet view. #40361

yansern commented Mar 23, 2020 •

edited by simison

Loading

matticbot commented Mar 23, 2020

matticbot commented Mar 23, 2020

matticbot commented Mar 23, 2020

simison Mar 23, 2020

yansern Mar 24, 2020

Addison-Stavlo left a comment

Addison-Stavlo Mar 23, 2020 •

edited

Loading

yansern Mar 24, 2020

Addison-Stavlo Mar 23, 2020

yansern Mar 24, 2020

noahtallen commented Mar 24, 2020

yansern commented Mar 24, 2020

yansern commented Mar 24, 2020

vindl commented Mar 24, 2020

vindl left a comment

simison commented Mar 24, 2020 •

edited

Loading

Addison-Stavlo left a comment

simison commented Mar 24, 2020

simison left a comment

Addison-Stavlo left a comment •

edited

Loading

noahtallen commented Mar 24, 2020

Addison-Stavlo commented Mar 24, 2020

yansern commented Mar 25, 2020

yansern commented Mar 25, 2020 •

edited

Loading

yansern commented Mar 25, 2020

vindl commented Mar 25, 2020

jasmussen commented Apr 22, 2020 •

edited

Loading

simison commented Apr 22, 2020

andrewserong commented Apr 23, 2020

[Gutenberg 7.7.1]: Fix editor header layout issues on mobile & tablet view. #40361

[Gutenberg 7.7.1]: Fix editor header layout issues on mobile & tablet view. #40361

Conversation

yansern commented Mar 23, 2020 • edited by simison Loading

Changes proposed in this Pull Request

Testing instructions

Known Issues / Remaining Work

Screenshots

WP-Calypso FSE Page Editor

WP-Calypso Non-FSE Page Editor (Has Issues)

WP-Admin FSE Page Editor (Has Issues)

WP-Admin Non-FSE Page Editor (Has Issues)

WP-Calypso Post Editor

WP-Admin Post Editor

matticbot commented Mar 23, 2020

matticbot commented Mar 23, 2020

matticbot commented Mar 23, 2020

simison Mar 23, 2020

Choose a reason for hiding this comment

yansern Mar 24, 2020

Choose a reason for hiding this comment

Addison-Stavlo left a comment

Choose a reason for hiding this comment

Addison-Stavlo Mar 23, 2020 • edited Loading

Choose a reason for hiding this comment

yansern Mar 24, 2020

Choose a reason for hiding this comment

Addison-Stavlo Mar 23, 2020

Choose a reason for hiding this comment

yansern Mar 24, 2020

Choose a reason for hiding this comment

noahtallen commented Mar 24, 2020

yansern commented Mar 24, 2020

yansern commented Mar 24, 2020

vindl commented Mar 24, 2020

vindl left a comment

Choose a reason for hiding this comment

simison commented Mar 24, 2020 • edited Loading

Addison-Stavlo left a comment

Choose a reason for hiding this comment

simison commented Mar 24, 2020

simison left a comment

Choose a reason for hiding this comment

Addison-Stavlo left a comment • edited Loading

Choose a reason for hiding this comment

noahtallen commented Mar 24, 2020

Addison-Stavlo commented Mar 24, 2020

yansern commented Mar 25, 2020

yansern commented Mar 25, 2020 • edited Loading

yansern commented Mar 25, 2020

vindl commented Mar 25, 2020

jasmussen commented Apr 22, 2020 • edited Loading

simison commented Apr 22, 2020

andrewserong commented Apr 23, 2020

yansern commented Mar 23, 2020 •

edited by simison

Loading

Addison-Stavlo Mar 23, 2020 •

edited

Loading

simison commented Mar 24, 2020 •

edited

Loading

Addison-Stavlo left a comment •

edited

Loading

yansern commented Mar 25, 2020 •

edited

Loading

jasmussen commented Apr 22, 2020 •

edited

Loading