WordPress-iOS
WordPress-iOS copied to clipboard
[Gutenberg] - Cancel failed or in progress uploads if the media block is removed
Related PRs:
- https://github.com/WordPress/gutenberg/pull/61251
To test follow the instructions in the Gutenberg PR's description.
Regression Notes
-
Potential unintended areas of impact
-
What I did to test those areas of impact (or what existing automated tests I relied on)
-
What automated tests I added (or what prevented me from doing so)
PR submission checklist:
- [ ] I have completed the Regression Notes.
- [ ] I have considered adding unit tests for my changes.
- [ ] I have considered adding accessibility improvements for my changes.
- [ ] I have considered if this change warrants user-facing release notes and have added them to
RELEASE-NOTES.txt
if necessary.
Testing checklist:
- [ ] WordPress.com sites and self-hosted Jetpack sites.
- [ ] Portrait and landscape orientations.
- [ ] Light and dark modes.
- [ ] Fonts: Larger, smaller and bold text.
- [ ] High contrast.
- [ ] VoiceOver.
- [ ] Languages with large words or with letters/accents not frequently used in English.
- [ ] Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
- [ ] iPhone and iPad.
- [ ] Multi-tasking: Split view and Slide over. (iPad)
📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App Name | ![]() |
|
Configuration | Release-Alpha | |
Build Number | pr23125-8fb6152 | |
Version | 24.9 | |
Bundle ID | org.wordpress.alpha | |
Commit | 8fb615260fcba3f8dfa7596cada5f0fbc1765f19 | |
App Center Build | WPiOS - One-Offs #9920 |
📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App Name | ![]() |
|
Configuration | Release-Alpha | |
Build Number | pr23125-8fb6152 | |
Version | 24.9 | |
Bundle ID | com.jetpack.alpha | |
Commit | 8fb615260fcba3f8dfa7596cada5f0fbc1765f19 | |
App Center Build | jetpack-installable-builds #8969 |
Block Removal
I verified the "block removed → upload deleted" scenario. The gutenbergDidRequestMediaUploadCancelation
method does get called and the upload gets deleted 👍
If you Undo, it does end up in a somewhat weird state. if it's feasible, it would be great to "reset" the view and ask you to pick a photo again otherwise it seems a bit confusing.
Update: I also tested Video and Gallery blocks.
Upload Progress
I tested the upload progress and thumbnail display with a happy path scenarios and it looks great. But I would suggest to also remove the following lines as they are now redundant:
case .thumbnailReady(let url) where ReachabilityUtils.isInternetReachable() && media.remoteStatus == .failed:
gutenberg.mediaUploadUpdate(id: mediaUploadID, state: .failed, progress: 0, url: url, serverID: nil)
case .thumbnailReady(let url) where !ReachabilityUtils.isInternetReachable() && media.remoteStatus == .failed:
// The progress value passed is ignored by the editor, allowing the UI to retain the last known progress before pausing
gutenberg.mediaUploadUpdate(id: mediaUploadID, state: .paused, progress: 0, url: url, serverID: nil)
My understanding is that the only reason they are there is because the previous Gutenberg
method required you to pass the .state
on any thumbnail update.
I tested it be removing these lines and it fixed one more issue for me: the app not displaying the thumbnail if the failure happens before the thumbnail is generated. Here's before and after:
If you Undo, it does end up in a somewhat weird state. if it's feasible, it would be great to "reset" the view and ask you to pick a photo again otherwise it seems a bit confusing.
Definitely, restoring it to the placeholder would make sense. I'll create a ticket as a follow-up task.
My understanding is that the only reason they are there is because the previous Gutenberg method required you to pass the .state on any thumbnail update.
That's probably the reason yes.
I tested it be removing these lines and it fixed one more issue for me: the app not displaying the thumbnail if the failure happens before the thumbnail is generated. Here's before and after:
Oh wow, let's remove them then! I've also encountered that issue with the empty placeholder. I'll update the PR.
Thank you for the feedback and testing!
👋 @kean and @geriux, semi-related to this topic I added this issue: https://github.com/wordpress-mobile/WordPress-iOS/issues/23166