[POS Settings] Hardware section navigation #16038
Conversation
Generated by 🚫 Danger |
|
|
| SafariView(url: WooConstants.URLs.inPersonPaymentsLearnMoreWCPay.asURL()) | ||
| } | ||
| .task { @MainActor in | ||
| // TODO: WOOMOB-1172 |
There was a problem hiding this comment.
This is temporary, most likely we'll fetch card reader information from passing in either a reference to the aggregate model or expose its card reader details.
There was a problem hiding this comment.
👍 Yes, the aggregate model works.
If we think that this state sharing in an aggregate model is not needed, and we only need information within this view, we could rely on a separate view model for this view that fetches information.
I had the same question for orders, and at least for order it felt like it made sense to have something separate and there was no value putting information within the aggregate model.
There was a problem hiding this comment.
Jinx! I just had the same question when reviewing your PR :D
I also think that in this case, for settings, it makes more sense to go through the aggregate, as we'll deal with settings across different features that are part of such aggregate, but we'll see what fits better 👍
| SafariView(url: WooConstants.URLs.inPersonPaymentsLearnMoreWCPay.asURL()) | ||
| } | ||
| .task { @MainActor in | ||
| // TODO: WOOMOB-1172 |
There was a problem hiding this comment.
👍 Yes, the aggregate model works.
If we think that this state sharing in an aggregate model is not needed, and we only need information within this view, we could rely on a separate view model for this view that fetches information.
I had the same question for orders, and at least for order it felt like it made sense to have something separate and there was no value putting information within the aggregate model.
|
|
||
| extension PointOfSaleSettingsView { | ||
| enum Constants { | ||
| static let sidebarWidthFraction: CGFloat = 0.35 |
There was a problem hiding this comment.
I think this is one of those we can share between Dashboard/Orders/Settings when we decide if we need a unified look and functionality.
| default: | ||
| EmptyView() | ||
| .frame(width: geometry.size.width * Constants.sidebarWidthFraction) | ||
| Group { |
There was a problem hiding this comment.
Wwe could avoid using Group if you would move the switch into a separate @ViewBuilder:
@ViewBuilder
private var detailView: some View {
switch selection {
case .store:
PointOfSaleSettingsStoreDetailView()
case .hardware:
PointOfSaleSettingsHardwareDetailView()
case .help:
PointOfSaleSettingsHelpDetailView()
default:
EmptyView()
}
}| cardReadersView | ||
| case .hardware(.scanners): | ||
| scannersView | ||
| case .scanner: |
There was a problem hiding this comment.
Could we remove scanner from the NavigationDestination then?
There was a problem hiding this comment.
Yes, I thought would be needed but it isn't since only buttons are there now. Updated: 9fdcf36
| enum NavigationDestination: Hashable { | ||
| case hardware(PointOfSaleSettingsView.HardwareDestination) | ||
| case scanner(ScannerDestination) | ||
| } |
There was a problem hiding this comment.
Could be removed
| enum NavigationDestination: Hashable { | |
| case hardware(PointOfSaleSettingsView.HardwareDestination) | |
| case scanner(ScannerDestination) | |
| } | |
| enum NavigationDestination: Hashable { | |
| case hardware(PointOfSaleSettingsView.HardwareDestination) | |
| } |
|
|
||
| struct PointOfSaleSettingsHardwareDetailView: View { | ||
| @State private var navigationPath: [PointOfSaleSettingsView.HardwareDestination] = [] | ||
| @State private var navigationPath: [NavigationDestination] = [] |
There was a problem hiding this comment.
In a broader conversation, I think the fact that "< Back" button is displayed within the details view, the "X" (Close) should be presented within the the list view.
And we likely need to use POSPageHeaderView and not the native navigation bar, and of course, not both. As I understand, we leave this for later 👍
There was a problem hiding this comment.
In a broader conversation, I think the fact that "< Back" button is displayed within the details view, the "X" (Close) should be presented within the the list view. And we likely need to use POSPageHeaderView and not the native navigation bar, and of course, not both. As I understand, we leave this for later 👍
Yes, that's something I'm precisely discussing today, as Android currently does not show a header across the full view (separates list and detail view), nor X to close the view that I can see. Neither this has been provided in the initial design drafts p1756257438170999?thread_ts=1755784099.956039&cid=C070SJRA8DP-slack-C070SJRA8DP
I think could even make sense to get the "Help" card up with the rest, and take that bottom-left position to go back to the dashboard, let's see what design thinks 👍
At the moment the scanner child view only has buttons that will prompt modal flow or web view, so no need to be part of the nav stack further down.
iamgabrielma
deleted the
task/WOOMOB-1039-pos-settings-hardware-section
branch
Closes WOOMOB-1039
Description
This PR adds the basic scaffolding and navigation for the Hardware section of POS settings, by allowing the merchant to navigate through card reader and barcode scanner setup in a detail view to the right, as well as see documentation full screen (for now) presented on top of the view.
Final UI is not handled yet, this will be done separately.
Screen.Recording.2025-08-26.at.10.14.20.mov
Testing information
...CTA, and go toSettingsHardware>Card Readers. Observe that you can open documentation full screen, and the last known reader model will appear if any.Hardware>Barcode scanner. Observe you can trigger the barcode scanner setup, as well as see the documentation.