Elementor: partially support document switching #21514

igorschoester · 2024-07-31T14:55:34Z

Context

In Elementor the main document is switched when: going to the site settings or using the recent posts in the top bar feature (beta). Our integration can not handle this.
With these changes it will at least be compatible with the initial document. Meaning the user can open the site settings, without losing our integration.
However, when using the recent posts feature to switch to another post, we hide our sidebar on purpose. As we can not deal with fetching the data for the other post on-the-fly yet.
When switching to another post and then discarding the changes, we will now also discard our form and redux state.

Summary

This PR can be summarized in the following changelog entry:

Fixes a bug where the Yoast tab would disappear when opening and closing the Site Settings in Elementor.

Relevant technical choices:

The original problem is that Elementor switches documents (e.g. post/page/entity) on the fly. Opening their Site Settings will switch the current document, then switch back on closing. They request these documents via AJAX, and along the data here is the tabs and controls for that specific document. This would then override our initial document tab and control registration. And we would not register again due to the PHP side not loading in on the specific AJAX request.

Registered the Yoast tab and control in JS instead of the PHP. There we can better check our form post/page ID against the requested document ID. As we can not handle actually editing anything else than our initial loaded data. (See some PHP alternatives here, though not with the ID comparisons)
- Left in the already refactored way to retrieve the post/post ID: Yoast\WP\SEO\Elementor\Infrastructure\Request_Post
Refactored some code out of the edititor integration file into separate files to decrease the amount of code in there. In the hope this will improve readability.
- Rendering React root and content
- Yoast form watcher
- Checking if a form change is relevant
- Post status watcher
- More menu registration and tab click/key listener
Moved some Elementor specific JS code around to be inside the elementor folder so that it is easier to find. Since we need to test the integration anyway, this seemed like a good moment to sneak that in here.
Added some Elementor hook improvements, needed to be able to get the conditions in there for example. For easy turning off of our callbacks.
Added reducer enhancers for freezing and snapshotting. I don't like these at all.
UI improvements; First screenshot is now, second is before (and Dutch 😇 )
- No margin around our sidebar. Notice the top and left/right space.
- No override on the tab icon color (which is different for an active or an inactive tab)

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

Either have a new site or enable the Editor Top Bar feature: Elementor > Settings > Features
Edit a post/page in Elementor

Switching post/page

Using the recent posts menu item, switch to another post/page
Verify the Yoast tab/sidebar is not there
Switch back to the post/page you were editing
Verify the Yoast tab/sidebar is there

Test the discard

Add/change the focus keyphrase (or any data really)
Switch to a different post/page and discard the changes
Switch back to the original post/page
Verify the changes you made are not there

Test save changes

Add/change the focus keyphrase (or any data really)
Switch to a different post/page and save the changes
Switch back to the original post/page
Verify the changes you made are there
Refresh the page
Verify your changes are still there (i.e. actually got saved)

Regression

Good to know: whatever is the post/page you load the editor in, should be the one you can still edit.
Good to check: are all form fields saved properly?

before switching
after switching
during switching, e.g. the analysis will still run and try to update things, it should not be able to

Relevant test scenarios

Changes should be tested with the browser console open
Changes should be tested on different posts/pages/taxonomies/custom post types/custom taxonomies
Changes should be tested on different editors (Default Block/Gutenberg/Classic/Elementor/other)
Changes should be tested on different browsers
Changes should be tested on multisite

Test instructions for QA when the code is in the RC

QA should use the same steps as above.

QA can test this PR by following these steps:

Impact check

This PR affects the following parts of the plugin, which may require extra testing:

Our full Elementor editor integration.

UI changes

This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Other environments

This PR also affects Shopify. I have added a changelog entry starting with [shopify-seo], added test instructions for Shopify and attached the Shopify label to this PR.

Documentation

I have written documentation for this change. For example, comments in the Relevant technical choices, comments in the code, documentation on Confluence / shared Google Drive / Yoast developer portal, or other.

Quality assurance

I have tested this code to the best of my abilities.
During testing, I had activated all plugins that Yoast SEO provides integrations for.
I have added unit tests to verify the code works as intended.
If any part of the code is behind a feature flag, my test instructions also cover cases where the feature flag is switched off.
I have written this PR in accordance with my team's definition of done.
I have checked that the base branch is correctly set.

Innovation

No innovation project is applicable for this PR.
This PR falls under an innovation project. I have attached the innovation label.
I have added my hours to the WBSO document.

Fixes https://github.com/Yoast/plugins-automated-testing/issues/1694

In an effort to have a better overview in the elementor-editor-integration

Saves some searching around for Elementor specific JS code

For `get_document_config` actions. This allows for the registration of the Yoast tab and control to take place. Part of compatibility with the Elementor editor V2.

Preparation for separate integration for the control registration. Both should be able to check if our "metabox" is enabled for the current post, this class serves the purpose of retrieving the post.

In favor of moving it to the JS side to test if we should show it or not

Return the callback like Elementor does -- not currently using

This is how Elementor calls it internally

Only run when the document is our form document

* fixes the interval running in favor of a MutationObserver * includes start/stop functionality * includes snapshot functionality

Not so much detecting change, but rather tests is they are deemed relevant. I.e. no scores or calculated changes.

Refactor to Elementor hook to: * not stop listening when switching document * to not run when not on our form document / no post status (conditionals)

This is in favor of the PHP registration to be able to check the document ID is our form ID. Move the link in the more menu to here too, simpler as it needs the tab constant too.

… store

coveralls · 2024-08-05T07:55:17Z

Pull Request Test Coverage Report for Build 0acfa8341c088e9f2b05d14a322cea190883dca0

Details

93 of 314 (29.62%) changed or added relevant lines in 18 files are covered.
1029 unchanged lines in 3 files lost coverage.
Overall coverage decreased (-0.004%) to 54.068%

Changes Missing Coverage	Changed/Added Lines	%
packages/js/src/elementor/initialize.js	1	0.0%
packages/js/src/elementor/helpers/is-form-id.js	3	0.0%
src/integrations/third-party/elementor.php	3	0.0%
packages/js/src/elementor/initializers/editor-store.js	5	0.0%
packages/js/src/elementor/initializers/post-status-watcher.js	7	0.0%
packages/js/src/elementor/helpers/update-save-as-draft-warning.js	8	0.0%
packages/js/src/elementor/initializers/editor-watcher.js	10	0.0%
packages/js/src/elementor/helpers/is-relevant-change.js	20	0.0%
packages/js/src/elementor/initializers/render-sidebar.js	20	0.0%
packages/js/src/elementor/helpers/hooks.js	22	0.0%

Files with Coverage Reduction	New Missed Lines	%
src/integrations/third-party/elementor.php	1	0.0%
packages/js/src/elementor/initializers/editor-store.js	2	0.0%
src/generated/container.php	1026	0.16%

Totals
Change from base Build 2238911e0b32747a02c544fef991c288ec97f4bc:	-0.004%
Covered Lines:	29482
Relevant Lines:	54825

💛 - Coveralls

Use the code from the previous commits, refactoring amongst others, the form watcher and the Yoast tab. Add events to freeze our form/state and take a snapshot when switching to another document. Add support to restore snapshots when the user discards the Elementor edits.

Elementor sets the color depending on whether the tab is active or not. We're now using same color as whatever Elementor sets. Notes: * the text comment seems to no longer apply * the icon seems to always be a span now

* it would not render when the element was already on the page * added tests

* add actions to notify our other plugins * add redux utils to the external window object

thijsoo

CR 🚧 Some question mostly

packages/js/src/elementor/helpers/is-relevant-change.js

packages/js/src/elementor/initializers/editor-integration.js

packages/js/src/elementor/initializers/editor-store.js

packages/js/src/elementor/initializers/editor-watcher.js

packages/js/src/elementor/initializers/form-watcher.js

src/elementor/infrastructure/request-post.php

src/integrations/third-party/elementor.php

thijsoo

CR + ACC 👍

igorschoester added UI change PRs that result in a change in the UI changelog: bugfix Needs to be included in the 'Bugfixes' category in the changelog labels Jul 31, 2024

igorschoester force-pushed the 1694-231-elementor-323-beta6-yoast-tab-is-not-displayed-after-user-navigated-to-site-settings branch from cf6a3b2 to 2d63599 Compare August 5, 2024 07:29

igorschoester added 4 commits August 5, 2024 09:29

Refactor: move elementor render to own files

4ebf0bc

In an effort to have a better overview in the elementor-editor-integration

Refactor: move panel menu item to own file

cf900b9

In an effort to have a better overview in the elementor-editor-integration

Move Elementor files into the folder

437123d

Saves some searching around for Elementor specific JS code

Also run our Elementor integration on elementor_ajax requests

42f41ca

For `get_document_config` actions. This allows for the registration of the Yoast tab and control to take place. Part of compatibility with the Elementor editor V2.

igorschoester force-pushed the 1694-231-elementor-323-beta6-yoast-tab-is-not-displayed-after-user-navigated-to-site-settings branch from 2d63599 to 880f8cd Compare August 5, 2024 07:29

igorschoester added 15 commits August 5, 2024 09:45

Refactor retrieval of the post object in Elementor

cd50247

Preparation for separate integration for the control registration. Both should be able to check if our "metabox" is enabled for the current post, this class serves the purpose of retrieving the post.

Remove control registration

4226024

In favor of moving it to the JS side to test if we should show it or not

No longer run elementor edit integration on Elementor edit requests

ff0d326

Fix Elementor hooks' callback arguments

5150729

Add getConditions support to our Elementor hooks

9b83489

Return the callback like Elementor does -- not currently using

Hooks: rename internal hook to command

30b9b2b

This is how Elementor calls it internally

Add helper to check if the current document equals the form document

123b814

Add conditions to the editor watcher

f4ce2a1

Only run when the document is our form document

Add reducer enhancer to freeze the state

8b5d9ed

Add reducer enhancer to take/restore a state snapshot

30a04ac

Abstract a form watcher

fb3902e

* fixes the interval running in favor of a MutationObserver * includes start/stop functionality * includes snapshot functionality

Refactor detect change

5cb523e

Not so much detecting change, but rather tests is they are deemed relevant. I.e. no scores or calculated changes.

Add post status watcher

51c1425

Refactor to Elementor hook to: * not stop listening when switching document * to not run when not on our form document / no post status (conditionals)

Refactor the Yoast tab registration

3dfc116

This is in favor of the PHP registration to be able to check the document ID is our form ID. Move the link in the more menu to here too, simpler as it needs the tab constant too.

Add the freeze and snapshot reducer enhancers to our Elementor editor…

4225091

… store

igorschoester force-pushed the 1694-231-elementor-323-beta6-yoast-tab-is-not-displayed-after-user-navigated-to-site-settings branch from 880f8cd to d06f227 Compare August 5, 2024 07:46

igorschoester added 5 commits August 5, 2024 12:29

Adapt style selector to new ID

7a6a8e9

Remove the margin/padding of our sidebar/tab

b3bc5ec

Fix Yoast icon color not having an active state

7cdc873

Elementor sets the color depending on whether the tab is active or not. We're now using same color as whatever Elementor sets. Notes: * the text comment seems to no longer apply * the icon seems to always be a span now

Fix RenderInPortalIfElementExists

20d2e01

* it would not render when the element was already on the page * added tests

Expose freeze and snapshot for our other plugins

60622fa

* add actions to notify our other plugins * add redux utils to the external window object

igorschoester force-pushed the 1694-231-elementor-323-beta6-yoast-tab-is-not-displayed-after-user-navigated-to-site-settings branch from 032813c to 60622fa Compare August 5, 2024 10:29

igorschoester marked this pull request as ready for review August 5, 2024 11:22

thijsoo requested changes Aug 6, 2024

View reviewed changes

Process CR

446523b

thijsoo approved these changes Aug 6, 2024

View reviewed changes

thijsoo added this to the 23.3 milestone Aug 6, 2024

thijsoo merged commit d2b8284 into trunk Aug 6, 2024
35 checks passed

thijsoo deleted the 1694-231-elementor-323-beta6-yoast-tab-is-not-displayed-after-user-navigated-to-site-settings branch August 6, 2024 09:53

igorschoester mentioned this pull request Aug 7, 2024

Elementor: fix timing issue after save and before closing the document #21522

Merged

22 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Elementor: partially support document switching #21514

Elementor: partially support document switching #21514

igorschoester commented Jul 31, 2024 •

edited by thijsoo

Loading

coveralls commented Aug 5, 2024 •

edited

Loading

thijsoo left a comment

thijsoo left a comment

Elementor: partially support document switching #21514

Elementor: partially support document switching #21514

Conversation

igorschoester commented Jul 31, 2024 • edited by thijsoo Loading

Context

Summary

Relevant technical choices:

Test instructions

Test instructions for the acceptance test before the PR gets merged

Test original bug: Site Settings

Switching post/page

Test the discard

Test save changes

Regression

Relevant test scenarios

Test instructions for QA when the code is in the RC

Impact check

UI changes

Other environments

Documentation

Quality assurance

Innovation

coveralls commented Aug 5, 2024 • edited Loading

Pull Request Test Coverage Report for Build 0acfa8341c088e9f2b05d14a322cea190883dca0

Details

💛 - Coveralls

thijsoo left a comment

Choose a reason for hiding this comment

thijsoo left a comment

Choose a reason for hiding this comment

igorschoester commented Jul 31, 2024 •

edited by thijsoo

Loading

coveralls commented Aug 5, 2024 •

edited

Loading