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

Gutenberg: Update selected editor when switching to classic (Calypso) #32421

Merged
merged 2 commits into from
Apr 25, 2019

Conversation

mmtr
Copy link
Member

@mmtr mmtr commented Apr 19, 2019

Changes proposed in this Pull Request

Previously, when a user clicked on the "Switch to Classic Editor" button available on the "More tools" menu, we were performing a request to the opt-in API endpoint to set the classic editor as the new desired editor and later redirecting the user to the classic editor URL.

Screen Shot 2019-04-18 at 13 12 47

This approach presents a problem with Jetpack sites since the opt-in API is not available there.

To workaround that, we have two options:

  1. Move the Gutenberg opt-in API to Jetpack.
  2. Delegate the new desired editor update to the controller of the classic editor URL.

On this PR we're exploring the 2nd option, since the 1st one will involve a huge amount of work.

Now, when the "Switch to Classic Editor" button is clicked, we just redirect to the classic editor URL which will include a set-editor=classic param. The Calypso controller of that URL will check that param and update the selected editor to classic when found.

Testing instructions

  • Make sure that your sandbox site has opted in to the block editor.
  • Apply D27149-code and sandbox widgets.wp.com.
  • Go to /block-editor and select your sandbox site.
  • Open the "More tools" menu and click on the "Switch to Classic Editor" button.
  • Make sure the Calypso classic editor is loaded and you see the set-editor=classic param on the URL.
  • Check your current selected editor is classic on the API: GET https://public-api.wordpress.com/wpcom/v2/sites/:site/gutenberg.
  • Switch again to the block editor using the "Learn more" button on the sidebar.

Screen Shot 2019-04-19 at 13 49 24

Screen Shot 2019-04-19 at 13 49 29

  • Make sure the Calypso iframe block editor is loaded and the set-editor=classic param is not present on the URL.
  • Check your current selected editor is gutenberg on the API: GET https://public-api.wordpress.com/wpcom/v2/sites/:site/gutenberg.

@mmtr mmtr 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 Packages labels Apr 19, 2019
@mmtr mmtr self-assigned this Apr 19, 2019
@matticbot
Copy link
Contributor

@matticbot
Copy link
Contributor

matticbot commented Apr 19, 2019

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

Sections (~122 bytes added 📈)

name         parsed_size           gzip_size
post-editor       +122 B  (+0.0%)      +48 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.

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.

@mmtr mmtr changed the title Gutenberg: Update selected editor when switching to classic Gutenberg: Update selected editor when switching to classic (Calypso) Apr 19, 2019
@mmtr mmtr requested a review from a team April 19, 2019 04:51
@obenland
Copy link
Member

It seems like wordpress.com is hard-coded in switchToClassic.url, which didn't let me change the editor. Only after manually putting together the URL I was able to do that. Is that worth changing?

@mmtr
Copy link
Member Author

mmtr commented Apr 23, 2019

It seems like wordpress.com is hard-coded in switchToClassic.url, which didn't let me change the editor. Only after manually putting together the URL I was able to do that. Is that worth changing?

We use wordpress.com when the origin of the requests is not whitelisted. I think you tested from calypso.live which is not whitelisted. Can you try from an environment which is included on the whitelist and confirm if you can reproduce the same issue?

Right now we whitelist these origins (D25762-code):

Copy link
Member

@codebykat codebykat left a comment

Choose a reason for hiding this comment

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

This tests well for me and the API key gets set correctly. My only question (not a blocker) is whether it would be cleaner to drop the parameter off the URL once we've set the editor preference accordingly -- it would look cleaner and not expose the messy inner workings to users.

@mmtr
Copy link
Member Author

mmtr commented Apr 25, 2019

My only question (not a blocker) is whether it would be cleaner to drop the parameter off the URL once we've set the editor preference accordingly -- it would look cleaner and not expose the messy inner workings to users.

I agree it would look cleaner but I don't know how to properly do that. All the approaches I can think of involves a page.redirect to the same URL but removing the no longer needed set-editor param.

That redirect means that the whole controller logic will be executed again so I wonder if there is any way of changing the URL and making sure that the controller ignores that change.

I'll defer this to a follow-up PR and possibly someone else with more knowledge about page.js can get a better alternative :)

@mmtr mmtr force-pushed the update/gutenberg-switch-classic-editor branch from 9ce0249 to 8536378 Compare April 25, 2019 02:31
@mmtr mmtr merged commit 8743ab7 into master Apr 25, 2019
@mmtr mmtr deleted the update/gutenberg-switch-classic-editor branch April 25, 2019 02:45
@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 Apr 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Goal] Gutenberg Working towards full integration with Gutenberg Packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants