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

Fix Site Editor close button for G v11.9 #57909

Merged
merged 1 commit into from
Nov 10, 2021

Conversation

Addison-Stavlo
Copy link
Contributor

@Addison-Stavlo Addison-Stavlo commented Nov 10, 2021

Changes proposed in this Pull Request

With Gutenberg v11.9, the navigation toggle was removed. This caused our override to close the editor to no longer work, thus the editor in 11.9 now tries to close to wp-admin instead of calypso, which is not allowed and results in this gray screen:

Screen Shot 2021-11-10 at 10 20 07 AM

Here, we add the selector for the new close button to keep the same override functionality. Once v12.0 is shipped to dotcom, we can get rid of this hacky goo altogether and go back to using the slot/fill for overriding the close button.

Testing instructions

  • Patch this diff to your sandbox
  • On a non-edge site, open the site editor and verify closing behavior is unchanged.
  • Patch D69833-code to your sandbox to enable the new entry point.
  • On an edge site, open the site editor and verify closing leads back to calypso.

Related to #

@Addison-Stavlo Addison-Stavlo requested review from vindl, Copons and a team November 10, 2021 15:15
@Addison-Stavlo Addison-Stavlo self-assigned this Nov 10, 2021
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 10, 2021
@matticbot
Copy link
Contributor

This PR modifies the release build for editing-toolkit

To test your changes on WordPress.com, run install-plugin.sh editing-toolkit fix/site-editor-close-button-override-11.9 on your sandbox.

To deploy your changes after merging, see the documentation: PCYsg-mMA-p2

@github-actions
Copy link

@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.

@Addison-Stavlo Addison-Stavlo merged commit ccda3ba into trunk Nov 10, 2021
@Addison-Stavlo Addison-Stavlo deleted the fix/site-editor-close-button-override-11.9 branch November 10, 2021 15: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 Nov 10, 2021
@fullofcaffeine
Copy link
Contributor

Curious about this:

Once v12.0 lands, we should be able to use a slotfill and get rid of this entire file.

Does that mean there's a fix for this in core that was introduced and scheduled for v12.0?

@creativecoder
Copy link
Contributor

@fullofcaffeine yes, see WordPress/gutenberg#36369

@Addison-Stavlo
Copy link
Contributor Author

It looks like that fix is being cherry picked into 11.9. We may need to update things as we are currently filling that slot for its previous context of being inside the nav sidebar and wrapping the < Dashboard button.

nelsonec87 pushed a commit that referenced this pull request Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants