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

Update header button styling and full width block margins for Gutenberg v8.0.0 #41575

Merged

Conversation

andrewserong
Copy link
Member

@andrewserong andrewserong commented Apr 29, 2020

This PR updates the FSE plugin with class names to add support for Gutenberg v8.0.0.

Changes proposed in this Pull Request

  • Update class names used for hiding the close button on mobile viewports
  • Update class names for removing additional margins on full width blocks

Testing instructions

Note: the whitespace at the top is dealt with separately in #41572

Mobile before Mobile after
image image
Full width blocks before Full width blocks after
image image
  • On a site using dotcom FSE, enable the Edge sticker and sandbox this PR using the diff automatically generated below ( D42611-code )
  • Edit a page, and on mobile the close / back button should be hidden as in the above screenshot
  • Edit a page that contains full width blocks, and they should not overflow the page 'container', as in the above screenshot
  • Remove the Edge sticker from your site and reload the editor, and make sure the styling still looks correct

Fixes #41513
Fixes part of #41521 (the other part is dealt with in #41572)

@matticbot
Copy link
Contributor

@andrewserong andrewserong requested review from a team and alshakero April 29, 2020 06:34
@andrewserong andrewserong self-assigned this Apr 29, 2020
@andrewserong andrewserong added [Goal] Gutenberg Working towards full integration with Gutenberg [Goal] Full Site Editing [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Apr 29, 2020
@matticbot
Copy link
Contributor

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.

@andrewserong
Copy link
Member Author

@Automattic/cylon 👋 — just added you as reviewers, too, but no worries if you don't have time to test it out. One question I had about the deployment process — once this is approved, do I deploy the diff immediately, too, or do we wait until the next FSE plugin release instead of deploying each diff separately?

@matticbot
Copy link
Contributor

Caution: This PR affects files in the FSE Plugin on WordPress.com
Please ensure your changes work on WordPress.com before merging.

D42611-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing in the FSE Plugin for more info: PCYsg-ly5-p2

@andrewserong andrewserong force-pushed the update/fse-plugin-gutenberg-v8.0.0-compatibility branch from 4f3e924 to 8f63603 Compare April 29, 2020 07:53
@alshakero
Copy link
Member

I'm not entirely sure if I'm testing this correctly. But I followed the instructions closely and I can't get the CSS change above in my browser.

I have a FSE site (alshakero.wordpress.com) . With GB 8.0.0-rc.1 and applied the patch, sandboxed my requests, sandboxes s0.wp.com, and still can't see the effect.

It seems that CSS coming from this URL https://s0.wp.com/_static/??-eJylUttygyAQ/aESkkxS40On3wKCui2wDgva/n1XnaTjVPqSN5Zz2aucBtFgSDYkmXrrLckhazmqCEpS+nZWWAMJ450lJjCpPzREL/I/LYRmq5+GBv0f3eByB4Fkm50TBGmlQ+jEisjbpa7O5+NVdg61Ys7sSdIApe3XE9YGE9cmWrKr72/8hOnSr9AOm8/7CEL+WhPsY49kynh24kgyzWM4cLgDthg9FbDlJbwNuUAwinqNKpoC7rhOkZSeJ7vPmAsvQNGOQIChJPWsVQVsvaRSWxpzKWlQ49LwVst36LLhi5kpHA8IvMlY8ODj7mwqZV/2DU1xIe50fEDv/u10vVV1fXq9VB8/uQM9CQ==?cssminify=yes is outdated and I don't know how to access the new CSS proposed here.

@noahtallen
Copy link
Contributor

Not sure if this is helpful, but I always sandbox the public API and the site I am testing against and that gets the changes to show up. You can also checkout syncing directly to your sandbox (i.e. yarn dev --sync) (should be in the FG page linked above)

once this is approved, do I deploy the diff immediately, too, or do we wait until the next FSE plugin release instead of deploying each diff separately?

good question!

  1. do not deploy this diff to simple sites unless it's urgent. You can and it won't break anything, but you wouldn't be able to deploy this exact code to atomic sites
  2. follow instructions at PCYsg-mMA-p2 to release a new version. (there are no scheduled releases right now, it's all ad hoc). You have to go through bumping the version of the plugin before releasing it if you want to get the changes deployed to the dotorg plugin repo, and hence to Atomic sites :)

@andrewserong
Copy link
Member Author

andrewserong commented Apr 30, 2020

Great, thanks for the tips @noahtallen!

@alshakero not sure if I can add anything extra to what Noah's already said, but I applied the diff to my sandbox to double-check this, and these were the steps I followed:

  • Applied D42611-code in my sandbox
  • Added the following to my hosts file, mapped to my sandbox:
    • public-api.wordpress.com
    • widgets.wp.com
    • s0.wp.com s1.wp.com s2.wp.com
    • andyslegacyfullsiteeditingsite . wordpress.com (but remove the spaces)

The dotcom-fse.css file is then being loaded from: https://andyslegacyfullsiteeditingsite . wordpress.com/wp-content/plugins/full-site-editing-plugin/90932076/dotcom-fse/dist/dotcom-fse.css?m=1588232461h&ver=1588232461 (with the spaces removed)

@andrewserong andrewserong added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Apr 30, 2020
@alshakero
Copy link
Member

Goddammit I forgot to sandbox the site itself! 🤕

@ramonjd
Copy link
Member

ramonjd commented Apr 30, 2020

Goddammit I forgot to sandbox the site itself!

Wait until you forget to ssh, and spend 30 mins wondering why it won't work. Then we can talk 🤣

Copy link
Member

@alshakero alshakero left a comment

Choose a reason for hiding this comment

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

Tested for both changes and LGTM

@andrewserong andrewserong merged commit 52db4bc into master May 4, 2020
@andrewserong andrewserong deleted the update/fse-plugin-gutenberg-v8.0.0-compatibility branch May 4, 2020 03:56
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Goal] Full Site Editing [Goal] Gutenberg Working towards full integration with Gutenberg
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wpcom FSE compatibility with Gutenberg v8.0.0: full width blocks overflow the page container
5 participants