-
Notifications
You must be signed in to change notification settings - Fork 2k
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 7.7.1]: Fix editor close button #40329
Conversation
- update handleCloseEditor fn - update tracking - update e2e gutenberg lib
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. |
Caution: This PR has changes that must be merged to WordPress.com |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Just a note that we'll have to be careful with deployment order here. Block editor diff should probably land around the same time we land Gutenberg plugin update, and at that time we should also sync FSE new plugin version containing these changes. |
Remember that Atomic users can have older Gutenberg version so we should support both for a while. I think there is a window global for the version? |
|
I think instead of detecting the version, we could just keep both .css selectors instead of removing the old .css selectors. |
If we merge the PR with support for 7.3, then it can be deployed before releasing 7.7.1 (which is ideal, as we don't want the back button to stop working for some gap period of time) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I "audited" the different classnames to see which ones should work for WP 5.4 AND Gutenberg 7.7.1+, and which ones are fine to drop compatibility with WP 5.4.
It might cause some visual inconsistency in dotcom-FSE if you've turned of the Gutenberg plugin. But honestly, the plugin is designed to only work with the latest version, and there aren't a large enough number of people using it in the first place. So the subset that could see weird visual things is pretty small (i.e. Atomic user who has kept their original theme and has turned off the Gutenberg plugin).
I did test that this fixes the FSE back button issues on 7.7.1, but I didn't get a chance to test the wpcom fixes. They look good to me though :)
apps/wpcom-block-editor/src/wpcom/features/tracking/wpcom-block-editor-close-click.js
Show resolved
Hide resolved
apps/wpcom-block-editor/src/calypso/features/iframe-bridge-server.js
Outdated
Show resolved
Hide resolved
...ll-site-editing/full-site-editing-plugin/dotcom-fse/plugins/close-button-override/style.scss
Outdated
Show resolved
Hide resolved
...ll-site-editing/full-site-editing-plugin/dotcom-fse/plugins/close-button-override/style.scss
Outdated
Show resolved
Hide resolved
...ll-site-editing/full-site-editing-plugin/dotcom-fse/plugins/close-button-override/style.scss
Show resolved
Hide resolved
…lugin - Add legacy selector for close editor button
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a lot of testing for this PR, and it works fantastically. It can be shipped immediately to simple sites and to FSE
By the way, I also created this diff for the wpcom block editor changes. That can be done at any time now that this works for the old and new versions: D40709-code.
dotcom-fse
With Gutenberg edge:
✅ SPT navigation on new pages
✅ navigating back from new/existing pages
✅ navigating to and back from template parts like the header/footer
✅ back from new and existing posts
Without Gutenberg edge:
✅ SPT navigation on new pages
✅ navigating back from new/existing pages
✅ navigating to and back from template parts like the header/footer
✅ back from new and existing posts
non-fse/normal sites
With Gutenberg edge:
✅ SPT navigation on new pages
✅ navigating back from new/existing pages
✅ back from new and existing posts
Without Gutenberg edge:
✅ SPT navigation on new pages
✅ navigating back from new/existing pages
✅ back from new and existing posts
Changes proposed in this Pull Request
Block editor updates:
FSE updates:
Testing instructions
Here is a diff for testing the wpcom-block-editor: D40709-code.
Checkout this PR locally and sync wp-block-editor widget and FSE plugin using FSE Plugin Development instructions in FG.
/block-editor/page/[EDGE_STICKERED_SITE].wordpress.com
Follow up:
Fixes #40315, Fixes #40334
Part of: #40078