-
Notifications
You must be signed in to change notification settings - Fork 20
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/170: wait for DOM manipulations before saving post #171
Conversation
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.
There is likely something
this.didTransform = true; | ||
} | ||
} catch (e) { | ||
// Do nothing |
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.
This feels like it will silently hide errors during block transformation. Do we need the try-catch here?
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. |
@dsawardekar I found the root cause of this issue. When images are added via 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. |
4eb6ca8
to
d6dc014
Compare
@mdesplenter if you're able to test and see if this helps with your issue, that'd be helpful, thanks! |
@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();" |
@mdesplenter thanks for testing it. I have tested this with 2 posts, both starting with the It's working for me, please watch the video below: Screen-2024-06-27-123901.AM.mp4 |
The script never reaches line 79 of ClassicBlockTransformer: this.didTransform = true; This is some example code (will remove it later) InleidingtitleCras 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"] titleCras 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 oneCras 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]` |
@mdesplenter thanks for sharing the details. This is working for me, let me know if I tested it incorrectly. I have tested it with 2 posts similar to the format you provided. Please watch the video below: Screen-2024-06-28-11807.AM.mp4 |
@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,
Based on how testing goes, we can then update the documentation to suggest a recommended |
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. |
I wanted to point out the 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. |
@Sidsector9 with #173 merged in, we good to close this PR? |
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
heading
. (Just to test if the PR works as expected to transform other elements to blocks).wp convert-to-blocks start --post_type=post
Changelog Entry
Credits
Props @mdesplenter @Sidsector9 @dsawardekar
Checklist: