Wait for editor changes to be debounced before closing modal

[joshuatf]

Submission Review Guidelines:

• I have followed the WooCommerce Contributing Guidelines and the WordPress Coding Standards.
• I have checked to ensure there aren't other open Pull Requests for the same update/change.
• I have reviewed my code for security best practices.
• Following the above guidelines will result in quick merges and clear and detailed feedback when appropriate.

Changes proposed in this Pull Request:

Making changes within the modal editor and quickly closing the editor previously would result in some of the changes being missed.

This PR capture the changes immediately, debounces on the onchange handler, and only closes the modal once the debouncing is complete.

Closes #38020 .

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

1. Navigate to Tools -> WCA Test Helper -> Features and enable the new product blocks editing experience
2. Navigate to Products -> Add new
3. Click on the "Add description" button
4. Make changes within the editor and quickly close the modal
5. Note that all changes are reflected in the preview (or reopen the modal to confirm)
6. Open and close the editor without changes to make sure it closes without issue

[github-actions]

Hi , @woocommerce/mothra

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

[louwie17]

Thanks for the updates, I did leave some suggestions to do this differently, basically two things:

• Call debouncedOnChange for both onInput & onChange (this is actually necessary :/ see inline comment for more info)
• Make use of the flush() method that useDebounce returns.

[louwie17]

It actually looks like you need both onChange and onInput :/ they are both being called for different circumstances.
For example, onChange only gets called when new blocks are created, but onInput when something within the block is updated. You can pretty easily see this if you console log both functions.
Also see this example: https://github.com/WordPress/gutenberg/blob/trunk/packages/block-editor/README.md?plain=1#L34-L35
And the description of the outputs: https://github.com/WordPress/gutenberg/blob/trunk/packages/block-editor/src/components/provider/README.md?plain=1#L14-L32

[joshuatf]

Good call. I consoled the outputs and noticed this as well.

A callback invoked when the blocks have been modified in a non-persistent manner.

I had read the above and re-reading it I'm still convinced the documentation is semantically incorrect. I would expect the non-persistent callback (onInput) to always fire while the persistent callback (onChange) would fire when those changes were persisted.

This behavior is all-around a little odd and does not match the browser API as stated in the docs. I'll open an issue upstream to get this fixed.

[louwie17]

This is a rather hacky way of doing this, and at-least at one point I managed to trigger the onClose but it wasn't being triggered until I made another block change (probably related to the difference between onInput and onChange.
But there is a much easier way to handle this, the useDebounce hook has a flush method that returns the last block result. So you can just do this within the handleClose:

const blocks = debouncedOnChange.flush();
if ( blocks ) {
  onChange( blocks );
}
onClose();

This way you can remove both the isClosing and isDebouncing states :)

[joshuatf]

This is a rather hacky way of doing this, and at-least at one point I managed to trigger the onClose but it wasn't being triggered until I made another block change (probably related to the difference between onInput and onChange.

I suspect you managed to do this based on the buggy behavior of onChange and onInput discussed above.

useDebounce hook has a flush method that returns the last block result

TIL! Thanks for sharing this- this is a much easier way to implement this.

[louwie17]

Thanks for the updates, I did leave some suggestions to do this differently, basically two things:

• Call debouncedOnChange for both onInput & onChange (this is actually necessary :/ see inline comment for more info)
• Make use of the flush() method that useDebounce returns.

[github-actions]

Test Results Summary

Commit SHA: e6ceafb

Test 🧪	Passed ✅	Failed 🚨	Broken 🚧	Skipped ⏭️	Unknown ❔	Total 📊	Duration ⏱️
API Tests	267	0	0	2	0	269	0m 52s
E2E Tests	189	0	0	10	0	199	21m 35s

---

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

[louwie17]

Thanks for the updates, this tested well 🚀