-
Notifications
You must be signed in to change notification settings - Fork 121
[POS Settings] Add two-panel settings view #16021
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 9 commits
770cc31
f93dada
239a39b
a2cd1a4
0d3d6bd
ceb4e1b
96edc8e
738ab10
f89e695
d818027
71ade8c
86fe351
cf5d8c7
78b3836
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import SwiftUI | ||
|
|
||
| struct PointOfSaleSettingsHardwareDetailView: View { | ||
| @Environment(\.dismiss) private var dismiss | ||
| @State private var navigationPath: [PointOfSaleSettingsView.HardwareDestination] = [] | ||
|
|
||
| var body: some View { | ||
| NavigationStack(path: $navigationPath) { | ||
| List(PointOfSaleSettingsView.HardwareDestination.allCases) { destination in | ||
| NavigationLink(value: destination) { | ||
| HStack(alignment: .firstTextBaseline) { | ||
| Image(systemName: destination.icon) | ||
| .font(.posBodyLargeRegular()) | ||
| VStack(alignment: .leading, spacing: POSPadding.xSmall) { | ||
| Text(destination.title) | ||
| .font(.posBodyLargeRegular()) | ||
| Text(destination.subtitle) | ||
| .font(.posBodyMediumRegular()) | ||
| .foregroundStyle(.secondary) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| .toolbar { | ||
| ToolbarItem(placement: .topBarTrailing) { | ||
| Button("Done") { dismiss() } | ||
| } | ||
| } | ||
| .navigationDestination(for: PointOfSaleSettingsView.HardwareDestination.self) { destination in | ||
| VStack(spacing: POSSpacing.medium) { | ||
| Image(systemName: destination.icon).font(.largeTitle) | ||
| .font(.posBodyLargeRegular()) | ||
| Text("\(destination.title) settings") | ||
| .font(.posBodyMediumRegular()) | ||
| Text("Some placeholder") | ||
| .font(.posBodyMediumRegular()) | ||
| .foregroundStyle(.secondary) | ||
| } | ||
| .padding() | ||
| .navigationTitle(destination.title) | ||
| } | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import SwiftUI | ||
|
|
||
| struct PointOfSaleSettingsHelpDetailView: View { | ||
| @Environment(\.dismiss) private var dismiss | ||
|
|
||
| var body: some View { | ||
| NavigationStack { | ||
| VStack(alignment: .leading) { | ||
| Text("Help Settings") | ||
| .font(.title2) | ||
| Text("Help-related configuration") | ||
| .font(.caption) | ||
| .foregroundStyle(.secondary) | ||
| } | ||
| .padding() | ||
| .toolbar { | ||
| ToolbarItem(placement: .topBarTrailing) { | ||
| Button("Done") { dismiss() } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import SwiftUI | ||
|
|
||
| struct PointOfSaleSettingsStoreDetailView: View { | ||
| @Environment(\.dismiss) private var dismiss | ||
|
|
||
| var body: some View { | ||
| NavigationStack { | ||
| VStack(alignment: .leading) { | ||
| Text("Store Settings") | ||
| .font(.title2) | ||
| Text("Store-related configuration") | ||
| .font(.caption) | ||
| .foregroundStyle(.secondary) | ||
| } | ||
| .padding() | ||
| .toolbar { | ||
| ToolbarItem(placement: .topBarTrailing) { | ||
| Button("Done") { dismiss() } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,213 @@ | ||
| import SwiftUI | ||
|
|
||
| struct PointOfSaleSettingsView: View { | ||
| @State private var selection: SidebarNavigation? = .store | ||
|
|
||
| var body: some View { | ||
| HStack(spacing: POSSpacing.none) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this should be 50/50 split...
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is definitely not 😂 , already addressed on #16038 |
||
| VStack { | ||
| List(selection: $selection) { | ||
| Section { | ||
| ForEach([SidebarNavigation.store, SidebarNavigation.hardware], id: \.self) { item in | ||
| HStack { | ||
| Image(systemName: item.icon) | ||
| .font(.posBodyLargeRegular()) | ||
| VStack(alignment: .leading) { | ||
| Text(item.title) | ||
| .font(.posBodyLargeRegular()) | ||
| Text(item.subtitle) | ||
| .font(.posBodyMediumRegular()) | ||
| .foregroundStyle(.secondary) | ||
| } | ||
| } | ||
| .tag(item) | ||
| } | ||
| } | ||
| } | ||
| .safeAreaInset(edge: .bottom) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wasn't able to pin the We do not necessarily need a list here and perhaps creating just a reusable "card" would work fine for the 3 items. If we do, this can be iterated on a separate PR. Thoughts?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, if a list makes it harder, let's just make our own cards. Also, this might move the help button above an on-screen keyboard, which would be a bit strange. It's likely that we'll end up presenting one in settings eventually. |
||
| Button { | ||
| selection = .help | ||
| } label: { | ||
| HStack { | ||
| Image(systemName: SidebarNavigation.help.icon) | ||
| .font(.posBodyLargeRegular()) | ||
| .foregroundStyle(selection == .help ? .white : .primary) | ||
| VStack(alignment: .leading) { | ||
| Text(SidebarNavigation.help.title) | ||
| .font(.posBodyLargeRegular()) | ||
| .foregroundStyle(selection == .help ? .white : .primary) | ||
| Text(SidebarNavigation.help.subtitle) | ||
| .font(.posBodyMediumRegular()) | ||
| .foregroundStyle(selection == .help ? .secondary : .secondary) | ||
| } | ||
| Spacer() | ||
| } | ||
| .padding(.vertical, POSPadding.small) | ||
| .padding(.horizontal, POSPadding.medium) | ||
| .contentShape(Rectangle()) | ||
| } | ||
| .buttonStyle(.plain) | ||
| .background( | ||
| RoundedRectangle(cornerRadius: POSCornerRadiusStyle.large.value, style: .continuous) | ||
| .fill(selection == .help ? Color.accentColor : Color.clear) | ||
| ) | ||
|
Comment on lines
+60
to
+63
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The selected version looks pretty odd, being purple while the other selected items go grey. Having rounded corners doesn't work well with the padding we have here either – which is closer to the screen edges because it's outside the list, but if we keep it this way then square corners would make more sense. I know UI's not final so no need to fix here, but thought I'd mention it. |
||
| } | ||
| } | ||
| Group { | ||
| switch selection { | ||
| case .store: | ||
| PointOfSaleSettingsStoreDetailView() | ||
| case .hardware: | ||
| PointOfSaleSettingsHardwareDetailView() | ||
| case .help: | ||
| PointOfSaleSettingsHelpDetailView() | ||
| default: | ||
| EmptyView() | ||
| } | ||
| } | ||
| .frame(maxWidth: .infinity, maxHeight: .infinity) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| extension PointOfSaleSettingsView { | ||
| enum HardwareDestination: Identifiable, CaseIterable { | ||
| case cardReaders | ||
| case scanners | ||
|
|
||
| var id: Self { self } | ||
|
|
||
| var title: String { | ||
| switch self { | ||
| case .cardReaders: | ||
| return Localization.hardwareNavigationCardReaderTitle | ||
| case .scanners: | ||
| return Localization.hardwareNavigationBarcodeTitle | ||
| } | ||
| } | ||
|
|
||
| var subtitle: String { | ||
| switch self { | ||
| case .cardReaders: | ||
| return Localization.hardwareNavigationBarcodeSubtitle | ||
| case .scanners: | ||
| return Localization.hardwareNavigationBarcodeSubtitle | ||
| } | ||
| } | ||
|
|
||
| var icon: String { | ||
| switch self { | ||
| case .cardReaders: | ||
| return "creditcard" | ||
| case .scanners: | ||
| return "qrcode.viewfinder" | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private extension PointOfSaleSettingsView { | ||
| enum SidebarNavigation: String, CaseIterable, Identifiable { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be nicer to define a protocol for settings items instead, then each can be separately defined closer to their content? Not sure about this, just something to consider. |
||
| case store | ||
| case hardware | ||
| case help | ||
|
|
||
| var id: Self { self } | ||
|
|
||
| var title: String { | ||
| switch self { | ||
| case .store: return Localization.sidebarNavigationStoreTitle | ||
| case .hardware: return Localization.sidebarNavigationHardwareTitle | ||
| case .help: return Localization.sidebarNavigationHelpTitle | ||
| } | ||
| } | ||
|
|
||
| var subtitle: String { | ||
| switch self { | ||
| case .store: return Localization.sidebarNavigationStoreSubtitle | ||
| case .hardware: return Localization.sidebarNavigationHardwareSubtitle | ||
| case .help: return Localization.sidebarNavigationHelpSubtitle | ||
| } | ||
| } | ||
|
|
||
| var icon: String { | ||
| switch self { | ||
| case .store: return "bag" | ||
| case .hardware: return "wrench.and.screwdriver" | ||
| case .help: return "questionmark.circle" | ||
| } | ||
| } | ||
| } | ||
|
|
||
| enum Localization { | ||
| static let navigationTitle = NSLocalizedString( | ||
| "pointOfSaleSettingsView.navigationTitle", | ||
| value: "Settings", | ||
| comment: "Title of the Point of Sale settings view." | ||
| ) | ||
|
|
||
| static let sidebarNavigationStoreTitle = NSLocalizedString( | ||
| "pointOfSaleSettingsView.sidebarNavigationStoreTitle", | ||
| value: "Store", | ||
| comment: "Title of the Store section within Point of Sale settings." | ||
| ) | ||
|
|
||
| static let sidebarNavigationHardwareTitle = NSLocalizedString( | ||
| "pointOfSaleSettingsView.sidebarNavigationHardwareTitle", | ||
| value: "Hardware", | ||
| comment: "Title of the Hardware section within Point of Sale settings." | ||
| ) | ||
|
|
||
| static let sidebarNavigationHelpTitle = NSLocalizedString( | ||
| "pointOfSaleSettingsView.sidebarNavigationHelpTitle", | ||
| value: "Help", | ||
| comment: "Title of the Help section within Point of Sale settings." | ||
| ) | ||
|
|
||
| static let sidebarNavigationStoreSubtitle = NSLocalizedString( | ||
| "pointOfSaleSettingsView.sidebarNavigationStoreSubtitle", | ||
| value: "Store configuration and settings", | ||
| comment: "Description of the settings to be found within the Store section." | ||
| ) | ||
|
|
||
| static let sidebarNavigationHardwareSubtitle = NSLocalizedString( | ||
| "pointOfSaleSettingsView.sidebarNavigationHardwareSubtitle", | ||
| value: "Manage hardware connections", | ||
| comment: "Description of the settings to be found within the Hardware section." | ||
| ) | ||
|
|
||
| static let sidebarNavigationHelpSubtitle = NSLocalizedString( | ||
| "pointOfSaleSettingsView.sidebarNavigationHelpSubtitle", | ||
| value: "Get help and support", | ||
| comment: "Description of the Help section in Point of Sale settings." | ||
| ) | ||
|
|
||
| static let hardwareNavigationBarcodeTitle = NSLocalizedString( | ||
| "pointOfSaleSettingsView.hardwareNavigationBarcodeTitle", | ||
| value: "Barcode scanners", | ||
| comment: "Navigation title of Barcode scanner settings." | ||
| ) | ||
|
|
||
| static let hardwareNavigationCardReaderTitle = NSLocalizedString( | ||
| "pointOfSaleSettingsView.hardwareNavigationCardReaderTitle", | ||
| value: "Card readers", | ||
| comment: "Navigation title of Card reader settings." | ||
| ) | ||
|
|
||
| static let hardwareNavigationCardReaderSubtitle = NSLocalizedString( | ||
| "pointOfSaleSettingsView.hardwareNavigationCardReaderSubtitle", | ||
| value: "Manage card reader connections", | ||
| comment: "Description of Card reader settings for connections." | ||
| ) | ||
|
|
||
| static let hardwareNavigationBarcodeSubtitle = NSLocalizedString( | ||
| "pointOfSaleSettingsView.hardwareNavigationBarcodeSubtitle", | ||
| value: "Configure barcode scanner settings", | ||
| comment: "Description of Barcode scanner settings configuration." | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| #Preview { | ||
| PointOfSaleSettingsView() | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's a
NavigationStackgotcha: I had to add the.toolbarfor the dismiss action to all destination views, rather than once to the parent Settings view, if we do, then these destinations will be presented full-screen as part of the stack, rather than encapsulated to the right-side only.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An alternative that seems to work is to dismiss theno longer needed since using aPointOfSaleSettingsViewthrough a viewmodifier attached to the parent view instead, so we could remove the.toolbarapproach from the subviews. For testing I used.posModalCloseButton(action: { dismiss() })since already exists, and seemed to work fine.POSPageHeaderView: 86fe351