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

[GB 13.7.x] Open Site Editor in the Gutenframe by using the site-editor.php route #65997

Merged

Conversation

fullofcaffeine
Copy link
Contributor

@fullofcaffeine fullofcaffeine commented Jul 26, 2022

Proposed Changes

Gutenberg 13.7 deprecates the old themes.php?page=gutenberg-edit-site and admin.php?page=gutenberg-edit-site in favor of WP core's site-editor.php. This PR changes the Site Editor admin route to site-editor.php in the Gutenframe.

Testing Instructions

  • Build Calypso from this branch, access calypso.localhost:3000;
  • Apply this diff to your sandbox: D84749-code and D8488-code -- it has backward-compatible fixes for 13.7 that should be tested with 13.6, too;
  • Open a sandboxed site that still has Gutenberg 13.6;
  • Open the Site Editor by clicking the Appearance->Site Editor menu item. It should open the editor inside the Gutenframe and shouldn't redirect to the wp-admin page!
  • Open the regular editor, it should load correctly, from the Gutenframe, too.
  • Now use a site that has Gutenberg 13.7.3 installed (or apply the gutenberg-edge sticker to your existing site);
  • Reload your local Calypso web app instance in the browser;
  • Test the Site Editor and the editor - they should load from the Gutenframe.

Pre-merge Checklist

Further context:

@fullofcaffeine fullofcaffeine marked this pull request as ready for review July 26, 2022 22:11
@github-actions
Copy link

github-actions bot commented Jul 26, 2022

@matticbot
Copy link
Contributor

matticbot commented Jul 26, 2022

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~32 bytes removed 📉 [gzipped])

name              parsed_size           gzip_size
gutenberg-editor        -40 B  (-0.0%)      -11 B  (-0.0%)
themes                  -20 B  (-0.0%)      -10 B  (-0.0%)
theme                   -20 B  (-0.0%)      -10 B  (-0.0%)
home                    -20 B  (-0.0%)      -11 B  (-0.0%)
checkout                -20 B  (-0.0%)      -10 B  (-0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~11 bytes removed 📉 [gzipped])

name                                                      parsed_size           gzip_size
async-load-calypso-blocks-product-purchase-features-list        -20 B  (-0.0%)      -11 B  (-0.1%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

Copy link
Contributor

@creativecoder creativecoder left a comment

Choose a reason for hiding this comment

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

This is working as expected for me...

Simple sites: now that site-editor.php is a valid route, this works perfectly, loading the Site editor in the Calypso iframe from wp-admin/site-editor.php.

Atomic sites: this won't work until Gutenberg 13.7 is released, as older versions of Gutenberg will redirect from site-editor.php to themes.php?page=gutenberg-edit-site and break out of the iframe. However, I can't get any Atomic site to load the Site editor within the iframe right now.

Jetpack sites: these will already redirect and break out of the iframe when using the latest version of Gutenberg, so this change will be a welcome fix.

But still, I think it's best to wait to deploy this until just before Gutenberg 13.7 is released to Atomic sites.

@creativecoder creativecoder 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 labels Jul 27, 2022
@creativecoder creativecoder requested a review from a team July 27, 2022 15:49
@creativecoder
Copy link
Contributor

Note that I tested this change on its own, apart from the associated diffs.

@fullofcaffeine
Copy link
Contributor Author

Jetpack sites: these will already redirect and break out of the iframe when using the latest version of Gutenberg, so this change will be a welcome fix.

By Jetpack sites, do you mean plain org sites running the Jetpack plugin + connected to Jetpack? I thought those sites didn't use the Gutenframe/Calypso, though?

@fullofcaffeine fullofcaffeine merged commit 3cb6495 into trunk Aug 1, 2022
@fullofcaffeine fullofcaffeine deleted the change/site-editor-route-in-preparation-for-gb-13.7 branch August 1, 2022 18:42
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 1, 2022
@fullofcaffeine fullofcaffeine added the caused-by-gutenberg-upgrade Labels an issue (or PR) as caused by or related to a Gutenberg upgrade on WPCOM. label Aug 1, 2022
@creativecoder
Copy link
Contributor

By Jetpack sites, do you mean plain org sites running the Jetpack plugin + connected to Jetpack? I thought those sites didn't use the Gutenframe/Calypso, though?

You're right, that's still true, afaik, and I forgot about it when I was testing. The Site editor isn't showing in the sidebar for any Jetpack (non-Atomic) sites in Calypso right now, so I was just going directly to the /site-editor/{site} link.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
caused-by-gutenberg-upgrade Labels an issue (or PR) as caused by or related to a Gutenberg upgrade on WPCOM. [Goal] Gutenberg Working towards full integration with Gutenberg
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants