Skip to content

Commit 4a6a63d

Browse files
jaclyncjaclync
authored
Merge pull request #2126 from woocommerce/issue/1713-image-upload-ux
Image Upload: update navigation UX
2 parents 684362f + 964615c commit 4a6a63d

File tree

8 files changed

+144
-30
lines changed

8 files changed

+144
-30
lines changed

‎WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormViewController.swift‎

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ final class ProductFormViewController: UIViewController {
2121
private var product: Product {
2222
didSet {
2323
defer {
24-
updateNavigationBar(isUpdateEnabled: product != originalProduct)
24+
let isUpdateEnabled = hasUnsavedChanges(product: product)
25+
updateNavigationBar(isUpdateEnabled: isUpdateEnabled)
2526
}
2627

2728
if isNameTheOnlyChange(oldProduct: oldValue, newProduct: product) {
@@ -168,6 +169,40 @@ private extension ProductFormViewController {
168169

169170
navigationController?.present(inProgressViewController, animated: true, completion: nil)
170171

172+
updateProductRemotely()
173+
}
174+
175+
func updateProductRemotely() {
176+
waitUntilAllImagesAreUploaded { [weak self] in
177+
self?.dispatchUpdateProductAction()
178+
}
179+
}
180+
181+
func waitUntilAllImagesAreUploaded(onCompletion: @escaping () -> Void) {
182+
let group = DispatchGroup()
183+
184+
// Waits for all product images to be uploaded before updating the product remotely.
185+
group.enter()
186+
let observationToken = productImageActionHandler.addUpdateObserver(self) { [weak self] (productImageStatuses, error) in
187+
guard productImageStatuses.hasPendingUpload == false else {
188+
return
189+
}
190+
191+
guard let self = self else {
192+
return
193+
}
194+
195+
self.product = self.productUpdater.imagesUpdated(images: productImageStatuses.images)
196+
group.leave()
197+
}
198+
199+
group.notify(queue: .main) {
200+
observationToken.cancel()
201+
onCompletion()
202+
}
203+
}
204+
205+
func dispatchUpdateProductAction() {
171206
let action = ProductAction.updateProduct(product: product) { [weak self] (product, error) in
172207
guard let product = product, error == nil else {
173208
let errorDescription = error?.localizedDescription ?? "No error specified"
@@ -384,7 +419,7 @@ private extension ProductFormViewController {
384419
//
385420
extension ProductFormViewController {
386421
override func shouldPopOnBackButton() -> Bool {
387-
if product != originalProduct {
422+
if hasUnsavedChanges(product: product) {
388423
presentBackNavigationActionSheet()
389424
return false
390425
}
@@ -396,6 +431,10 @@ extension ProductFormViewController {
396431
self?.navigationController?.popViewController(animated: true)
397432
})
398433
}
434+
435+
private func hasUnsavedChanges(product: Product) -> Bool {
436+
return product != originalProduct || productImageActionHandler.productImageStatuses.hasPendingUpload
437+
}
399438
}
400439

401440

‎WooCommerce/Classes/ViewRelated/Products/Media/ProductImageActionHandler.swift‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,12 @@ final class ProductImageActionHandler {
136136
}
137137
allStatuses = (productImageStatuses: imageStatuses, error: nil)
138138
}
139+
140+
/// Resets the product images to the ones from the given Product.
141+
///
142+
func resetProductImages(to product: Product) {
143+
allStatuses = (productImageStatuses: product.imageStatuses, error: nil)
144+
}
139145
}
140146

141147
private extension ProductImageActionHandler {

‎WooCommerce/Classes/ViewRelated/Products/Media/ProductImageStatus.swift‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,19 @@ extension Collection where Element == ProductImageStatus {
2424
}
2525
}
2626
}
27+
28+
/// Whether there are still any images being uploaded.
29+
///
30+
var hasPendingUpload: Bool {
31+
return contains(where: {
32+
switch $0 {
33+
case .uploading:
34+
return true
35+
default:
36+
return false
37+
}
38+
})
39+
}
2740
}
2841

2942
extension ProductImageStatus {

‎WooCommerce/Classes/ViewRelated/Products/Media/ProductImagesViewController.swift‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ final class ProductImagesViewController: UIViewController {
1313

1414
private let siteID: Int64
1515
private let productID: Int64
16+
private let product: Product
1617

1718
private let productImageActionHandler: ProductImageActionHandler
1819
private let productUIImageLoader: ProductUIImageLoader
@@ -58,6 +59,7 @@ final class ProductImagesViewController: UIViewController {
5859
productImageActionHandler: ProductImageActionHandler,
5960
productUIImageLoader: ProductUIImageLoader,
6061
completion: @escaping Completion) {
62+
self.product = product
6163
self.siteID = product.siteID
6264
self.productID = product.productID
6365
self.productImageActionHandler = productImageActionHandler
@@ -171,17 +173,24 @@ extension ProductImagesViewController {
171173
presentDiscardChangesActionSheet()
172174
return false
173175
}
176+
resetProductImages()
174177
return true
175178
}
176179

177180
private func presentDiscardChangesActionSheet() {
178181
UIAlertController.presentDiscardChangesActionSheet(viewController: self, onDiscard: { [weak self] in
182+
self?.resetProductImages()
179183
self?.navigationController?.popViewController(animated: true)
180184
})
181185
}
182186

187+
private func resetProductImages() {
188+
productImageActionHandler.resetProductImages(to: product)
189+
}
190+
183191
private func hasOutstandingChanges() -> Bool {
184192
return originalProductImages != productImages
193+
|| productImageStatuses.hasPendingUpload
185194
}
186195
}
187196

‎WooCommerce/WooCommerce.xcodeproj/project.pbxproj‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@
5858
0216272B2379662C000208D2 /* DefaultProductFormTableViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0216272A2379662C000208D2 /* DefaultProductFormTableViewModel.swift */; };
5959
0218B4EC242E06F00083A847 /* MediaType+WPMediaType.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0218B4EB242E06F00083A847 /* MediaType+WPMediaType.swift */; };
6060
0219B03723964527007DCD5E /* PaginatedProductShippingClassListSelectorDataSource.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0219B03623964527007DCD5E /* PaginatedProductShippingClassListSelectorDataSource.swift */; };
61-
021AEF9C2407B07300029D28 /* ProductImageStatus+ImagesTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 021AEF9B2407B07300029D28 /* ProductImageStatus+ImagesTests.swift */; };
61+
021AEF9C2407B07300029D28 /* ProductImageStatus+HelpersTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 021AEF9B2407B07300029D28 /* ProductImageStatus+HelpersTests.swift */; };
6262
021AEF9E2407F55C00029D28 /* PHAssetImageLoader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 021AEF9D2407F55C00029D28 /* PHAssetImageLoader.swift */; };
6363
021E2A1723A9FE5A00B1DE07 /* ProductInventorySettingsViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 021E2A1523A9FE5A00B1DE07 /* ProductInventorySettingsViewController.swift */; };
6464
021E2A1823A9FE5A00B1DE07 /* ProductInventorySettingsViewController.xib in Resources */ = {isa = PBXBuildFile; fileRef = 021E2A1623A9FE5A00B1DE07 /* ProductInventorySettingsViewController.xib */; };
@@ -848,7 +848,7 @@
848848
0216272A2379662C000208D2 /* DefaultProductFormTableViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DefaultProductFormTableViewModel.swift; sourceTree = "<group>"; };
849849
0218B4EB242E06F00083A847 /* MediaType+WPMediaType.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "MediaType+WPMediaType.swift"; sourceTree = "<group>"; };
850850
0219B03623964527007DCD5E /* PaginatedProductShippingClassListSelectorDataSource.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PaginatedProductShippingClassListSelectorDataSource.swift; sourceTree = "<group>"; };
851-
021AEF9B2407B07300029D28 /* ProductImageStatus+ImagesTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ProductImageStatus+ImagesTests.swift"; sourceTree = "<group>"; };
851+
021AEF9B2407B07300029D28 /* ProductImageStatus+HelpersTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ProductImageStatus+HelpersTests.swift"; sourceTree = "<group>"; };
852852
021AEF9D2407F55C00029D28 /* PHAssetImageLoader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PHAssetImageLoader.swift; sourceTree = "<group>"; };
853853
021E2A1523A9FE5A00B1DE07 /* ProductInventorySettingsViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductInventorySettingsViewController.swift; sourceTree = "<group>"; };
854854
021E2A1623A9FE5A00B1DE07 /* ProductInventorySettingsViewController.xib */ = {isa = PBXFileReference; lastKnownFileType = file.xib; path = ProductInventorySettingsViewController.xib; sourceTree = "<group>"; };
@@ -1855,7 +1855,7 @@
18551855
children = (
18561856
027B8BBC23FE0DE10040944E /* ProductImageActionHandlerTests.swift */,
18571857
02A275BD23FE57DC005C560F /* ProductUIImageLoaderTests.swift */,
1858-
021AEF9B2407B07300029D28 /* ProductImageStatus+ImagesTests.swift */,
1858+
021AEF9B2407B07300029D28 /* ProductImageStatus+HelpersTests.swift */,
18591859
);
18601860
path = Media;
18611861
sourceTree = "<group>";
@@ -4618,7 +4618,7 @@
46184618
45B9C64523A945C0007FC4C5 /* PriceInputFormatterTests.swift in Sources */,
46194619
453904F523BB8BD5007C4956 /* ProductTaxClassListSelectorDataSourceTests.swift in Sources */,
46204620
746FC23D2200A62B00C3096C /* DateWooTests.swift in Sources */,
4621-
021AEF9C2407B07300029D28 /* ProductImageStatus+ImagesTests.swift in Sources */,
4621+
021AEF9C2407B07300029D28 /* ProductImageStatus+HelpersTests.swift in Sources */,
46224622
024A543622BA84DB00F4F38E /* DeveloperEmailCheckerTests.swift in Sources */,
46234623
B541B2132189E7FD008FE7C1 /* ScannerWooTests.swift in Sources */,
46244624
B57C5C9A21B80E7100FF82B2 /* DataWooTests.swift in Sources */,

‎WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageActionHandlerTests.swift‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,27 @@ final class ProductImageActionHandlerTests: XCTestCase {
182182
let expectedImageStatuses = expectedImageStatusesFromSiteMediaLibrary + mockRemoteProductImageStatuses
183183
XCTAssertEqual(productImageActionHandler.productImageStatuses, expectedImageStatuses)
184184
}
185+
186+
// MARK: - `resetProductImages(to:)`
187+
188+
func testResettingProductImagesToAProduct() {
189+
// Arrange
190+
let mockProduct = MockProduct().product(images: [])
191+
let productImageActionHandler = ProductImageActionHandler(siteID: 123,
192+
product: mockProduct)
193+
194+
// Action
195+
let mockProductImages = [
196+
ProductImage(imageID: 1, dateCreated: Date(), dateModified: Date(), src: "", name: "", alt: ""),
197+
ProductImage(imageID: 2, dateCreated: Date(), dateModified: Date(), src: "", name: "", alt: "")
198+
]
199+
let anotherMockProduct = MockProduct().product(images: mockProductImages)
200+
productImageActionHandler.resetProductImages(to: anotherMockProduct)
201+
202+
// Assert
203+
let expectedProductImageStatuses = mockProductImages.map { ProductImageStatus.remote(image: $0) }
204+
XCTAssertEqual(productImageActionHandler.productImageStatuses, expectedProductImageStatuses)
205+
}
185206
}
186207

187208
private extension ProductImageActionHandlerTests {

‎WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageStatus+HelpersTests.swift‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
import Photos
2+
import XCTest
3+
4+
@testable import WooCommerce
5+
@testable import Yosemite
6+
7+
final class ProductImageStatus_HelpersTests: XCTestCase {
8+
// MARK: - `images`
9+
10+
func testImagesReturnsEmptyIfThereAreNoRemoteImages() {
11+
let statuses: [ProductImageStatus] = [
12+
.uploading(asset: PHAsset())
13+
]
14+
XCTAssertEqual(statuses.images, [])
15+
}
16+
17+
func testImagesReturnsProductImagesIfThereAreRemoteImages() {
18+
let productImage = ProductImage(imageID: 17, dateCreated: Date(), dateModified: Date(), src: "", name: nil, alt: nil)
19+
20+
let statuses: [ProductImageStatus] = [
21+
.uploading(asset: PHAsset()),
22+
.remote(image: productImage)
23+
]
24+
XCTAssertEqual(statuses.images, [productImage])
25+
}
26+
27+
// MARK: - `hasPendingUpload`
28+
29+
func testHasPendingUploadWithNoImages() {
30+
let statuses: [ProductImageStatus] = []
31+
XCTAssertFalse(statuses.hasPendingUpload)
32+
}
33+
34+
func testHasPendingUploadWithAllRemoteImages() {
35+
let productImage = ProductImage(imageID: 17, dateCreated: Date(), dateModified: Date(), src: "", name: nil, alt: nil)
36+
37+
let statuses: [ProductImageStatus] = [.remote(image: productImage), .remote(image: productImage)]
38+
XCTAssertFalse(statuses.hasPendingUpload)
39+
}
40+
41+
func testHasPendingUploadWithBothStatusTypes() {
42+
let productImage = ProductImage(imageID: 17, dateCreated: Date(), dateModified: Date(), src: "", name: nil, alt: nil)
43+
44+
let statuses: [ProductImageStatus] = [
45+
.uploading(asset: PHAsset()),
46+
.remote(image: productImage)
47+
]
48+
XCTAssertTrue(statuses.hasPendingUpload)
49+
}
50+
}

‎WooCommerce/WooCommerceTests/ViewRelated/Products/Media/ProductImageStatus+ImagesTests.swift‎

Lines changed: 0 additions & 24 deletions
This file was deleted.

0 commit comments

Comments
 (0)