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.

Sign up for GitHub

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

Jump to bottom

[Product Creation AI v2] View and Remove package image #13213

Merged
merged 10 commits into from
Jul 2, 2024
Merged

[Product Creation AI v2] View and Remove package image #13213

merged 10 commits into from
Jul 2, 2024

Conversation

selanthiraiyan
Copy link
Contributor

@selanthiraiyan selanthiraiyan commented Jul 1, 2024 •
edited
Loading

Closes: #13104

Description

Adds a way to view and remove the selected package photo for the product creation with AI flow.

Changes

  • Show notice upon removing package photo with undo option.
  • Add view to show selected package photo in full screen.
  • Unit tests.

Steps to reproduce

  1. Create a JN site with Jetpack AI plugin
  2. Turn on productCreationAIv2M1 feature flag
  3. Build and run the app
  4. Navigate to the products tab
  5. Tap + button on top right
  6. Tap "Create a product with AI"
  7. Tap "Read text from product photo" button below the text field
  8. Validate that selecting a photo from device/camera/media library works.
  9. The selected photo should be displayed on the button below the text field.
  10. Tap the ellipsis button -> Tap on "Remove Photo", and the selected image should be removed.
  11. Validate that a notice is displayed with the text "Photo removed"
  12. Tapping the "Undo" button should revert the change.
  13. Tap the ellipsis button again -> Tap on "View Photo" and validate that the selected image is displayed in a sheet.

Screenshots

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

@selanthiraiyan selanthiraiyan added the feature: add/edit products Related to adding or editing products. label Jul 1, 2024
@selanthiraiyan selanthiraiyan added this to the 19.4 milestone Jul 1, 2024
@selanthiraiyan selanthiraiyan marked this pull request as ready for review July 1, 2024 15:06
@selanthiraiyan selanthiraiyan mentioned this pull request Jul 1, 2024
Closed
3 tasks
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jul 1, 2024 •
edited
Loading

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 Numberpr13213-cd28966
Version19.3
Bundle IDcom.automattic.alpha.woocommerce
Commitcd28966
App Center BuildWooCommerce - Prototype Builds #9835
 Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@itsmeichigo itsmeichigo self-assigned this Jul 2, 2024
Base automatically changed from feat/13104-select-image to trunk July 2, 2024 03:36
itsmeichigo
itsmeichigo approved these changes Jul 2, 2024
View reviewed changes
Copy link
Contributor

@itsmeichigo itsmeichigo left a comment

Choose a reason for hiding this comment

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

This works as described 👍

...ducts/Add Product/AddProductWithAI/StartingInfo/ProductCreationAIStartingInfoViewModel.swift Outdated
Comment on lines 14 to 15
@Published var isShowingMediaPickerSourceSheet: Bool = false
@Published var isShowingViewPhotoSheet: Bool = false
Copy link
Contributor

Choose a reason for hiding this comment

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

Super nit:

Suggested change
@Published var isShowingMediaPickerSourceSheet: Bool = false
@Published var isShowingViewPhotoSheet: Bool = false
@Published var isShowingMediaPickerSourceSheet = false
@Published var isShowingViewPhotoSheet = false

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update in 9e98a5b

...ducts/Add Product/AddProductWithAI/StartingInfo/ProductCreationAIStartingInfoViewModel.swift
@@ -50,20 +54,45 @@ final class ProductCreationAIStartingInfoViewModel: ObservableObject {
}

func didTapRemovePhoto() {
let previousState = imageState
Copy link
Contributor

Choose a reason for hiding this comment

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

❓ Should we keep a reference of the previous state in the class's scope? I wonder if it can get deallocated when this method is finished, causing an undefined behavior when tapping the Undo button.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Started capturing the previousState from inside the closure in 26a747a

...ated/Products/Add Product/AddProductWithAI/ProductCreationAIStartingInfoViewModelTests.swift Outdated
let sut = ProductCreationAIStartingInfoViewModel(siteID: siteID)

// When
sut.didTapReadTextFromPhoto()
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mean didTapReplacePhoto?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Fixed in d2f11ad

@selanthiraiyan
Copy link
Contributor Author

Thanks for the review, Huong! I am enabling auto-merge now. Please feel free to add comments, if any. I will address them in upcoming PRs.

@selanthiraiyan selanthiraiyan enabled auto-merge July 2, 2024 04:22
@selanthiraiyan selanthiraiyan merged commit 9bf0dc0 into trunk Jul 2, 2024
21 of 22 checks passed
@selanthiraiyan selanthiraiyan deleted the feat/13104-remove-image branch July 2, 2024 04:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@itsmeichigo itsmeichigo itsmeichigo approved these changes

@pmusolino pmusolino Awaiting requested review from pmusolino

Assignees

@itsmeichigo itsmeichigo

Labels
feature: add/edit products Related to adding or editing products.
Projects
None yet
Milestone
19.4
Development

Successfully merging this pull request may close these issues.

Image selection
3 participants
@selanthiraiyan @wpmobilebot @itsmeichigo