Unsaved changes should consider image uploads in progress

[adamsilverstein]

Description

When users are uploading media and navigate away, uploads and image display break.

Gutenberg displays an "Unsaved Changes" warning if users try to navigate away from the editor with unsaved changes (triggered by window.beforeunload). The check for unsaved changes misses images that are still being uploaded allowing users to navigate away while WordPress is still uploading or processing an image.

In progress uploads may never complete regeneration if PHP is able to generate all sub sizes in one go, because our retry mechanism will not fire. Regardless, Gutenberg will continue marking the image as "in progress" with a spinner when the upload doesn't complete.

Step-by-step reproduction instructions

1. Create a new post and publish it.
2. drag in one or more very large images that will take several seconds to be processed
3. before image processing completes, hit update to update the post
4. click view post to navigate away
5. the image may or may show, the srcset will be incomplete (missing sizes)
6. edit the post again, note the images are stuck in an uploading state.

Screen recording

https://share.getcloudapp.com/8LuDGjnD

Screenshots

Environment info

• WordPress trunk
• tested with and without Gutenberg plugin (built at trunk)

Please confirm that you have searched existing issues in the repo.

I did, but I missed this issue which is a duplicate and is now closed - #16030

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

[adamsilverstein]

👋🏼 @ellatrix - I got something working in #41120 but it is a bit hacky because I ran into a couple of problems. If you can take a look, I would appreciate any tips!

• I can't import wordpress/editor into the image component, i got a precommit error ("24:1 error '@wordpress/editor' should be listed in the project's dependencies. Run 'npm i -S @wordpress/editor' to add it import/no-extraneous-dependencies") which I ignored to upload something. can i access the store here, or should this logic live somewhere else? what am i doing wrong?
• where do you suggest i try adding the locking? i also tried in the upload middleware that didn't work as well when I tried.
• ideally each image upload would have its own lock so as you uploaded a series of images, post saving would only unlock after all images completed uploading. Right now in my PR they all share the same lock key, so one unlock overrides all locks. I couldn't find a good lock key to use, we don't have a unique id for the image yet? is there am index or uid from the block I could use?

[Mamaduka]

Hi, @adamsilverstein 👋

• The block-library shouldn't depend on the @wordpress/editor package since non-WP environments also load the blocks. You can still use string as a store name if it's an essential or WP-specific block.
• Maybe we could use the mediaUpload utility from the editor package.
• The useInstanceId might help with unique IDs.

[adamsilverstein]

Hey @Mamaduka - thanks for the tips!

The block-library shouldn't depend on the @wordpress/editor package since non-WP environments also load the blocks. You can still use string as a store name if it's an essential or WP-specific block.

Right, hence the errors. Good to know I can can use the "string" method, also sounds like i shouldn't do that!

Maybe we could use the mediaUpload utility from the editor package.

Good suggestion, I saw this being used but didn't dig into internals. I'll give it a try.

The useInstanceId might help with unique IDs.

Excellent, I'll need that to track multiple uploads.

[adamsilverstein]

@Mamaduka addressed feedback - updated in #41120

[mtias]

@adamsilverstein this seems to be a duplicate of #5936. Just mentioning so both can be closed when settled.

[mtias]

There's also #16030

[adamsilverstein]

@adamsilverstein this seems to be a duplicate of #5936. Just mentioning so both can be closed when settled.

Thanks for pointing that out @mtias - that one is about autosave during image uploading so slightly different. I'll double check that locking post saving also locks autosaving, if not I'll update to lock both.

[adamsilverstein]

There's also #16030

Despite the title I can't tell if this ticket will be resolved with my PR, it looks like the author wants some event trigger on upload completion which makes me consider another approach. Currently, I'm working to detect when an image upload starts and finishes and locking and locking post saving accordingly. One possible variation would be to track "isImageUploading" in state, then use that bit of state for the conditional to saves. Not sure which makes more sense structurally, just something to consider.

[adamsilverstein]

@Mamaduka #41120 is ready for another review