Skip to content
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

Background image upload: Fix issue showing uploaded images while saving is in progress #15107

Merged
merged 8 commits into from
Feb 14, 2025

Conversation

itsmeichigo
Copy link
Contributor

@itsmeichigo itsmeichigo commented Feb 11, 2025

Closes: #15093

Description

This PR fixes the issue when uploaded images are not displayed when background image saving is in progress.

The solution:

  • Enable reusing the action handler even when there are no pending uploads.
  • Persist the product saved in the background and use it to determine hasUnsavedChangesOnImages.
  • Remove product from active uploads when all images are done uploading. This is to make the logic simpler.

Steps to reproduce

  1. Create or open an existing product with no images.
  2. Select a few images to upload, save the changes and navigate back to the product list.
  3. Wait a bit for the upload to complete but before the saving is done (notice the loading indicator is removed but the cover photo is not updated yet).
  4. Open the product form and notice that the image list is filled with the uploaded images.
  5. Navigate back and confirm that you're not asked to save any changes (neither when background saving is done or not).
  6. Repeat the steps 1-4. Remove the new images and tap Save. Confirm that everything works correctly - the Save button is hidden after saving is done.

Testing information

Tested on simulator iPhone 16 Pro iOS 18.2 and confirmed that:

  • Reopening product before background image saving is done shows uploaded images correctly.
  • Background image saving handles the saving automatically.
  • Saving the product after background image saving works correctly.
  • No regression for loading animation on products with pending uploads.

Screenshots

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-02-11.at.17.11.41.mp4

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

  • The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
  • Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
  • Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on all devices (phone/tablet) and no regressions are added.

@itsmeichigo itsmeichigo added type: bug A confirmed bug. feature: add/edit products Related to adding or editing products. labels Feb 11, 2025
@itsmeichigo itsmeichigo added this to the 21.8 milestone Feb 11, 2025
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Feb 11, 2025

WooCommerce iOS📲 You can test the changes from this Pull Request in WooCommerce iOS by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS WooCommerce iOS
Build Numberpr15107-5b269e1
Version21.7
Bundle IDcom.automattic.alpha.woocommerce
Commit5b269e1
App Center BuildWooCommerce - Prototype Builds #12952
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@itsmeichigo itsmeichigo marked this pull request as ready for review February 11, 2025 11:18
Comment on lines +62 to +68
override func prepareForReuse() {
super.prepareForReuse()
syncingOverlayView?.removeFromSuperview()
selectedProductImageOverlayView?.removeFromSuperview()
syncingOverlayView = nil
selectedProductImageOverlayView = nil
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attempt an additional fix for the reuse issue mentioned in #15052.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to have fixed the issue, I'm not able to replicate it anymore! 👏

@pmusolino pmusolino self-assigned this Feb 13, 2025
Copy link
Member

@pmusolino pmusolino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything works as expected 👏

@itsmeichigo itsmeichigo merged commit 3fe7850 into trunk Feb 14, 2025
12 checks passed
@itsmeichigo itsmeichigo deleted the fix/15093-empty-image-list-product-form branch February 14, 2025 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: add/edit products Related to adding or editing products. type: bug A confirmed bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Background image upload: Product form is empty when opened before saving is done
3 participants