search

10up / convert-to-blocks

Convert to Blocks is a WordPress plugin that transforms classic editor content to blocks on the fly.
https://wordpress.org/plugins/convert-to-blocks/
GNU General Public License v2.0
155 stars 20 forks source link

fix/170: wait for DOM manipulations before saving post #171

Closed Sidsector9 closed 4 months ago

Sidsector9 commented 5 months ago

Description of the Change

The issue happens because when images are added via replaceBlocks(), it takes some time for the browser to first inject <figure /> and then append <img /> to the figure, and since this is an asynchronous process, the client.save() triggers before all the DOM manipulations inside the blocks are completed.

This PR implements MutationObserver to detect if there are any DOM modifications within the replaced block and waits until all DOM are done before saving the post.

Closes #170

How to test the Change

  1. Activate classic editor.
  2. Create 2 posts with gallery and add heading. (Just to test if the PR works as expected to transform other elements to blocks).
  3. Disable the classic editor.
  4. Run the following: wp convert-to-blocks start --post_type=post
  5. Open the URL displayed in the terminal.
  6. It will start converting blocks in both the posts.
  7. Wait until terminal shows the message: "Migration finished successfully."
  8. Open both the posts and reload the page to verify if the transformed blocks are visible as expected.

Changelog Entry

Fixed - Issue with saving post before the convert to blocks transform completed.

Credits

Props @mdesplenter @Sidsector9 @dsawardekar

Checklist:

dsawardekar commented 5 months ago

I tested this on a hybrid site migration that mixes blocks & CE, and found that it stalls silently instead of moving to the next post. I'll try to to take a look next week.

Sidsector9 commented 5 months ago

@dsawardekar I found the root cause of this issue. When images are added via replaceBlocks(), it takes some time for the browser to first inject <figure /> and then append <img /> to the figure, and since this is an asynchronous process, the client.save() triggers before all the HTML injecting is done.

I was able to fix this using Mutation Observer, saving after detecting changes in subtree but I've done this outside in my own plugin for trials. I'll try next week to implement the same here.

jeffpaul commented 5 months ago

@mdesplenter if you're able to test and see if this helps with your issue, that'd be helpful, thanks!

mdesplenter commented 5 months ago

@jeffpaul Just tested it. When there is nothing to transform it goes to the next post as expected, but when it does have to transform blocks it won't save and doesn't continue. Using breakpoint in the console it stopped on "const result = await transformer.execute();"

Sidsector9 commented 5 months ago

@mdesplenter thanks for testing it. I have tested this with 2 posts, both starting with the <h2 /> tags followed by a gallery. Can you share what kind of post content you have? It'll help us debug it further.

It's working for me, please watch the video below:

https://github.com/10up/convert-to-blocks/assets/17757960/2c834c6c-0e56-4100-bf11-8ad86f67a27d

mdesplenter commented 5 months ago

The script never reaches line 79 of ClassicBlockTransformer: this.didTransform = true; It awaits a lot of promises and returns to await Promise.all(promises); from the convertBlocks function.

This is some example code (will remove it later)

Inleiding

title

Cras id nibh aliquet, ullamcorper dolor at, rhoncus erat. Praesent in iaculis enim. Duis consectetur ultricies mi, malesuada convallis est. Mauris ultrices, odio vitae dignissim vulputate, massa odio feugiat mauris, sed pretium mauris nunc sit amet ex. Proin faucibus elit pulvinar eros euismod, at malesuada sem maximus. Curabitur non cursus risus, vitae pharetra ipsum. Sed eu pellentesque nibh. Nullam urna sem, feugiat quis felis nec, sodales tincidunt est. Duis fermentum arcu nec blandit eleifend. Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia curae;

[gallery columns="1" size="full" ids="35714"]

[gallery columns="4" size="full" ids="35717,35718,35719,35720"]

title

Cras id nibh aliquet, ullamcorper dolor at, rhoncus erat. Praesent in iaculis enim. Duis consectetur ultricies mi, malesuada convallis est. Mauris ultrices, odio vitae dignissim vulputate, massa odio feugiat mauris, sed pretium mauris nunc sit amet ex. Proin faucibus elit pulvinar eros euismod, at malesuada sem maximus. Curabitur non cursus risus, vitae pharetra ipsum. Sed eu pellentesque nibh. Nullam urna sem, feugiat quis felis nec, sodales tincidunt est. Duis fermentum arcu nec blandit eleifend. Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia curae;

Vivamus ac luctus dui. Nunc lacinia sollicitudin ante, sit amet tristique tortor consequat nec. Donec ut cursus libero. Etiam at massa condimentum, condimentum lacus nec, varius arcu. Suspendisse vitae nulla non odio ullamcorper molestie. Aliquam erat dolor, fermentum vel condimentum nec, porttitor ut mauris. Praesent vitae diam at mi accumsan ultrices a ut purus. Pellentesque tristique accumsan elit, non rutrum metus egestas sed.

[gallery columns="1" size="full" ids="35721"]

[gallery columns="2" size="full" ids="35735,35722"]

Title , another one

Cras id nibh aliquet, ullamcorper dolor at, rhoncus erat. Praesent in iaculis enim. Duis consectetur ultricies mi, malesuada convallis est. Mauris ultrices, odio vitae dignissim vulputate, massa odio feugiat mauris, sed pretium mauris nunc sit amet ex. Proin faucibus elit pulvinar eros euismod, at malesuada sem maximus. Curabitur non cursus risus, vitae pharetra ipsum. Sed eu pellentesque nibh. Nullam urna sem, feugiat quis felis nec, sodales tincidunt est. Duis fermentum arcu nec blandit eleifend. Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia curae;

Vivamus ac luctus dui. Nunc lacinia sollicitudin ante, sit amet tristique tortor consequat nec. Donec ut cursus libero. Etiam at massa condimentum, condimentum lacus nec, varius arcu. Suspendisse vitae nulla non odio ullamcorper molestie. Aliquam erat dolor, fermentum vel condimentum nec, porttitor ut mauris. Praesent vitae diam at mi accumsan ultrices a ut purus. Pellentesque tristique accumsan elit, non rutrum metus egestas sed.

 

[gallery size="full" ids="35736,35725,35726"]

[gallery size="full" ids="35741,35742,35744"]

 

[button type="primary" url="#"]Custom shortcode![/button]`

Sidsector9 commented 5 months ago

@mdesplenter thanks for sharing the details. This is working for me, let me know if I tested it incorrectly. Also just to confirm, did you run the build step?

I have tested it with 2 posts similar to the format you provided. Please watch the video below:

https://github.com/10up/convert-to-blocks/assets/17757960/100891de-09b6-4a0d-87ab-86766b1e58ef

dsawardekar commented 5 months ago

@Sidsector9 I'm getting similar stalling on a larger database. I think waiting for promises is going to be always be tricky because legacy content can be diverse. We do not have any control over the block type mix in the post_content. Similarly mutation may or may not happen too.

My suggestion here is to update this as,

  1. Refactor the BE to return a init_delay in the localized vars. (https://github.com/10up/convert-to-blocks/blob/develop/includes/ConvertToBlocks/MigrationAgent.php#L31)
  2. Filter this, with apply_filters( 'convert_to_blocks_init_delay', 500, $post_id ) , so that users can override it per their legacy content needs. Default to 500 ms, so that existing projects are unaffected.
  3. On the client-side, alter the init code to use the init_delay. (https://github.com/10up/convert-to-blocks/blob/develop/assets/js/editor/editor.js#L72)

Based on how testing goes, we can then update the documentation to suggest a recommended init_delay for different types of legacy content.

mdesplenter commented 5 months ago

Note Deactivating all my other active plugins does help to trigger the save action action again. The main plugin that is bugging/breaking during loading is the Yoast SEO plugin, it's adding UI screens while the converter already started. I don't think you need it when converting to blocks, so I would add to the FAQ that users should disable as much plugins as possible before converting.

But still have the first problem with the images in galleries not getting converted in time. The init delay sounds good for my use case, will try that when it's here to test.

Sidsector9 commented 5 months ago

I wanted to point out the init_delay here won't work.

It's not the lack of delay in beginning the transform process that is causing the problem. The issue is because the editor is saving posts with static blocks before the blocks have finished DOM updates.

If it is delay that we are going forward with, then we should delay client.save() as suggested by @mdesplenter in the issue.

I have opened an alternative PR #173

@jeffpaul @dsawardekar suggested we keep this PR open and instead open a new PR with an alternative approach as they will be revisiting this issue next week.

jeffpaul commented 4 months ago

@Sidsector9 with #173 merged in, we good to close this PR?