Skip to content

Commit f5b2d02

Browse files
itsmeichigoitsmeichigo
authored
Merge pull request #5223 from woocommerce/issue/5211-move-item-popover
Shipping Labels: Move action sheet to individual package items to fix issue displaying popovers on iPad
2 parents 8e7158b + 89ed435 commit f5b2d02

File tree

7 files changed

+203
-149
lines changed

7 files changed

+203
-149
lines changed

‎WooCommerce/Classes/Model/ShippingLabelPackageItem.swift‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ import Yosemite
44
/// A struct that keeps information about items contained in a package in Shipping Label purchase flow.
55
///
66
struct ShippingLabelPackageItem: Equatable {
7+
/// Unique ID of the package item
8+
let id = UUID().uuidString
9+
710
/// ID of the product or variation
811
let productOrVariationID: Int64
912

‎WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/Create Shipping Label Form/Package Details/Multi-package/ShippingLabelPackagesForm.swift‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import SwiftUI
33
struct ShippingLabelPackagesForm: View {
44
@ObservedObject private var viewModel: ShippingLabelPackagesFormViewModel
55
@Environment(\.presentationMode) var presentation
6-
@State private var showingMoveItemActionSheet: Bool = false
76

87
init(viewModel: ShippingLabelPackagesFormViewModel) {
98
self.viewModel = viewModel
@@ -13,22 +12,16 @@ struct ShippingLabelPackagesForm: View {
1312
var body: some View {
1413
GeometryReader { geometry in
1514
ScrollView {
16-
ForEach(Array(viewModel.itemViewModels.enumerated()), id: \.offset) { index, element in
17-
ShippingLabelSinglePackage(packageNumber: index + 1,
18-
isCollapsible: viewModel.foundMultiplePackages,
19-
safeAreaInsets: geometry.safeAreaInsets,
20-
shouldShowMoveItemActionSheet: $showingMoveItemActionSheet,
21-
viewModel: element)
15+
ForEach(viewModel.itemViewModels) { element in
16+
ShippingLabelSinglePackage(isCollapsible: viewModel.foundMultiplePackages,
17+
safeAreaInsets: geometry.safeAreaInsets,
18+
viewModel: element)
2219
}
2320
.padding(.bottom, insets: geometry.safeAreaInsets)
2421
}
2522
.background(Color(.listBackground))
2623
.ignoresSafeArea(.container, edges: [.horizontal, .bottom])
2724
}
28-
.actionSheet(isPresented: $showingMoveItemActionSheet, content: {
29-
ActionSheet(title: Text(viewModel.moveItemActionSheetMessage),
30-
buttons: viewModel.moveItemActionSheetButtons)
31-
})
3225
.navigationTitle(Localization.title)
3326
.toolbar {
3427
ToolbarItem(placement: .confirmationAction) {

‎WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/Create Shipping Label Form/Package Details/Multi-package/ShippingLabelPackagesFormViewModel.swift‎

Lines changed: 80 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,6 @@ final class ShippingLabelPackagesFormViewModel: ObservableObject {
1313
selectedPackages.count > 1
1414
}
1515

16-
/// Message displayed on the Move Item action sheet.
17-
///
18-
@Published private(set) var moveItemActionSheetMessage: String = ""
19-
20-
/// Option buttons displayed on the Move Item action sheet.
21-
///
22-
@Published private(set) var moveItemActionSheetButtons: [ActionSheet.Button] = []
23-
2416
/// References of view models for child items.
2517
///
2618
@Published private(set) var itemViewModels: [ShippingLabelSinglePackageViewModel] = []
@@ -42,9 +34,9 @@ final class ShippingLabelPackagesFormViewModel: ObservableObject {
4234

4335
private var cancellables: Set<AnyCancellable> = []
4436

45-
/// Validation states of all items by index of each package.
37+
/// Validation states of all items by ID of each package.
4638
///
47-
private var packagesValidation: [Int: Bool] = [:] {
39+
private var packagesValidation: [String: Bool] = [:] {
4840
didSet {
4941
configureDoneButton()
5042
}
@@ -60,7 +52,11 @@ final class ShippingLabelPackagesFormViewModel: ObservableObject {
6052

6153
/// List of selected package with basic info.
6254
///
63-
@Published private var selectedPackages: [ShippingLabelPackageAttributes] = []
55+
private var selectedPackages: [ShippingLabelPackageAttributes] = [] {
56+
didSet {
57+
configureItemViewModels(order: order)
58+
}
59+
}
6460

6561
/// Products contained inside the Order and fetched from Core Data
6662
///
@@ -120,88 +116,92 @@ private extension ShippingLabelPackagesFormViewModel {
120116
/// Set up item view models on change selected packages.
121117
///
122118
func configureItemViewModels(order: Order) {
123-
$selectedPackages
124-
.map { [weak self] selectedPackages -> [ShippingLabelSinglePackageViewModel] in
125-
guard let self = self else {
126-
return []
127-
}
128-
return selectedPackages.enumerated().map { index, details in
129-
return .init(order: order,
130-
orderItems: details.items,
131-
packagesResponse: self.packagesResponse,
132-
selectedPackageID: details.packageID,
133-
totalWeight: details.totalWeight,
134-
isOriginalPackaging: details.isOriginalPackaging,
135-
onItemMoveRequest: { [weak self] productOrVariationID, packageName in
136-
self?.updateMoveItemActionSheet(for: productOrVariationID, from: details, packageIndex: index, packageName: packageName)
137-
},
138-
onPackageSwitch: { [weak self] newPackage in
139-
self?.switchPackage(currentPackage: details, newPackage: newPackage)
140-
},
141-
onPackagesSync: { [weak self] packagesResponse in
142-
self?.packagesResponse = packagesResponse
143-
self?.onPackageSyncCompletion(packagesResponse)
144-
})
119+
itemViewModels = selectedPackages.enumerated().map { index, details -> ShippingLabelSinglePackageViewModel in
120+
return .init(id: details.id,
121+
order: order,
122+
orderItems: details.items,
123+
packageNumber: index + 1,
124+
packagesResponse: self.packagesResponse,
125+
selectedPackageID: details.packageID,
126+
totalWeight: details.totalWeight,
127+
isOriginalPackaging: details.isOriginalPackaging,
128+
onItemMoveRequest: { [weak self] in
129+
self?.itemViewModels.forEach {
130+
$0.dismissPopover()
145131
}
132+
},
133+
onPackageSwitch: { [weak self] newPackage in
134+
self?.switchPackage(currentPackage: details, newPackage: newPackage)
135+
},
136+
onPackagesSync: { [weak self] packagesResponse in
137+
self?.packagesResponse = packagesResponse
138+
self?.onPackageSyncCompletion(packagesResponse)
139+
})
140+
}
141+
142+
// We need the updated `itemViewModels` to get package names for selection,
143+
// so we have to update buttons after creating the view models.
144+
itemViewModels.enumerated().forEach { index, model in
145+
guard let details = selectedPackages.first(where: { $0.id == model.id }) else {
146+
return
146147
}
147-
.sink { [weak self] viewModels in
148-
self?.itemViewModels = viewModels
149-
self?.observeItemViewModels()
150-
}
151-
.store(in: &cancellables)
148+
let actionSheetButtons = moveItemActionButtons(for: details, packageIndex: index)
149+
model.updateActionSheetButtons(actionSheetButtons)
150+
}
151+
observeItemViewModels()
152152
}
153153

154154
/// Update title and buttons for the Move Item action sheet.
155155
///
156-
func updateMoveItemActionSheet(for productOrVariationID: Int64,
157-
from currentPackage: ShippingLabelPackageAttributes,
158-
packageIndex: Int,
159-
packageName: String) {
160-
moveItemActionSheetMessage = String(format: Localization.moveItemActionSheetMessage, packageIndex + 1, packageName)
161-
moveItemActionSheetButtons = {
162-
var buttons: [ActionSheet.Button] = []
163-
164-
// Add options to move to other packages.
165-
for (index, package) in selectedPackages.enumerated() {
166-
guard !package.isOriginalPackaging else {
167-
continue
168-
}
169-
if index != packageIndex {
170-
guard let name = itemViewModels[safe: index]?.packageName else {
156+
func moveItemActionButtons(for currentPackage: ShippingLabelPackageAttributes,
157+
packageIndex: Int) -> [String: [ActionSheet.Button]] {
158+
var actionButtons: [String: [ActionSheet.Button]] = [:]
159+
currentPackage.items
160+
.forEach { item in
161+
var buttons: [ActionSheet.Button] = []
162+
163+
// Add options to move to other packages.
164+
for (index, package) in selectedPackages.enumerated() {
165+
guard !package.isOriginalPackaging else {
171166
continue
172167
}
173-
let packageTitle = String(format: Localization.packageName, index + 1, name)
174-
buttons.append(.default(Text(packageTitle), action: { [weak self] in
175-
ServiceLocator.analytics.track(.shippingLabelItemMoved, withProperties: ["destination": "existing_package"])
176-
self?.moveItem(productOrVariationID: productOrVariationID, currentPackageIndex: packageIndex, newPackageIndex: index)
177-
}))
168+
if index != packageIndex {
169+
guard let name = itemViewModels.first(where: { $0.id == package.id })?.packageName else {
170+
continue
171+
}
172+
let packageTitle = String(format: Localization.packageName, index + 1, name)
173+
buttons.append(.default(Text(packageTitle), action: { [weak self] in
174+
ServiceLocator.analytics.track(.shippingLabelItemMoved, withProperties: ["destination": "existing_package"])
175+
self?.moveItem(productOrVariationID: item.productOrVariationID, currentPackageIndex: packageIndex, newPackageIndex: index)
176+
}))
177+
}
178178
}
179-
}
180179

181-
if !currentPackage.isOriginalPackaging {
182-
let hasMultipleItems = currentPackage.items.count > 1 || currentPackage.items.first(where: { $0.quantity > 1 }) != nil
183-
if hasMultipleItems {
184-
// Add option to add item to new package if current package has more than one item.
180+
if !currentPackage.isOriginalPackaging {
181+
let hasMultipleItems = currentPackage.items.count > 1 || currentPackage.items.first(where: { $0.quantity > 1 }) != nil
182+
if hasMultipleItems {
183+
// Add option to add item to new package if current package has more than one item.
184+
buttons.append(.default(Text(Localization.addToNewPackage)) { [weak self] in
185+
ServiceLocator.analytics.track(.shippingLabelItemMoved, withProperties: ["destination": "new_package"])
186+
self?.addItemToNewPackage(productOrVariationID: item.productOrVariationID, packageIndex: packageIndex)
187+
})
188+
}
189+
190+
// Add option to ship in original package
191+
buttons.append(.default(Text(Localization.shipInOriginalPackage)) { [weak self] in
192+
ServiceLocator.analytics.track(.shippingLabelItemMoved, withProperties: ["destination": "original_packaging"])
193+
self?.shipInOriginalPackage(productOrVariationID: item.productOrVariationID, packageIndex: packageIndex)
194+
})
195+
} else {
185196
buttons.append(.default(Text(Localization.addToNewPackage)) { [weak self] in
186197
ServiceLocator.analytics.track(.shippingLabelItemMoved, withProperties: ["destination": "new_package"])
187-
self?.addItemToNewPackage(productOrVariationID: productOrVariationID, packageIndex: packageIndex)
198+
self?.addItemToNewPackage(productOrVariationID: item.productOrVariationID, packageIndex: packageIndex)
188199
})
189200
}
190-
191-
// Add option to ship in original package
192-
buttons.append(.default(Text(Localization.shipInOriginalPackage)) { [weak self] in
193-
ServiceLocator.analytics.track(.shippingLabelItemMoved, withProperties: ["destination": "original_packaging"])
194-
self?.shipInOriginalPackage(productOrVariationID: productOrVariationID, packageIndex: packageIndex)
195-
})
196-
} else {
197-
buttons.append(.default(Text(Localization.addToNewPackage)) { [weak self] in
198-
ServiceLocator.analytics.track(.shippingLabelItemMoved, withProperties: ["destination": "new_package"])
199-
self?.addItemToNewPackage(productOrVariationID: productOrVariationID, packageIndex: packageIndex)
200-
})
201+
buttons.append(.cancel())
202+
actionButtons[item.id] = buttons
201203
}
202-
buttons.append(.cancel())
203-
return buttons
204-
}()
204+
return actionButtons
205205
}
206206

207207
/// Move the item with `productOrVariationID` from `currentPackage` to a new package,
@@ -361,10 +361,10 @@ private extension ShippingLabelPackagesFormViewModel {
361361
///
362362
func observeItemViewModels() {
363363
packagesValidation.removeAll()
364-
itemViewModels.enumerated().forEach { (index, item) in
364+
itemViewModels.forEach { item in
365365
item.$isValidPackage
366366
.sink { [weak self] isValid in
367-
self?.packagesValidation[index] = isValid && item.selectedPackageID.isNotEmpty
367+
self?.packagesValidation[item.id] = isValid && item.selectedPackageID.isNotEmpty
368368
}
369369
.store(in: &cancellables)
370370
}
@@ -425,10 +425,6 @@ private extension ShippingLabelPackagesFormViewModel {
425425

426426
private extension ShippingLabelPackagesFormViewModel {
427427
enum Localization {
428-
static let moveItemActionSheetMessage = NSLocalizedString("This item is currently in Package %1$d: %2$@. Where would you like to move it?",
429-
comment: "Message in action sheet when an order item is about to " +
430-
"be moved on Package Details screen of Shipping Label flow." +
431-
"The package name reads like: Package 1: Custom Envelope.")
432428
static let packageName = NSLocalizedString("Package %1$d: %2$@",
433429
comment: "Name of package to be listed in Move Item action sheet " +
434430
"on Package Details screen of Shipping Label flow.")

‎WooCommerce/Classes/ViewRelated/Orders/Order Details/Shipping Labels/Create Shipping Label Form/Package Details/Multi-package/ShippingLabelSinglePackage.swift‎

Lines changed: 7 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,50 +5,31 @@ struct ShippingLabelSinglePackage: View {
55
@ObservedObject private var viewModel: ShippingLabelSinglePackageViewModel
66
@State private var isShowingPackageSelection = false
77
@State private var isCollapsed: Bool = false
8-
@Binding private var shouldShowMoveItemActionSheet: Bool
98

109
private let isCollapsible: Bool
11-
private let packageNumber: Int
1210
private let safeAreaInsets: EdgeInsets
1311

14-
init(packageNumber: Int,
15-
isCollapsible: Bool,
12+
init(isCollapsible: Bool,
1613
safeAreaInsets: EdgeInsets,
17-
shouldShowMoveItemActionSheet: Binding<Bool>,
1814
viewModel: ShippingLabelSinglePackageViewModel) {
19-
self.packageNumber = packageNumber
2015
self.isCollapsible = isCollapsible
2116
self.safeAreaInsets = safeAreaInsets
2217
self.viewModel = viewModel
23-
self._shouldShowMoveItemActionSheet = shouldShowMoveItemActionSheet
2418
}
2519

2620
var body: some View {
2721
CollapsibleView(isCollapsible: isCollapsible, isCollapsed: $isCollapsed, safeAreaInsets: safeAreaInsets) {
28-
ShippingLabelPackageNumberRow(packageNumber: packageNumber, numberOfItems: viewModel.itemsRows.count, isValid: viewModel.isValidPackage)
22+
ShippingLabelPackageNumberRow(packageNumber: viewModel.packageNumber, numberOfItems: viewModel.itemsRows.count, isValid: viewModel.isValidPackage)
2923
} content: {
3024
ListHeaderView(text: Localization.itemsToFulfillHeader, alignment: .left)
3125
.padding(.horizontal, insets: safeAreaInsets)
3226

3327
Divider()
3428

3529
ForEach(viewModel.itemsRows) { productItemRow in
36-
HStack {
37-
productItemRow
38-
Spacer()
39-
Button(action: {
40-
viewModel.requestMovingItem(productItemRow.productOrVariationID)
41-
shouldShowMoveItemActionSheet = true
42-
ServiceLocator.analytics.track(.shippingLabelMoveItemTapped)
43-
}, label: {
44-
Text(Localization.moveButton)
45-
.font(.footnote)
46-
.foregroundColor(Color(UIColor(color: .accent)))
47-
})
48-
.padding(.trailing, Constants.horizontalPadding)
49-
}
50-
.padding(.horizontal, insets: safeAreaInsets)
51-
.background(Color(.listForeground))
30+
productItemRow
31+
.padding(.horizontal, insets: safeAreaInsets)
32+
.background(Color(.listForeground))
5233
Divider()
5334
.padding(.horizontal, insets: safeAreaInsets)
5435
.padding(.leading, Constants.horizontalPadding)
@@ -148,7 +129,6 @@ private extension ShippingLabelSinglePackage {
148129
static let footer = NSLocalizedString("Sum of products and package weight",
149130
comment: "Title of the footer in Shipping Label Package Detail screen")
150131
static let invalidWeight = NSLocalizedString("Invalid weight", comment: "Error message when total weight is invalid in Package Detail screen")
151-
static let moveButton = NSLocalizedString("Move", comment: "Button on each order item of the Package Details screen in Shipping Labels flow.")
152132
static let originalPackaging = NSLocalizedString("Original packaging",
153133
comment: "Row title for detail of package shipped in original " +
154134
"packaging on Package Details screen in Shipping Labels flow.")
@@ -179,13 +159,11 @@ struct ShippingLabelSinglePackage_Previews: PreviewProvider {
179159
selectedPackageID: "Box 1",
180160
totalWeight: "",
181161
isOriginalPackaging: false,
182-
onItemMoveRequest: { _, _ in },
162+
onItemMoveRequest: {},
183163
onPackageSwitch: { _ in },
184164
onPackagesSync: { _ in })
185-
ShippingLabelSinglePackage(packageNumber: 1,
186-
isCollapsible: true,
165+
ShippingLabelSinglePackage(isCollapsible: true,
187166
safeAreaInsets: .zero,
188-
shouldShowMoveItemActionSheet: .constant(false),
189167
viewModel: viewModel)
190168
}
191169
}

0 commit comments

Comments
 (0)