Gutenframe: Only try to select the PostLockedModal in appropriate conditions

[Copons]

Reported on Slack: p1697833272217969-slack-C04DZ8M0GHW

Proposed Changes

• Stop always querying the DOM for the PostLockedModal's button, and only do it when the modal is supposed to be displayed.

Testing Instructions

Note: using "Hide whitespace" would make the code easier to review, since most lines have simply been moved into a deeper nesting level.

• Enter the wpcom-block-editor folder.
• Start the dev env with yarn dev --sync.
• Sandbox widgets.wp.com.
• Sandbox a site with two separate test users.
• Log in to the site with both users (e.g. in two different browser profiles), and edit the same post.
  • Ensure you are editing the post in Calypso; the URL should be like https://wordpress.com/post/{ SITE_FRAGMENT }/{ POST_ID }.
• I'm unsure how to trigger the Post Lock modal, but at some point after typing on sides, it will show up automatically. 🤷

• Ensure the two document.querySelector( 'div.editor-post-locked-modal__buttons > a' ) are only executed when the pre-conditions are cleared, and not every time the whole state updates.
  • To test this, it's just easier to scatter console.log throughout the code.

Pre-merge Checklist

• Has the general commit checklist been followed? (PCYsg-hS-p2)
• https://wpcalypso.wordpress.com/devdocs/docs/testing/index.md for your changes?
• Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
• Have you checked for TypeScript, React or other console errors?
• Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
• Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
• For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-ajp-p2)?

[github-actions]

Calypso Live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-85324

Jetpack Cloud live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-85324&env=jetpack

[matticbot]

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

• wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug try/wpcom-block-editor-stop-always-selecting-dom on your sandbox.

[matticbot]

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.

[dsas]

I don't know if I'm confused or if something is broken, but these console.logs never fire for me. Here's my testing diff:

--- apps/wpcom-block-editor/src/calypso/features/iframe-bridge-server.js
+++ apps/wpcom-block-editor/src/calypso/features/iframe-bridge-server.js
@@ -151,6 +151,7 @@ function handlePostLocked( calypsoPort ) {
                        const lockedDialogButtons = document.querySelectorAll(
                                'div.editor-post-locked-modal__buttons > a'
                        );
+            console.log('1. querying for modal buttons');

                        if ( lockedDialogButtons.length === 3 ) {
                                //signal the parent frame to navigate to All Posts
@@ -196,6 +197,7 @@ function handlePostLockTakeover( calypsoPort ) {

                if ( isLocked && isLockTakeover ) {
                        const allPostsButton = document.querySelector( 'div.editor-post-locked-modal__buttons > a' );
+            console.log('2. querying for modal buttons');

                        if ( allPostsButton ) {
                                //handle All Posts button click event

[dsas]

I'm confused, my dialog just has one button:

document.querySelectorAll('div.editor-post-locked-modal__buttons > a');
NodeList [ a.components-button.is-primary ]
0: <a class="components-button is-primary" href="edit.php?post_type=post">
length: 1

[dsas]

Looking upstream, I think there an only ever be two buttons for the modal: https://github.com/WordPress/gutenberg/blob/187224a67b839ace8e9c42b85f351fda996a2361/packages/editor/src/components/post-locked-modal/index.js#L247

[dsas]

It used to be three buttons, but changed to two with WordPress/gutenberg#37852, this probably needs updating. I still can't trigger it so maybe something else is outdated....I'll open an issue

[dsas]

#83426

[dsas]

I don't know if I'm confused or if something is broken, but these console.logs never fire for me. Here's my testing diff:

Oh actually one of my users is redirecting from the calypso url to the wp-admin url...let me figure that out.

[Copons]

@dsas I haven't managed to test handlePostLocked too. I'm also confused by the check, but I think the change respects the original intent, whatever it was.

The other function works for me, with the correct redirect URL.

[dsas]

Even when testing according to instructions properly I can't seem to make the handlePostLocked unsub fn ever have the if be true - my console.log never fires...I can't get it to pass with the original code either, isLocked and isLockTakeover are both true or the former is false and the latter undefined.

The handlePostLockTakeover unsub fn manages to enter its if twice when isLocked and isLockTakeover are true.

None of the DOM look-ups are being spammed though and everything seems to work as it did before.