From 14ff226ee95e8dd742be593a12e2a3a1e066ab8e Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Fri, 1 Nov 2024 13:55:49 +0000 Subject: [PATCH 1/4] Move item list responsibility to Views --- WooCommerce/Classes/POS/POSItemsService.swift | 22 +++++ .../POS/PointOfSaleAggregateModel.swift | 61 +----------- .../CardReaderConnectionStatusView.swift | 1 - .../Classes/POS/Presentation/CartView.swift | 2 - .../POS/Presentation/ItemListView.swift | 56 +++++------ .../PointOfSaleDashboardView.swift | 92 ++++++++++++++++--- .../PointOfSaleEntryPointView.swift | 11 +-- .../Classes/POS/Presentation/TotalsView.swift | 1 - .../POS/ViewModels/ItemListViewModel.swift | 53 ----------- .../ItemListViewModelProtocol.swift | 11 --- .../PointOfSaleDashboardViewModel.swift | 23 ----- .../WooCommerce.xcodeproj/project.pbxproj | 12 +-- 12 files changed, 141 insertions(+), 204 deletions(-) create mode 100644 WooCommerce/Classes/POS/POSItemsService.swift delete mode 100644 WooCommerce/Classes/POS/ViewModels/ItemListViewModel.swift delete mode 100644 WooCommerce/Classes/POS/ViewModels/ItemListViewModelProtocol.swift diff --git a/WooCommerce/Classes/POS/POSItemsService.swift b/WooCommerce/Classes/POS/POSItemsService.swift new file mode 100644 index 00000000000..3f87f294178 --- /dev/null +++ b/WooCommerce/Classes/POS/POSItemsService.swift @@ -0,0 +1,22 @@ +import Foundation +import protocol Yosemite.POSItemProvider +import protocol Yosemite.POSItem + +struct POSItemsService { + + private let itemProvider: POSItemProvider + + init(itemProvider: POSItemProvider) { + self.itemProvider = itemProvider + } + + @MainActor + func fetchItems(pageNumber: Int) async throws -> [any POSItem] { + var newItems = try await itemProvider.providePointOfSaleItems(pageNumber: pageNumber) + if pageNumber == 1 { + newItems.insert(POSDiscount(), at: 0) + } + + return newItems + } +} diff --git a/WooCommerce/Classes/POS/PointOfSaleAggregateModel.swift b/WooCommerce/Classes/POS/PointOfSaleAggregateModel.swift index 79e28d1c503..cbd5452049a 100644 --- a/WooCommerce/Classes/POS/PointOfSaleAggregateModel.swift +++ b/WooCommerce/Classes/POS/PointOfSaleAggregateModel.swift @@ -3,7 +3,6 @@ import Combine import protocol Yosemite.POSOrderServiceProtocol import protocol Yosemite.POSItem -import protocol Yosemite.POSItemProvider import struct Yosemite.POSProduct import struct Yosemite.Order import struct Yosemite.OrderItem @@ -19,28 +18,23 @@ final class PointOfSaleAggregateModel: ObservableObject { } @Published private(set) var orderStage: OrderStage = .building - private var allItems: [any POSDisplayableItem] = [] @Published private(set) var cart: [CartItem] = [] @Published private(set) var orderState: PointOfSaleOrderState = .idle @Published private(set) var order: Order? = nil @Published private(set) var connectionStatus: CardPresentPaymentReaderConnectionStatus = .disconnected @Published private(set) var paymentState: PointOfSalePaymentState = .acceptingCard - @Published private(set) var itemListState: PointOfSaleItemListState = .initialLoading private let orderService: POSOrderServiceProtocol let cardPresentPaymentService: CardPresentPaymentFacade - private let itemProvider: POSItemProvider private let analytics: Analytics private var cancellables: Set = [] private var startPaymentOnReaderConnection: AnyCancellable? private var cardReaderDisconnection: AnyCancellable? - init(itemProvider: POSItemProvider, - cardPresentPaymentService: CardPresentPaymentFacade, + init(cardPresentPaymentService: CardPresentPaymentFacade, orderService: POSOrderServiceProtocol, analytics: Analytics) { - self.itemProvider = itemProvider self.cardPresentPaymentService = cardPresentPaymentService self.orderService = orderService self.analytics = analytics @@ -58,57 +52,6 @@ final class PointOfSaleAggregateModel: ObservableObject { .store(in: &cancellables) } - @MainActor - func loadInitialItems() async { - do { - itemListState = .initialLoading - try await fetchItems(pageNumber: 1) - } catch { - itemListState = .error(PointOfSaleErrorState.errorOnLoadingProducts()) - } - } - - @MainActor - func loadItems(pageNumber: Int) async { - do { - itemListState = .loading(allItems) - try await fetchItems(pageNumber: pageNumber) - } catch { - itemListState = .error(PointOfSaleErrorState.errorOnLoadingProducts()) - } - } - - @MainActor - private func fetchItems(pageNumber: Int) async throws { - var newItems = try await itemProvider.providePointOfSaleItems(pageNumber: pageNumber) - if pageNumber == 1 { - newItems.insert(POSDiscount(), at: 0) - } - let uniqueNewItems = newItems - .filter { newItem in - !allItems.contains(where: { $0.id == newItem.itemID }) - } - .compactMap(createPOSDisplayableItem(for:)) - - allItems.append(contentsOf: uniqueNewItems) - - if allItems.count == 0 { - itemListState = .empty - } else { - itemListState = .loaded(allItems) - } - } - - @MainActor - func reloadItems() async { - removeAllItems() - await loadItems(pageNumber: 1) - } - - func removeAllItems() { - allItems.removeAll() - } - func startNewOrder() { clearOrder() removeAllItemsFromCart() @@ -137,7 +80,7 @@ final class PointOfSaleAggregateModel: ObservableObject { @MainActor func submitCart() async { orderStage = .finalizing - await startSyncingOrder(with: cart, allItems: allItems.map { $0.item }) + await startSyncingOrder(with: cart, allItems: cart.map { $0.item }) } private func startSyncingOrder(with cartItems: [CartItem], allItems: [POSItem]) async { diff --git a/WooCommerce/Classes/POS/Presentation/CardReaderConnection/CardReaderConnectionStatusView.swift b/WooCommerce/Classes/POS/Presentation/CardReaderConnection/CardReaderConnectionStatusView.swift index f6e51c8b405..303308569c7 100644 --- a/WooCommerce/Classes/POS/Presentation/CardReaderConnection/CardReaderConnectionStatusView.swift +++ b/WooCommerce/Classes/POS/Presentation/CardReaderConnection/CardReaderConnectionStatusView.swift @@ -162,7 +162,6 @@ private extension CardReaderConnectionStatusView { import class WooFoundation.MockAnalyticsPreview #Preview { let posModel = PointOfSaleAggregateModel( - itemProvider: POSItemProviderPreview(), cardPresentPaymentService: CardPresentPaymentPreviewService(), orderService: POSOrderPreviewService(), analytics: MockAnalyticsPreview()) diff --git a/WooCommerce/Classes/POS/Presentation/CartView.swift b/WooCommerce/Classes/POS/Presentation/CartView.swift index a47f7292bf5..6b5be7978f7 100644 --- a/WooCommerce/Classes/POS/Presentation/CartView.swift +++ b/WooCommerce/Classes/POS/Presentation/CartView.swift @@ -270,7 +270,6 @@ import class WooFoundation.MockAnalyticsProviderPreview // TODO: // Simplify this by mocking `CartViewModel` let posModel = PointOfSaleAggregateModel( - itemProvider: POSItemProviderPreview(), cardPresentPaymentService: CardPresentPaymentPreviewService(), orderService: POSOrderPreviewService(), analytics: MockAnalyticsPreview()) @@ -279,7 +278,6 @@ import class WooFoundation.MockAnalyticsProviderPreview currencyFormatter: .init(currencySettings: .init())) let cartViewModel = CartViewModel(analytics: MockAnalyticsPreview(), posModel: posModel) - let itemsListViewModel = ItemListViewModel(posModel: posModel) let dashboardViewModel = PointOfSaleDashboardViewModel( posModel: posModel, connectivityObserver: POSConnectivityObserverPreview()) diff --git a/WooCommerce/Classes/POS/Presentation/ItemListView.swift b/WooCommerce/Classes/POS/Presentation/ItemListView.swift index 7a747a24334..35d0b24e769 100644 --- a/WooCommerce/Classes/POS/Presentation/ItemListView.swift +++ b/WooCommerce/Classes/POS/Presentation/ItemListView.swift @@ -2,24 +2,24 @@ import SwiftUI import protocol Yosemite.POSItem struct ItemListView: View { - @ObservedObject var viewModel: ItemListViewModel - @ObservedObject var posModel: PointOfSaleAggregateModel @Environment(\.floatingControlAreaSize) var floatingControlAreaSize: CGSize @Environment(\.dynamicTypeSize) private var dynamicTypeSize - init(viewModel: ItemListViewModel, - posModel: PointOfSaleAggregateModel) { - self.viewModel = viewModel - self.posModel = posModel - } + @Binding var itemListState: PointOfSaleItemListState + + var loadNextItems: () async -> Void + var reloadItems: () async -> Void + + @State private var isHeaderBannerDismissed: Bool = UserDefaults.standard.bool(forKey: BannerState.isSimpleProductsOnlyBannerDismissedKey) + @State private var showSimpleProductsModal: Bool = false var body: some View { VStack { headerView - .posModal(isPresented: $viewModel.showSimpleProductsModal) { - SimpleProductsOnlyInformation(isPresented: $viewModel.showSimpleProductsModal) + .posModal(isPresented: $showSimpleProductsModal) { + SimpleProductsOnlyInformation(isPresented: $showSimpleProductsModal) } - switch posModel.itemListState { + switch itemListState { case .initialLoading, .empty, .error: // These cases are handled directly in the dashboard, we do not render // a specific view within the ItemListView to handle them @@ -29,7 +29,7 @@ struct ItemListView: View { } } .refreshable { - await posModel.reloadItems() + await reloadItems() } .background(Color.posPrimaryBackground) .accessibilityElement(children: .contain) @@ -44,10 +44,10 @@ private extension ItemListView { VStack { HStack { POSHeaderTitleView() - if !viewModel.shouldShowHeaderBanner { + if isHeaderBannerDismissed { Spacer() Button(action: { - viewModel.simpleProductsInfoButtonTapped() + showSimpleProductsModal = true }, label: { Image(systemName: "info.circle") .font(.posTitleRegular) @@ -56,7 +56,7 @@ private extension ItemListView { .padding(.trailing, Constants.infoIconPadding) } } - if !dynamicTypeSize.isAccessibilitySize, viewModel.shouldShowHeaderBanner { + if !dynamicTypeSize.isAccessibilitySize, !isHeaderBannerDismissed { bannerCardView .padding(.horizontal, Constants.bannerCardPadding) } @@ -92,7 +92,8 @@ private extension ItemListView { .padding(.vertical, Constants.bannerVerticalPadding) VStack { Button(action: { - viewModel.dismissBanner() + isHeaderBannerDismissed = true + UserDefaults.standard.set(isHeaderBannerDismissed, forKey: BannerState.isSimpleProductsOnlyBannerDismissedKey) }, label: { Image(systemName: "xmark") .font(.posBodyRegular) @@ -110,7 +111,7 @@ private extension ItemListView { .shadow(color: Color.black.opacity(0.08), radius: 4, y: 2) .accessibilityAddTraits(.isButton) .onTapGesture { - viewModel.simpleProductsInfoButtonTapped() + showSimpleProductsModal = true } .padding(.bottom, Constants.bannerCardPadding) } @@ -126,7 +127,7 @@ private extension ItemListView { func listView(_ items: [any POSDisplayableItem]) -> some View { ScrollView { VStack { - if dynamicTypeSize.isAccessibilitySize, viewModel.shouldShowHeaderBanner { + if dynamicTypeSize.isAccessibilitySize, !isHeaderBannerDismissed { bannerCardView } ForEach(items, id: \.id) { item in @@ -138,13 +139,13 @@ private extension ItemListView { .background(GeometryReader { proxy in Color.clear .onChange(of: proxy.frame(in: .global).maxY) { maxY in - if case .loading = posModel.itemListState { + if case .loading = itemListState { return } let viewHeight = UIScreen.main.bounds.height if maxY < viewHeight { Task { - await viewModel.loadNextItems() + await loadNextItems() } } } @@ -204,15 +205,18 @@ private extension ItemListView { } } + +extension ItemListView { + struct BannerState { + static let isSimpleProductsOnlyBannerDismissedKey = "isSimpleProductsOnlyBannerDismissed" + } +} + #if DEBUG import class WooFoundation.MockAnalyticsPreview #Preview { - let posModel = PointOfSaleAggregateModel( - itemProvider: POSItemProviderPreview(), - cardPresentPaymentService: CardPresentPaymentPreviewService(), - orderService: POSOrderPreviewService(), - analytics: MockAnalyticsPreview()) - ItemListView(viewModel: ItemListViewModel(posModel: posModel), - posModel: posModel) + ItemListView(itemListState: .constant(.initialLoading), + loadNextItems: {}, + reloadItems: {}) } #endif diff --git a/WooCommerce/Classes/POS/Presentation/PointOfSaleDashboardView.swift b/WooCommerce/Classes/POS/Presentation/PointOfSaleDashboardView.swift index be3403d34fe..8471a839158 100644 --- a/WooCommerce/Classes/POS/Presentation/PointOfSaleDashboardView.swift +++ b/WooCommerce/Classes/POS/Presentation/PointOfSaleDashboardView.swift @@ -4,30 +4,34 @@ struct PointOfSaleDashboardView: View { @ObservedObject private var viewModel: PointOfSaleDashboardViewModel @ObservedObject private var totalsViewModel: TotalsViewModel private let cartViewModel: CartViewModel - private let itemListViewModel: ItemListViewModel @ObservedObject private var posModel: PointOfSaleAggregateModel @State var showExitPOSModal: Bool = false @State var showSupport: Bool = false + @State private var itemListState: PointOfSaleItemListState = .initialLoading + + private let itemsService: POSItemsService + @State private var allItems: [any POSDisplayableItem] = [] + private var currentPage: Int = Constants.initialPage init(viewModel: PointOfSaleDashboardViewModel, totalsViewModel: TotalsViewModel, cartViewModel: CartViewModel, - itemListViewModel: ItemListViewModel, - posModel: PointOfSaleAggregateModel) { + posModel: PointOfSaleAggregateModel, + itemsService: POSItemsService) { self.viewModel = viewModel self.totalsViewModel = totalsViewModel self.cartViewModel = cartViewModel - self.itemListViewModel = itemListViewModel self.posModel = posModel + self.itemsService = itemsService } @State private var floatingSize: CGSize = .zero var body: some View { ZStack(alignment: .bottomLeading) { - switch posModel.itemListState { + switch itemListState { case .initialLoading: PointOfSaleLoadingView() .transition(.opacity) @@ -35,7 +39,7 @@ struct PointOfSaleDashboardView: View { case .error(let errorContents): PointOfSaleItemListErrorView(error: errorContents, onRetry: { Task { - await posModel.reloadItems() + await reloadItems() } }) case .empty: @@ -52,7 +56,7 @@ struct PointOfSaleDashboardView: View { .offset(x: Constants.floatingControlHorizontalOffset, y: -Constants.floatingControlVerticalOffset) .trackSize(size: $floatingSize) .accessibilitySortPriority(1) - .renderedIf(posModel.itemListState != .initialLoading) + .renderedIf(itemListState != .initialLoading) POSConnectivityView() .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .top) @@ -64,7 +68,7 @@ struct PointOfSaleDashboardView: View { CGSizeMake(floatingSize.width + Constants.floatingControlHorizontalOffset, floatingSize.height + Constants.floatingControlVerticalOffset)) .environment(\.posBackgroundAppearance, posModel.paymentState != .processingPayment ? .primary : .secondary) - .animation(.easeInOut, value: posModel.itemListState == .initialLoading) + .animation(.easeInOut, value: itemListState == .initialLoading) .animation(.easeInOut(duration: Constants.connectivityAnimationDuration), value: viewModel.showsConnectivityError) .background(Color.posPrimaryBackground) .navigationBarBackButtonHidden(true) @@ -87,7 +91,7 @@ struct PointOfSaleDashboardView: View { supportForm } .task { - await posModel.loadInitialItems() + await loadInitialItems() } } @@ -162,6 +166,64 @@ private extension PointOfSaleDashboardView { } } +private extension PointOfSaleDashboardView { + @MainActor + func loadInitialItems() async { + do { + itemListState = .initialLoading + try await fetchItems(pageNumber: 1) + } catch { + itemListState = .error(PointOfSaleErrorState.errorOnLoadingProducts()) + } + } + + @MainActor + func loadNextItems() async { + // TODO: Optimize API calls. gh-14186 + // If there are no more pages to fetch, we can avoid the next call. + let nextPage = currentPage + 1 + await loadItems(pageNumber: nextPage) + } + + @MainActor + func loadItems(pageNumber: Int) async { + do { + itemListState = .loading(allItems) + try await fetchItems(pageNumber: pageNumber) + } catch { + itemListState = .error(PointOfSaleErrorState.errorOnLoadingProducts()) + } + } + + @MainActor + func fetchItems(pageNumber: Int) async throws { + let newItems = try await itemsService.fetchItems(pageNumber: pageNumber) + let uniqueNewItems = newItems + .filter { newItem in + !allItems.contains(where: { $0.id == newItem.itemID }) + } + .compactMap(createPOSDisplayableItem(for:)) + + allItems.append(contentsOf: uniqueNewItems) + + if allItems.count == 0 { + itemListState = .empty + } else { + itemListState = .loaded(allItems) + } + } + + @MainActor + func reloadItems() async { + removeAllItems() + await loadItems(pageNumber: 1) + } + + func removeAllItems() { + allItems.removeAll() + } +} + struct FloatingControlAreaSizeKey: EnvironmentKey { static let defaultValue = CGSize.zero } @@ -183,6 +245,8 @@ private extension PointOfSaleDashboardView { static let exitPOSSheetMaxWidth: CGFloat = 900.0 static let supportTag = "origin:point-of-sale" static let connectivityAnimationDuration: CGFloat = 1.0 + + static let initialPage: Int = 1 } enum Localization { @@ -212,7 +276,9 @@ private extension PointOfSaleDashboardView { } var productListView: some View { - ItemListView(viewModel: itemListViewModel, posModel: posModel) + ItemListView(itemListState: $itemListState, + loadNextItems: loadNextItems, + reloadItems: reloadItems) } } @@ -223,7 +289,6 @@ import class WooFoundation.MockAnalyticsProviderPreview #Preview { let posModel = PointOfSaleAggregateModel( - itemProvider: POSItemProviderPreview(), cardPresentPaymentService: CardPresentPaymentPreviewService(), orderService: POSOrderPreviewService(), analytics: MockAnalyticsPreview()) @@ -232,7 +297,6 @@ import class WooFoundation.MockAnalyticsProviderPreview currencyFormatter: .init(currencySettings: .init())) let cartVM = CartViewModel(analytics: MockAnalyticsPreview(), posModel: posModel) - let itemsListVM = ItemListViewModel(posModel: posModel) let posVM = PointOfSaleDashboardViewModel( posModel: posModel, connectivityObserver: POSConnectivityObserverPreview()) @@ -241,8 +305,8 @@ import class WooFoundation.MockAnalyticsProviderPreview PointOfSaleDashboardView(viewModel: posVM, totalsViewModel: totalsVM, cartViewModel: cartVM, - itemListViewModel: itemsListVM, - posModel: posModel) + posModel: posModel, + itemsService: POSItemsService(itemProvider: POSItemProviderPreview())) } } #endif diff --git a/WooCommerce/Classes/POS/Presentation/PointOfSaleEntryPointView.swift b/WooCommerce/Classes/POS/Presentation/PointOfSaleEntryPointView.swift index 055757fe9a1..6f1ff0ca0a8 100644 --- a/WooCommerce/Classes/POS/Presentation/PointOfSaleEntryPointView.swift +++ b/WooCommerce/Classes/POS/Presentation/PointOfSaleEntryPointView.swift @@ -8,9 +8,9 @@ struct PointOfSaleEntryPointView: View { @StateObject private var viewModel: PointOfSaleDashboardViewModel @StateObject private var totalsViewModel: TotalsViewModel @StateObject private var cartViewModel: CartViewModel - @StateObject private var itemListViewModel: ItemListViewModel @StateObject private var posModalManager = POSModalManager() @StateObject private var posModel: PointOfSaleAggregateModel + private var itemProvider: POSItemProvider private let onPointOfSaleModeActiveStateChange: ((Bool) -> Void) @@ -22,8 +22,9 @@ struct PointOfSaleEntryPointView: View { analytics: Analytics) { self.onPointOfSaleModeActiveStateChange = onPointOfSaleModeActiveStateChange + self.itemProvider = itemProvider + let posModel = PointOfSaleAggregateModel( - itemProvider: itemProvider, cardPresentPaymentService: cardPresentPaymentService, orderService: orderService, analytics: analytics) @@ -34,7 +35,6 @@ struct PointOfSaleEntryPointView: View { let cartViewModel = CartViewModel( analytics: analytics, posModel: posModel) - let itemListViewModel = ItemListViewModel(posModel: posModel) self._viewModel = StateObject(wrappedValue: PointOfSaleDashboardViewModel( posModel: posModel, @@ -42,7 +42,6 @@ struct PointOfSaleEntryPointView: View { ) self._cartViewModel = StateObject(wrappedValue: cartViewModel) self._totalsViewModel = StateObject(wrappedValue: totalsViewModel) - self._itemListViewModel = StateObject(wrappedValue: itemListViewModel) self._posModel = StateObject(wrappedValue: posModel) } @@ -50,8 +49,8 @@ struct PointOfSaleEntryPointView: View { PointOfSaleDashboardView(viewModel: viewModel, totalsViewModel: totalsViewModel, cartViewModel: cartViewModel, - itemListViewModel: itemListViewModel, - posModel: posModel) + posModel: posModel, + itemsService: POSItemsService(itemProvider: itemProvider)) .environmentObject(posModalManager) .environmentObject(posModel) .onAppear { diff --git a/WooCommerce/Classes/POS/Presentation/TotalsView.swift b/WooCommerce/Classes/POS/Presentation/TotalsView.swift index fe107815501..9e663485dcb 100644 --- a/WooCommerce/Classes/POS/Presentation/TotalsView.swift +++ b/WooCommerce/Classes/POS/Presentation/TotalsView.swift @@ -392,7 +392,6 @@ private extension View { import class WooFoundation.MockAnalyticsPreview #Preview { let posModel = PointOfSaleAggregateModel( - itemProvider: POSItemProviderPreview(), cardPresentPaymentService: CardPresentPaymentPreviewService(), orderService: POSOrderPreviewService(), analytics: MockAnalyticsPreview()) diff --git a/WooCommerce/Classes/POS/ViewModels/ItemListViewModel.swift b/WooCommerce/Classes/POS/ViewModels/ItemListViewModel.swift deleted file mode 100644 index 1fc96e30dd8..00000000000 --- a/WooCommerce/Classes/POS/ViewModels/ItemListViewModel.swift +++ /dev/null @@ -1,53 +0,0 @@ -import Combine -import SwiftUI -import protocol Yosemite.POSItem - -final class ItemListViewModel: ItemListViewModelProtocol { - let posModel: PointOfSaleAggregateModel - - @Published private(set) var isHeaderBannerDismissed: Bool = false - @Published var showSimpleProductsModal: Bool = false - - private var currentPage: Int = Constants.initialPage - - var shouldShowHeaderBanner: Bool { - // The banner it's shown as long as it hasn't already been dismissed once: - if UserDefaults.standard.bool(forKey: BannerState.isSimpleProductsOnlyBannerDismissedKey) == true { - return false - } - return !isHeaderBannerDismissed && posModel.itemListState != .empty - } - - init(posModel: PointOfSaleAggregateModel) { - self.posModel = posModel - } - - @MainActor - func loadNextItems() async { - // TODO: Optimize API calls. gh-14186 - // If there are no more pages to fetch, we can avoid the next call. - let nextPage = currentPage + 1 - await posModel.loadItems(pageNumber: nextPage) - } - - func dismissBanner() { - isHeaderBannerDismissed = true - UserDefaults.standard.set(isHeaderBannerDismissed, forKey: BannerState.isSimpleProductsOnlyBannerDismissedKey) - } - - func simpleProductsInfoButtonTapped() { - showSimpleProductsModal = true - } -} - -extension ItemListViewModel { - struct BannerState { - static let isSimpleProductsOnlyBannerDismissedKey = "isSimpleProductsOnlyBannerDismissed" - } -} - -private extension ItemListViewModel { - enum Constants { - static let initialPage: Int = 1 - } -} diff --git a/WooCommerce/Classes/POS/ViewModels/ItemListViewModelProtocol.swift b/WooCommerce/Classes/POS/ViewModels/ItemListViewModelProtocol.swift deleted file mode 100644 index be556d658b5..00000000000 --- a/WooCommerce/Classes/POS/ViewModels/ItemListViewModelProtocol.swift +++ /dev/null @@ -1,11 +0,0 @@ -import Combine -import Foundation -import protocol Yosemite.POSItem - -protocol ItemListViewModelProtocol: ObservableObject { - var isHeaderBannerDismissed: Bool { get } - var shouldShowHeaderBanner: Bool { get } - - func loadNextItems() async - func dismissBanner() -} diff --git a/WooCommerce/Classes/POS/ViewModels/PointOfSaleDashboardViewModel.swift b/WooCommerce/Classes/POS/ViewModels/PointOfSaleDashboardViewModel.swift index 4f38647a796..5e735ca9cfa 100644 --- a/WooCommerce/Classes/POS/ViewModels/PointOfSaleDashboardViewModel.swift +++ b/WooCommerce/Classes/POS/ViewModels/PointOfSaleDashboardViewModel.swift @@ -11,8 +11,6 @@ final class PointOfSaleDashboardViewModel: ObservableObject { private let connectivityObserver: ConnectivityObserver - @Published var isError: Bool = false - @Published var isEmpty: Bool = false @Published var showsConnectivityError: Bool = false private var cancellables: Set = [] @@ -22,31 +20,10 @@ final class PointOfSaleDashboardViewModel: ObservableObject { self.posModel = posModel self.connectivityObserver = connectivityObserver - observeItemListState() observeConnectivity() } } -private extension PointOfSaleDashboardViewModel { - func observeItemListState() { - posModel.$itemListState - .receive(on: DispatchQueue.main) - .sink { [weak self] state in - guard let self = self else { return } - switch state { - case .error: - self.isError = true - case .empty: - self.isEmpty = true - default: - self.isError = false - self.isEmpty = false - } - } - .store(in: &cancellables) - } -} - private extension PointOfSaleDashboardViewModel { func observeConnectivity() { connectivityObserver.statusPublisher diff --git a/WooCommerce/WooCommerce.xcodeproj/project.pbxproj b/WooCommerce/WooCommerce.xcodeproj/project.pbxproj index e2617c860b8..6295a33fdd6 100644 --- a/WooCommerce/WooCommerce.xcodeproj/project.pbxproj +++ b/WooCommerce/WooCommerce.xcodeproj/project.pbxproj @@ -35,7 +35,6 @@ 0188CA0F2C65622A0051BF1C /* PointOfSaleCardPresentPaymentValidatingOrderErrorMessageViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0188CA0E2C65622A0051BF1C /* PointOfSaleCardPresentPaymentValidatingOrderErrorMessageViewModel.swift */; }; 0188CA112C6565320051BF1C /* PointOfSaleCardPresentPaymentValidatingOrderErrorMessageView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0188CA102C6565320051BF1C /* PointOfSaleCardPresentPaymentValidatingOrderErrorMessageView.swift */; }; 018D5C7E2CA6B4A60085EBEE /* CurrencySettings+Sanitized.swift in Sources */ = {isa = PBXBuildFile; fileRef = 018D5C7D2CA6B49D0085EBEE /* CurrencySettings+Sanitized.swift */; }; - 019C5EB02C516F7B005B82D3 /* ItemListViewModelProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 019C5EAF2C516F7B005B82D3 /* ItemListViewModelProtocol.swift */; }; 019C5EB22C5171E6005B82D3 /* MockItemListViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 019C5EB12C5171E6005B82D3 /* MockItemListViewModel.swift */; }; 01ADC1362C9AB4810036F7D2 /* PointOfSaleCardPresentPaymentIntentCreationErrorMessageViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 01ADC1352C9AB4810036F7D2 /* PointOfSaleCardPresentPaymentIntentCreationErrorMessageViewModel.swift */; }; 01ADC1382C9AB6050036F7D2 /* PointOfSaleCardPresentPaymentIntentCreationErrorMessageView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 01ADC1372C9AB6050036F7D2 /* PointOfSaleCardPresentPaymentIntentCreationErrorMessageView.swift */; }; @@ -832,6 +831,7 @@ 20B7E3172CD100DC007FD997 /* PointOfSalePaymentState.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20B7E3162CD100DC007FD997 /* PointOfSalePaymentState.swift */; }; 20B7E3192CD13CE3007FD997 /* PointOfSaleItemListState.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20B7E3182CD13CE3007FD997 /* PointOfSaleItemListState.swift */; }; 20B7E3512CD3DAB5007FD997 /* POSDisplayableItem.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20B7E3502CD3DAB5007FD997 /* POSDisplayableItem.swift */; }; + 20B7E3552CD4F59C007FD997 /* POSItemsService.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20B7E3542CD4F59C007FD997 /* POSItemsService.swift */; }; 20BBD62C2B3060A300A903F6 /* AddOrderComponentsSection.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20BBD62B2B3060A300A903F6 /* AddOrderComponentsSection.swift */; }; 20BCF6EE2B0E478B00954840 /* WooPaymentsDepositsOverviewViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20BCF6ED2B0E478B00954840 /* WooPaymentsDepositsOverviewViewModel.swift */; }; 20BCF6F02B0E48CC00954840 /* WooPaymentsDepositsOverviewViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 20BCF6EF2B0E48CC00954840 /* WooPaymentsDepositsOverviewViewModelTests.swift */; }; @@ -1495,7 +1495,6 @@ 6832C7CA26DA5C4500BA4088 /* LabeledTextViewTableViewCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6832C7C926DA5C4500BA4088 /* LabeledTextViewTableViewCell.swift */; }; 6832C7CC26DA5FDF00BA4088 /* LabeledTextViewTableViewCell.xib in Resources */ = {isa = PBXBuildFile; fileRef = 6832C7CB26DA5FDE00BA4088 /* LabeledTextViewTableViewCell.xib */; }; 683421642ACE9391009021D7 /* ProductDiscountView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 683421632ACE9391009021D7 /* ProductDiscountView.swift */; }; - 683763162C2E44B800AD51D0 /* ItemListViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 683763152C2E44B800AD51D0 /* ItemListViewModel.swift */; }; 683763182C2E497000AD51D0 /* ItemListViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 683763172C2E497000AD51D0 /* ItemListViewModelTests.swift */; }; 6837631A2C2E6F5900AD51D0 /* CartViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 683763192C2E6F5900AD51D0 /* CartViewModelTests.swift */; }; 6837631C2C2E847D00AD51D0 /* CartViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6837631B2C2E847D00AD51D0 /* CartViewModel.swift */; }; @@ -3122,7 +3121,6 @@ 0188CA0E2C65622A0051BF1C /* PointOfSaleCardPresentPaymentValidatingOrderErrorMessageViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PointOfSaleCardPresentPaymentValidatingOrderErrorMessageViewModel.swift; sourceTree = ""; }; 0188CA102C6565320051BF1C /* PointOfSaleCardPresentPaymentValidatingOrderErrorMessageView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PointOfSaleCardPresentPaymentValidatingOrderErrorMessageView.swift; sourceTree = ""; }; 018D5C7D2CA6B49D0085EBEE /* CurrencySettings+Sanitized.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CurrencySettings+Sanitized.swift"; sourceTree = ""; }; - 019C5EAF2C516F7B005B82D3 /* ItemListViewModelProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ItemListViewModelProtocol.swift; sourceTree = ""; }; 019C5EB12C5171E6005B82D3 /* MockItemListViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockItemListViewModel.swift; sourceTree = ""; }; 01ADC1352C9AB4810036F7D2 /* PointOfSaleCardPresentPaymentIntentCreationErrorMessageViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PointOfSaleCardPresentPaymentIntentCreationErrorMessageViewModel.swift; sourceTree = ""; }; 01ADC1372C9AB6050036F7D2 /* PointOfSaleCardPresentPaymentIntentCreationErrorMessageView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PointOfSaleCardPresentPaymentIntentCreationErrorMessageView.swift; sourceTree = ""; }; @@ -3924,6 +3922,7 @@ 20B7E3162CD100DC007FD997 /* PointOfSalePaymentState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PointOfSalePaymentState.swift; sourceTree = ""; }; 20B7E3182CD13CE3007FD997 /* PointOfSaleItemListState.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PointOfSaleItemListState.swift; sourceTree = ""; }; 20B7E3502CD3DAB5007FD997 /* POSDisplayableItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = POSDisplayableItem.swift; sourceTree = ""; }; + 20B7E3542CD4F59C007FD997 /* POSItemsService.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = POSItemsService.swift; sourceTree = ""; }; 20BBD62B2B3060A300A903F6 /* AddOrderComponentsSection.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AddOrderComponentsSection.swift; sourceTree = ""; }; 20BCF6ED2B0E478B00954840 /* WooPaymentsDepositsOverviewViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WooPaymentsDepositsOverviewViewModel.swift; sourceTree = ""; }; 20BCF6EF2B0E48CC00954840 /* WooPaymentsDepositsOverviewViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WooPaymentsDepositsOverviewViewModelTests.swift; sourceTree = ""; }; @@ -4521,7 +4520,6 @@ 6832C7C926DA5C4500BA4088 /* LabeledTextViewTableViewCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LabeledTextViewTableViewCell.swift; sourceTree = ""; }; 6832C7CB26DA5FDE00BA4088 /* LabeledTextViewTableViewCell.xib */ = {isa = PBXFileReference; lastKnownFileType = file.xib; path = LabeledTextViewTableViewCell.xib; sourceTree = ""; }; 683421632ACE9391009021D7 /* ProductDiscountView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductDiscountView.swift; sourceTree = ""; }; - 683763152C2E44B800AD51D0 /* ItemListViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ItemListViewModel.swift; sourceTree = ""; }; 683763172C2E497000AD51D0 /* ItemListViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ItemListViewModelTests.swift; sourceTree = ""; }; 683763192C2E6F5900AD51D0 /* CartViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CartViewModelTests.swift; sourceTree = ""; }; 6837631B2C2E847D00AD51D0 /* CartViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CartViewModel.swift; sourceTree = ""; }; @@ -6788,8 +6786,6 @@ isa = PBXGroup; children = ( 026826922BF59D830036F959 /* PointOfSaleDashboardViewModel.swift */, - 019C5EAF2C516F7B005B82D3 /* ItemListViewModelProtocol.swift */, - 683763152C2E44B800AD51D0 /* ItemListViewModel.swift */, C7F746C82C4868670023ADD0 /* TotalsViewModelProtocol.swift */, 6885E2CB2C32B14B004C8D70 /* TotalsViewModel.swift */, 016BCAFE2C4F907F009D8367 /* CartViewModelProtocol.swift */, @@ -7103,6 +7099,7 @@ 026826972BF59D9E0036F959 /* Utils */, 026826A12BF59DED0036F959 /* Presentation */, 20B7E3122CCFFEB4007FD997 /* PointOfSaleAggregateModel.swift */, + 20B7E3542CD4F59C007FD997 /* POSItemsService.swift */, 20B7E3502CD3DAB5007FD997 /* POSDisplayableItem.swift */, 20B7E3182CD13CE3007FD997 /* PointOfSaleItemListState.swift */, 20B7E3162CD100DC007FD997 /* PointOfSalePaymentState.swift */, @@ -14880,7 +14877,6 @@ 683DF5FF2C6AF46500A5CDC6 /* POSHeaderTitleView.swift in Sources */, DEA90D392C9156A20021ABC3 /* BlazeCampaignDetailWebViewModel.swift in Sources */, DE7842F726F2E9340030C792 /* UIViewController+Connectivity.swift in Sources */, - 019C5EB02C516F7B005B82D3 /* ItemListViewModelProtocol.swift in Sources */, E1C47209267A1ECC00D06DA1 /* CrashLoggingStack.swift in Sources */, B991F5382B7BCA27001ADF83 /* LocallyStoredStateNameRetriever.swift in Sources */, CCF87BBE279047BC00461C43 /* InfiniteScrollList.swift in Sources */, @@ -14888,6 +14884,7 @@ DE7E5E8C2B4E9353002E28D2 /* ErrorStateView.swift in Sources */, 20ADE9412C6A02B700C91265 /* POSErrorXMark.swift in Sources */, DE63115B2AF1E13200587641 /* WPComLoginCoordinator.swift in Sources */, + 20B7E3552CD4F59C007FD997 /* POSItemsService.swift in Sources */, EE3B17B62AA03837004D3E0C /* CelebrationView.swift in Sources */, D85A3C5226C15DE200C0E026 /* InPersonPaymentsPluginNotSupportedVersionView.swift in Sources */, EE57C11D297AC27300BC31E7 /* TrackEventRequestNotificationHandler.swift in Sources */, @@ -15103,7 +15100,6 @@ 6837631C2C2E847D00AD51D0 /* CartViewModel.swift in Sources */, 45CDAFAE2434CFCA00F83C22 /* ProductCatalogVisibilityViewController.swift in Sources */, D85B8333222FABD1002168F3 /* StatusListTableViewCell.swift in Sources */, - 683763162C2E44B800AD51D0 /* ItemListViewModel.swift in Sources */, DE2E8EB729547771002E4B14 /* ApplicationPasswordDisabledViewModel.swift in Sources */, 0259D65D2582248D003B1CD6 /* PrintShippingLabelViewController.swift in Sources */, D881A31B256B5CC500FE5605 /* ULErrorViewController.swift in Sources */, From 1d652af4c2360fc2fb6a4afe8eb7a002f1731618 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Fri, 1 Nov 2024 15:22:10 +0000 Subject: [PATCH 2/4] Attempt to move item list state to listview MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unfortunately it doesn’t quite work to bind upwards for the dashboard view to show fullscreen stuff instead of the item list. Fundamentally, if the itemlist isn’t showing, it’s state isn’t considered any more. --- .../POS/PointOfSaleItemListState.swift | 4 +- .../POS/Presentation/ItemListView.swift | 104 ++++++++++++-- .../PointOfSaleDashboardView.swift | 129 ++++++------------ .../PointOfSaleEntryPointView.swift | 2 +- 4 files changed, 141 insertions(+), 98 deletions(-) diff --git a/WooCommerce/Classes/POS/PointOfSaleItemListState.swift b/WooCommerce/Classes/POS/PointOfSaleItemListState.swift index e7ca26fba36..5b5f0652590 100644 --- a/WooCommerce/Classes/POS/PointOfSaleItemListState.swift +++ b/WooCommerce/Classes/POS/PointOfSaleItemListState.swift @@ -1,6 +1,7 @@ import protocol Yosemite.POSItem enum PointOfSaleItemListState: Equatable { + case initializing case empty case initialLoading case loading(_ existingItems: [any POSDisplayableItem]) @@ -10,7 +11,8 @@ enum PointOfSaleItemListState: Equatable { // Equatable conformance for testing: static func == (lhs: PointOfSaleItemListState, rhs: PointOfSaleItemListState) -> Bool { switch (lhs, rhs) { - case (.initialLoading, .initialLoading), + case (.initializing, .initializing), + (.initialLoading, .initialLoading), (.empty, .empty): return true case (.loading(let lhsItems), .loading(let rhsItems)), diff --git a/WooCommerce/Classes/POS/Presentation/ItemListView.swift b/WooCommerce/Classes/POS/Presentation/ItemListView.swift index 35d0b24e769..1eca433963d 100644 --- a/WooCommerce/Classes/POS/Presentation/ItemListView.swift +++ b/WooCommerce/Classes/POS/Presentation/ItemListView.swift @@ -2,17 +2,25 @@ import SwiftUI import protocol Yosemite.POSItem struct ItemListView: View { - @Environment(\.floatingControlAreaSize) var floatingControlAreaSize: CGSize + @Environment(\.floatingControlAreaSize) private var floatingControlAreaSize: CGSize @Environment(\.dynamicTypeSize) private var dynamicTypeSize - @Binding var itemListState: PointOfSaleItemListState - - var loadNextItems: () async -> Void - var reloadItems: () async -> Void + @Binding var itemListContainerState: ItemListContainerViewState @State private var isHeaderBannerDismissed: Bool = UserDefaults.standard.bool(forKey: BannerState.isSimpleProductsOnlyBannerDismissedKey) @State private var showSimpleProductsModal: Bool = false + @State private var itemListState: PointOfSaleItemListState = .initializing + let itemsService: POSItemsService + @State private var allItems: [any POSDisplayableItem] = [] + private var currentPage: Int = Constants.initialPage + + init(itemListContainerState: Binding, + itemsService: POSItemsService) { + self._itemListContainerState = itemListContainerState + self.itemsService = itemsService + } + var body: some View { VStack { headerView @@ -20,14 +28,29 @@ struct ItemListView: View { SimpleProductsOnlyInformation(isPresented: $showSimpleProductsModal) } switch itemListState { - case .initialLoading, .empty, .error: - // These cases are handled directly in the dashboard, we do not render - // a specific view within the ItemListView to handle them + case .initializing, .initialLoading, .empty, .error: + // These cases are shown in the dashboard, we do not render + // a specific view within the ItemListView to handle them, we just set bindings EmptyView() case .loading(let items), .loaded(let items): listView(items) } } + .task { + await loadInitialItems() + } + .onChange(of: itemListState, perform: { newValue in + switch newValue { + case .initialLoading: + itemListContainerState = .initialLoading + case .empty: + itemListContainerState = .empty + case .error(let errorState): + itemListContainerState = .error(errorState, reloadItems) + case .initializing, .loaded, .loading: + itemListContainerState = .show + } + }) .refreshable { await reloadItems() } @@ -154,6 +177,64 @@ private extension ItemListView { } } +private extension ItemListView { + @MainActor + func loadInitialItems() async { + do { + itemListState = .initialLoading + try await fetchItems(pageNumber: 1) + } catch { + itemListState = .error(PointOfSaleErrorState.errorOnLoadingProducts()) + } + } + + @MainActor + func loadNextItems() async { + // TODO: Optimize API calls. gh-14186 + // If there are no more pages to fetch, we can avoid the next call. + let nextPage = currentPage + 1 + await loadItems(pageNumber: nextPage) + } + + @MainActor + func loadItems(pageNumber: Int) async { + do { + itemListState = .loading(allItems) + try await fetchItems(pageNumber: pageNumber) + } catch { + itemListState = .error(PointOfSaleErrorState.errorOnLoadingProducts()) + } + } + + @MainActor + func fetchItems(pageNumber: Int) async throws { + let newItems = try await itemsService.fetchItems(pageNumber: pageNumber) + let uniqueNewItems = newItems + .filter { newItem in + !allItems.contains(where: { $0.id == newItem.itemID }) + } + .compactMap(createPOSDisplayableItem(for:)) + + allItems.append(contentsOf: uniqueNewItems) + + if allItems.count == 0 { + itemListState = .empty + } else { + itemListState = .loaded(allItems) + } + } + + @MainActor + func reloadItems() async { + removeAllItems() + await loadItems(pageNumber: 1) + } + + func removeAllItems() { + allItems.removeAll() + } +} + /// Constants /// private extension ItemListView { @@ -169,6 +250,8 @@ private extension ItemListView { static let iconPadding: CGFloat = 26 static let itemListPadding: CGFloat = 16 static let bannerCardPadding: CGFloat = 16 + + static let initialPage: Int = 1 } enum Localization { @@ -215,8 +298,7 @@ extension ItemListView { #if DEBUG import class WooFoundation.MockAnalyticsPreview #Preview { - ItemListView(itemListState: .constant(.initialLoading), - loadNextItems: {}, - reloadItems: {}) + ItemListView(itemListContainerState: .constant(.show), + itemsService: POSItemsService(itemProvider: POSItemProviderPreview())) } #endif diff --git a/WooCommerce/Classes/POS/Presentation/PointOfSaleDashboardView.swift b/WooCommerce/Classes/POS/Presentation/PointOfSaleDashboardView.swift index 8471a839158..26bcd5f2f11 100644 --- a/WooCommerce/Classes/POS/Presentation/PointOfSaleDashboardView.swift +++ b/WooCommerce/Classes/POS/Presentation/PointOfSaleDashboardView.swift @@ -1,50 +1,53 @@ import SwiftUI +import protocol Yosemite.POSItemProvider struct PointOfSaleDashboardView: View { @ObservedObject private var viewModel: PointOfSaleDashboardViewModel @ObservedObject private var totalsViewModel: TotalsViewModel private let cartViewModel: CartViewModel + private var itemProvider: POSItemProvider + @ObservedObject private var posModel: PointOfSaleAggregateModel + @State private var itemListContainerState: ItemListContainerViewState = .initializing @State var showExitPOSModal: Bool = false @State var showSupport: Bool = false - @State private var itemListState: PointOfSaleItemListState = .initialLoading - - private let itemsService: POSItemsService - @State private var allItems: [any POSDisplayableItem] = [] - private var currentPage: Int = Constants.initialPage init(viewModel: PointOfSaleDashboardViewModel, totalsViewModel: TotalsViewModel, cartViewModel: CartViewModel, posModel: PointOfSaleAggregateModel, - itemsService: POSItemsService) { + itemProvider: POSItemProvider) { self.viewModel = viewModel self.totalsViewModel = totalsViewModel self.cartViewModel = cartViewModel self.posModel = posModel - self.itemsService = itemsService + self.itemProvider = itemProvider } @State private var floatingSize: CGSize = .zero var body: some View { ZStack(alignment: .bottomLeading) { - switch itemListState { + switch itemListContainerState { case .initialLoading: - PointOfSaleLoadingView() - .transition(.opacity) - .ignoresSafeArea() - case .error(let errorContents): - PointOfSaleItemListErrorView(error: errorContents, onRetry: { + contentView + .accessibilitySortPriority(2) + .overlay { + PointOfSaleLoadingView() + .transition(.opacity) + .ignoresSafeArea() + } + case .error(let errorState, let reloadItems): + PointOfSaleItemListErrorView(error: errorState, onRetry: { Task { await reloadItems() } }) case .empty: PointOfSaleItemListEmptyView() - case .loading, .loaded: + case .initializing, .show: contentView .accessibilitySortPriority(2) } @@ -56,7 +59,7 @@ struct PointOfSaleDashboardView: View { .offset(x: Constants.floatingControlHorizontalOffset, y: -Constants.floatingControlVerticalOffset) .trackSize(size: $floatingSize) .accessibilitySortPriority(1) - .renderedIf(itemListState != .initialLoading) + .renderedIf(itemListContainerState != .initialLoading) POSConnectivityView() .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .top) @@ -68,7 +71,7 @@ struct PointOfSaleDashboardView: View { CGSizeMake(floatingSize.width + Constants.floatingControlHorizontalOffset, floatingSize.height + Constants.floatingControlVerticalOffset)) .environment(\.posBackgroundAppearance, posModel.paymentState != .processingPayment ? .primary : .secondary) - .animation(.easeInOut, value: itemListState == .initialLoading) + .animation(.easeInOut, value: itemListContainerState == .initialLoading) .animation(.easeInOut(duration: Constants.connectivityAnimationDuration), value: viewModel.showsConnectivityError) .background(Color.posPrimaryBackground) .navigationBarBackButtonHidden(true) @@ -90,18 +93,16 @@ struct PointOfSaleDashboardView: View { .sheet(isPresented: $showSupport) { supportForm } - .task { - await loadInitialItems() - } } private var contentView: some View { GeometryReader { geometry in HStack { if posModel.orderStage == .building { - productListView - .accessibilitySortPriority(2) - .transition(.move(edge: .leading)) + ItemListView(itemListContainerState: $itemListContainerState, + itemsService: POSItemsService(itemProvider: itemProvider)) + .accessibilitySortPriority(2) + .transition(.move(edge: .leading)) } if !posModel.paymentState.cardHasBeenTapped { @@ -166,63 +167,7 @@ private extension PointOfSaleDashboardView { } } -private extension PointOfSaleDashboardView { - @MainActor - func loadInitialItems() async { - do { - itemListState = .initialLoading - try await fetchItems(pageNumber: 1) - } catch { - itemListState = .error(PointOfSaleErrorState.errorOnLoadingProducts()) - } - } - @MainActor - func loadNextItems() async { - // TODO: Optimize API calls. gh-14186 - // If there are no more pages to fetch, we can avoid the next call. - let nextPage = currentPage + 1 - await loadItems(pageNumber: nextPage) - } - - @MainActor - func loadItems(pageNumber: Int) async { - do { - itemListState = .loading(allItems) - try await fetchItems(pageNumber: pageNumber) - } catch { - itemListState = .error(PointOfSaleErrorState.errorOnLoadingProducts()) - } - } - - @MainActor - func fetchItems(pageNumber: Int) async throws { - let newItems = try await itemsService.fetchItems(pageNumber: pageNumber) - let uniqueNewItems = newItems - .filter { newItem in - !allItems.contains(where: { $0.id == newItem.itemID }) - } - .compactMap(createPOSDisplayableItem(for:)) - - allItems.append(contentsOf: uniqueNewItems) - - if allItems.count == 0 { - itemListState = .empty - } else { - itemListState = .loaded(allItems) - } - } - - @MainActor - func reloadItems() async { - removeAllItems() - await loadItems(pageNumber: 1) - } - - func removeAllItems() { - allItems.removeAll() - } -} struct FloatingControlAreaSizeKey: EnvironmentKey { static let defaultValue = CGSize.zero @@ -245,8 +190,6 @@ private extension PointOfSaleDashboardView { static let exitPOSSheetMaxWidth: CGFloat = 900.0 static let supportTag = "origin:point-of-sale" static let connectivityAnimationDuration: CGFloat = 1.0 - - static let initialPage: Int = 1 } enum Localization { @@ -274,11 +217,27 @@ private extension PointOfSaleDashboardView { var totalsView: some View { TotalsView(viewModel: totalsViewModel, posModel: posModel) } +} - var productListView: some View { - ItemListView(itemListState: $itemListState, - loadNextItems: loadNextItems, - reloadItems: reloadItems) +enum ItemListContainerViewState: Equatable { + case initializing + case initialLoading + case empty + case error(_ errorState: PointOfSaleErrorState, _ reloadItems: () async -> Void) + case show + + static func ==(lhs: ItemListContainerViewState, rhs: ItemListContainerViewState) -> Bool { + switch (lhs, rhs) { + case (.initializing, .initializing), + (.initialLoading, .initialLoading), + (.empty, .empty), + (.show, .show): + return true + case (.error(let lhsErrorState, _), .error(let rhsErrorState, _)): + return lhsErrorState == rhsErrorState + default: + return false + } } } @@ -306,7 +265,7 @@ import class WooFoundation.MockAnalyticsProviderPreview totalsViewModel: totalsVM, cartViewModel: cartVM, posModel: posModel, - itemsService: POSItemsService(itemProvider: POSItemProviderPreview())) + itemProvider: POSItemProviderPreview()) } } #endif diff --git a/WooCommerce/Classes/POS/Presentation/PointOfSaleEntryPointView.swift b/WooCommerce/Classes/POS/Presentation/PointOfSaleEntryPointView.swift index 6f1ff0ca0a8..92474ad146a 100644 --- a/WooCommerce/Classes/POS/Presentation/PointOfSaleEntryPointView.swift +++ b/WooCommerce/Classes/POS/Presentation/PointOfSaleEntryPointView.swift @@ -50,7 +50,7 @@ struct PointOfSaleEntryPointView: View { totalsViewModel: totalsViewModel, cartViewModel: cartViewModel, posModel: posModel, - itemsService: POSItemsService(itemProvider: itemProvider)) + itemProvider: itemProvider) .environmentObject(posModalManager) .environmentObject(posModel) .onAppear { From 24222f0c4f638ba556f00ea4d6d2d08ad4f4a42f Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Fri, 1 Nov 2024 15:25:00 +0000 Subject: [PATCH 3/4] Revert "Attempt to move item list state to listview" This reverts commit 1d652af4c2360fc2fb6a4afe8eb7a002f1731618. --- .../POS/PointOfSaleItemListState.swift | 4 +- .../POS/Presentation/ItemListView.swift | 104 ++------------ .../PointOfSaleDashboardView.swift | 129 ++++++++++++------ .../PointOfSaleEntryPointView.swift | 2 +- 4 files changed, 98 insertions(+), 141 deletions(-) diff --git a/WooCommerce/Classes/POS/PointOfSaleItemListState.swift b/WooCommerce/Classes/POS/PointOfSaleItemListState.swift index 5b5f0652590..e7ca26fba36 100644 --- a/WooCommerce/Classes/POS/PointOfSaleItemListState.swift +++ b/WooCommerce/Classes/POS/PointOfSaleItemListState.swift @@ -1,7 +1,6 @@ import protocol Yosemite.POSItem enum PointOfSaleItemListState: Equatable { - case initializing case empty case initialLoading case loading(_ existingItems: [any POSDisplayableItem]) @@ -11,8 +10,7 @@ enum PointOfSaleItemListState: Equatable { // Equatable conformance for testing: static func == (lhs: PointOfSaleItemListState, rhs: PointOfSaleItemListState) -> Bool { switch (lhs, rhs) { - case (.initializing, .initializing), - (.initialLoading, .initialLoading), + case (.initialLoading, .initialLoading), (.empty, .empty): return true case (.loading(let lhsItems), .loading(let rhsItems)), diff --git a/WooCommerce/Classes/POS/Presentation/ItemListView.swift b/WooCommerce/Classes/POS/Presentation/ItemListView.swift index 1eca433963d..35d0b24e769 100644 --- a/WooCommerce/Classes/POS/Presentation/ItemListView.swift +++ b/WooCommerce/Classes/POS/Presentation/ItemListView.swift @@ -2,25 +2,17 @@ import SwiftUI import protocol Yosemite.POSItem struct ItemListView: View { - @Environment(\.floatingControlAreaSize) private var floatingControlAreaSize: CGSize + @Environment(\.floatingControlAreaSize) var floatingControlAreaSize: CGSize @Environment(\.dynamicTypeSize) private var dynamicTypeSize - @Binding var itemListContainerState: ItemListContainerViewState + @Binding var itemListState: PointOfSaleItemListState + + var loadNextItems: () async -> Void + var reloadItems: () async -> Void @State private var isHeaderBannerDismissed: Bool = UserDefaults.standard.bool(forKey: BannerState.isSimpleProductsOnlyBannerDismissedKey) @State private var showSimpleProductsModal: Bool = false - @State private var itemListState: PointOfSaleItemListState = .initializing - let itemsService: POSItemsService - @State private var allItems: [any POSDisplayableItem] = [] - private var currentPage: Int = Constants.initialPage - - init(itemListContainerState: Binding, - itemsService: POSItemsService) { - self._itemListContainerState = itemListContainerState - self.itemsService = itemsService - } - var body: some View { VStack { headerView @@ -28,29 +20,14 @@ struct ItemListView: View { SimpleProductsOnlyInformation(isPresented: $showSimpleProductsModal) } switch itemListState { - case .initializing, .initialLoading, .empty, .error: - // These cases are shown in the dashboard, we do not render - // a specific view within the ItemListView to handle them, we just set bindings + case .initialLoading, .empty, .error: + // These cases are handled directly in the dashboard, we do not render + // a specific view within the ItemListView to handle them EmptyView() case .loading(let items), .loaded(let items): listView(items) } } - .task { - await loadInitialItems() - } - .onChange(of: itemListState, perform: { newValue in - switch newValue { - case .initialLoading: - itemListContainerState = .initialLoading - case .empty: - itemListContainerState = .empty - case .error(let errorState): - itemListContainerState = .error(errorState, reloadItems) - case .initializing, .loaded, .loading: - itemListContainerState = .show - } - }) .refreshable { await reloadItems() } @@ -177,64 +154,6 @@ private extension ItemListView { } } -private extension ItemListView { - @MainActor - func loadInitialItems() async { - do { - itemListState = .initialLoading - try await fetchItems(pageNumber: 1) - } catch { - itemListState = .error(PointOfSaleErrorState.errorOnLoadingProducts()) - } - } - - @MainActor - func loadNextItems() async { - // TODO: Optimize API calls. gh-14186 - // If there are no more pages to fetch, we can avoid the next call. - let nextPage = currentPage + 1 - await loadItems(pageNumber: nextPage) - } - - @MainActor - func loadItems(pageNumber: Int) async { - do { - itemListState = .loading(allItems) - try await fetchItems(pageNumber: pageNumber) - } catch { - itemListState = .error(PointOfSaleErrorState.errorOnLoadingProducts()) - } - } - - @MainActor - func fetchItems(pageNumber: Int) async throws { - let newItems = try await itemsService.fetchItems(pageNumber: pageNumber) - let uniqueNewItems = newItems - .filter { newItem in - !allItems.contains(where: { $0.id == newItem.itemID }) - } - .compactMap(createPOSDisplayableItem(for:)) - - allItems.append(contentsOf: uniqueNewItems) - - if allItems.count == 0 { - itemListState = .empty - } else { - itemListState = .loaded(allItems) - } - } - - @MainActor - func reloadItems() async { - removeAllItems() - await loadItems(pageNumber: 1) - } - - func removeAllItems() { - allItems.removeAll() - } -} - /// Constants /// private extension ItemListView { @@ -250,8 +169,6 @@ private extension ItemListView { static let iconPadding: CGFloat = 26 static let itemListPadding: CGFloat = 16 static let bannerCardPadding: CGFloat = 16 - - static let initialPage: Int = 1 } enum Localization { @@ -298,7 +215,8 @@ extension ItemListView { #if DEBUG import class WooFoundation.MockAnalyticsPreview #Preview { - ItemListView(itemListContainerState: .constant(.show), - itemsService: POSItemsService(itemProvider: POSItemProviderPreview())) + ItemListView(itemListState: .constant(.initialLoading), + loadNextItems: {}, + reloadItems: {}) } #endif diff --git a/WooCommerce/Classes/POS/Presentation/PointOfSaleDashboardView.swift b/WooCommerce/Classes/POS/Presentation/PointOfSaleDashboardView.swift index 26bcd5f2f11..8471a839158 100644 --- a/WooCommerce/Classes/POS/Presentation/PointOfSaleDashboardView.swift +++ b/WooCommerce/Classes/POS/Presentation/PointOfSaleDashboardView.swift @@ -1,53 +1,50 @@ import SwiftUI -import protocol Yosemite.POSItemProvider struct PointOfSaleDashboardView: View { @ObservedObject private var viewModel: PointOfSaleDashboardViewModel @ObservedObject private var totalsViewModel: TotalsViewModel private let cartViewModel: CartViewModel - private var itemProvider: POSItemProvider - @ObservedObject private var posModel: PointOfSaleAggregateModel - @State private var itemListContainerState: ItemListContainerViewState = .initializing @State var showExitPOSModal: Bool = false @State var showSupport: Bool = false + @State private var itemListState: PointOfSaleItemListState = .initialLoading + + private let itemsService: POSItemsService + @State private var allItems: [any POSDisplayableItem] = [] + private var currentPage: Int = Constants.initialPage init(viewModel: PointOfSaleDashboardViewModel, totalsViewModel: TotalsViewModel, cartViewModel: CartViewModel, posModel: PointOfSaleAggregateModel, - itemProvider: POSItemProvider) { + itemsService: POSItemsService) { self.viewModel = viewModel self.totalsViewModel = totalsViewModel self.cartViewModel = cartViewModel self.posModel = posModel - self.itemProvider = itemProvider + self.itemsService = itemsService } @State private var floatingSize: CGSize = .zero var body: some View { ZStack(alignment: .bottomLeading) { - switch itemListContainerState { + switch itemListState { case .initialLoading: - contentView - .accessibilitySortPriority(2) - .overlay { - PointOfSaleLoadingView() - .transition(.opacity) - .ignoresSafeArea() - } - case .error(let errorState, let reloadItems): - PointOfSaleItemListErrorView(error: errorState, onRetry: { + PointOfSaleLoadingView() + .transition(.opacity) + .ignoresSafeArea() + case .error(let errorContents): + PointOfSaleItemListErrorView(error: errorContents, onRetry: { Task { await reloadItems() } }) case .empty: PointOfSaleItemListEmptyView() - case .initializing, .show: + case .loading, .loaded: contentView .accessibilitySortPriority(2) } @@ -59,7 +56,7 @@ struct PointOfSaleDashboardView: View { .offset(x: Constants.floatingControlHorizontalOffset, y: -Constants.floatingControlVerticalOffset) .trackSize(size: $floatingSize) .accessibilitySortPriority(1) - .renderedIf(itemListContainerState != .initialLoading) + .renderedIf(itemListState != .initialLoading) POSConnectivityView() .frame(maxWidth: .infinity, maxHeight: .infinity, alignment: .top) @@ -71,7 +68,7 @@ struct PointOfSaleDashboardView: View { CGSizeMake(floatingSize.width + Constants.floatingControlHorizontalOffset, floatingSize.height + Constants.floatingControlVerticalOffset)) .environment(\.posBackgroundAppearance, posModel.paymentState != .processingPayment ? .primary : .secondary) - .animation(.easeInOut, value: itemListContainerState == .initialLoading) + .animation(.easeInOut, value: itemListState == .initialLoading) .animation(.easeInOut(duration: Constants.connectivityAnimationDuration), value: viewModel.showsConnectivityError) .background(Color.posPrimaryBackground) .navigationBarBackButtonHidden(true) @@ -93,16 +90,18 @@ struct PointOfSaleDashboardView: View { .sheet(isPresented: $showSupport) { supportForm } + .task { + await loadInitialItems() + } } private var contentView: some View { GeometryReader { geometry in HStack { if posModel.orderStage == .building { - ItemListView(itemListContainerState: $itemListContainerState, - itemsService: POSItemsService(itemProvider: itemProvider)) - .accessibilitySortPriority(2) - .transition(.move(edge: .leading)) + productListView + .accessibilitySortPriority(2) + .transition(.move(edge: .leading)) } if !posModel.paymentState.cardHasBeenTapped { @@ -167,7 +166,63 @@ private extension PointOfSaleDashboardView { } } +private extension PointOfSaleDashboardView { + @MainActor + func loadInitialItems() async { + do { + itemListState = .initialLoading + try await fetchItems(pageNumber: 1) + } catch { + itemListState = .error(PointOfSaleErrorState.errorOnLoadingProducts()) + } + } + @MainActor + func loadNextItems() async { + // TODO: Optimize API calls. gh-14186 + // If there are no more pages to fetch, we can avoid the next call. + let nextPage = currentPage + 1 + await loadItems(pageNumber: nextPage) + } + + @MainActor + func loadItems(pageNumber: Int) async { + do { + itemListState = .loading(allItems) + try await fetchItems(pageNumber: pageNumber) + } catch { + itemListState = .error(PointOfSaleErrorState.errorOnLoadingProducts()) + } + } + + @MainActor + func fetchItems(pageNumber: Int) async throws { + let newItems = try await itemsService.fetchItems(pageNumber: pageNumber) + let uniqueNewItems = newItems + .filter { newItem in + !allItems.contains(where: { $0.id == newItem.itemID }) + } + .compactMap(createPOSDisplayableItem(for:)) + + allItems.append(contentsOf: uniqueNewItems) + + if allItems.count == 0 { + itemListState = .empty + } else { + itemListState = .loaded(allItems) + } + } + + @MainActor + func reloadItems() async { + removeAllItems() + await loadItems(pageNumber: 1) + } + + func removeAllItems() { + allItems.removeAll() + } +} struct FloatingControlAreaSizeKey: EnvironmentKey { static let defaultValue = CGSize.zero @@ -190,6 +245,8 @@ private extension PointOfSaleDashboardView { static let exitPOSSheetMaxWidth: CGFloat = 900.0 static let supportTag = "origin:point-of-sale" static let connectivityAnimationDuration: CGFloat = 1.0 + + static let initialPage: Int = 1 } enum Localization { @@ -217,27 +274,11 @@ private extension PointOfSaleDashboardView { var totalsView: some View { TotalsView(viewModel: totalsViewModel, posModel: posModel) } -} -enum ItemListContainerViewState: Equatable { - case initializing - case initialLoading - case empty - case error(_ errorState: PointOfSaleErrorState, _ reloadItems: () async -> Void) - case show - - static func ==(lhs: ItemListContainerViewState, rhs: ItemListContainerViewState) -> Bool { - switch (lhs, rhs) { - case (.initializing, .initializing), - (.initialLoading, .initialLoading), - (.empty, .empty), - (.show, .show): - return true - case (.error(let lhsErrorState, _), .error(let rhsErrorState, _)): - return lhsErrorState == rhsErrorState - default: - return false - } + var productListView: some View { + ItemListView(itemListState: $itemListState, + loadNextItems: loadNextItems, + reloadItems: reloadItems) } } @@ -265,7 +306,7 @@ import class WooFoundation.MockAnalyticsProviderPreview totalsViewModel: totalsVM, cartViewModel: cartVM, posModel: posModel, - itemProvider: POSItemProviderPreview()) + itemsService: POSItemsService(itemProvider: POSItemProviderPreview())) } } #endif diff --git a/WooCommerce/Classes/POS/Presentation/PointOfSaleEntryPointView.swift b/WooCommerce/Classes/POS/Presentation/PointOfSaleEntryPointView.swift index 92474ad146a..6f1ff0ca0a8 100644 --- a/WooCommerce/Classes/POS/Presentation/PointOfSaleEntryPointView.swift +++ b/WooCommerce/Classes/POS/Presentation/PointOfSaleEntryPointView.swift @@ -50,7 +50,7 @@ struct PointOfSaleEntryPointView: View { totalsViewModel: totalsViewModel, cartViewModel: cartViewModel, posModel: posModel, - itemProvider: itemProvider) + itemsService: POSItemsService(itemProvider: itemProvider)) .environmentObject(posModalManager) .environmentObject(posModel) .onAppear { From 2013bd9da850c033a9aa9c811e36a184d36cb675 Mon Sep 17 00:00:00 2001 From: Josh Heald Date: Fri, 1 Nov 2024 15:41:06 +0000 Subject: [PATCH 4/4] Make itemsService stateless --- WooCommerce/Classes/POS/POSItemsService.swift | 12 +++++-- .../PointOfSaleDashboardView.swift | 33 +++++++------------ 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/WooCommerce/Classes/POS/POSItemsService.swift b/WooCommerce/Classes/POS/POSItemsService.swift index 3f87f294178..46ec7f9b226 100644 --- a/WooCommerce/Classes/POS/POSItemsService.swift +++ b/WooCommerce/Classes/POS/POSItemsService.swift @@ -11,12 +11,20 @@ struct POSItemsService { } @MainActor - func fetchItems(pageNumber: Int) async throws -> [any POSItem] { + func fetchItems(pageNumber: Int, + currentItems: [any POSDisplayableItem]) async throws -> [any POSDisplayableItem] { var newItems = try await itemProvider.providePointOfSaleItems(pageNumber: pageNumber) if pageNumber == 1 { newItems.insert(POSDiscount(), at: 0) } - return newItems + let uniqueNewItems = newItems + .filter { newItem in + !currentItems.contains(where: { $0.id == newItem.itemID }) + } + .compactMap(createPOSDisplayableItem(for:)) + + let allItems = currentItems + uniqueNewItems + return allItems } } diff --git a/WooCommerce/Classes/POS/Presentation/PointOfSaleDashboardView.swift b/WooCommerce/Classes/POS/Presentation/PointOfSaleDashboardView.swift index 8471a839158..7a1d39bb94c 100644 --- a/WooCommerce/Classes/POS/Presentation/PointOfSaleDashboardView.swift +++ b/WooCommerce/Classes/POS/Presentation/PointOfSaleDashboardView.swift @@ -12,7 +12,6 @@ struct PointOfSaleDashboardView: View { @State private var itemListState: PointOfSaleItemListState = .initialLoading private let itemsService: POSItemsService - @State private var allItems: [any POSDisplayableItem] = [] private var currentPage: Int = Constants.initialPage init(viewModel: PointOfSaleDashboardViewModel, @@ -171,7 +170,7 @@ private extension PointOfSaleDashboardView { func loadInitialItems() async { do { itemListState = .initialLoading - try await fetchItems(pageNumber: 1) + try await fetchItems(pageNumber: 1, currentItems: []) } catch { itemListState = .error(PointOfSaleErrorState.errorOnLoadingProducts()) } @@ -181,30 +180,27 @@ private extension PointOfSaleDashboardView { func loadNextItems() async { // TODO: Optimize API calls. gh-14186 // If there are no more pages to fetch, we can avoid the next call. + guard case .loaded(let currentItems) = itemListState else { + return + } let nextPage = currentPage + 1 - await loadItems(pageNumber: nextPage) + await loadItems(pageNumber: nextPage, currentItems: currentItems) } @MainActor - func loadItems(pageNumber: Int) async { + func loadItems(pageNumber: Int, currentItems: [any POSDisplayableItem]) async { do { - itemListState = .loading(allItems) - try await fetchItems(pageNumber: pageNumber) + itemListState = .loading(currentItems) + try await fetchItems(pageNumber: pageNumber, currentItems: currentItems) } catch { itemListState = .error(PointOfSaleErrorState.errorOnLoadingProducts()) } } @MainActor - func fetchItems(pageNumber: Int) async throws { - let newItems = try await itemsService.fetchItems(pageNumber: pageNumber) - let uniqueNewItems = newItems - .filter { newItem in - !allItems.contains(where: { $0.id == newItem.itemID }) - } - .compactMap(createPOSDisplayableItem(for:)) - - allItems.append(contentsOf: uniqueNewItems) + func fetchItems(pageNumber: Int, currentItems: [any POSDisplayableItem]) async throws { + let allItems = try await itemsService.fetchItems(pageNumber: pageNumber, + currentItems: currentItems) if allItems.count == 0 { itemListState = .empty @@ -215,12 +211,7 @@ private extension PointOfSaleDashboardView { @MainActor func reloadItems() async { - removeAllItems() - await loadItems(pageNumber: 1) - } - - func removeAllItems() { - allItems.removeAll() + await loadItems(pageNumber: 1, currentItems: []) } }