-
Notifications
You must be signed in to change notification settings - Fork 121
[Local Catalog] POS Settings: add local catalog sync settings UI #16125
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
Conversation
…her settings tabs.
|
|
joshheald
left a comment
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.
Looks good, thanks. A few small suggestions inline
|
|
||
| VStack(spacing: POSSpacing.medium) { | ||
| // TODO: WOOMOB-1100 - replace with catalog data | ||
| fieldRowView(label: Localization.catalogSize, value: "1,250 products, 3,420 variations (1.8 GB)") |
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.
1.8GB scared me for a moment. Might be best to either make a note in this view that this is fake data, or use something clearly impossible (500TB or something)...
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.
Yea, there's a TODO note above to replace the value, but after an initial look I don't think there's a straightforward way to calculate the catalog size for a site since the database could include data for more than the current site. I removed the storage space for now in 96feb4e, and I will add an issue to implement the storage space info if it's included in the final design.
| catalogStatusView | ||
| managingDataUsageView | ||
| manualCatalogUpdateView |
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.
| catalogStatusView | |
| managingDataUsageView | |
| manualCatalogUpdateView | |
| catalogStatus | |
| managingDataUsage | |
| manualCatalogUpdate |
They're not full views anyway, but even if they were, the word "view" seems unnecessary here
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.
Makes sense, renamed in 3c4cac2.
| private var backgroundColor: Color { | ||
| Color.posOnSecondaryContainer | ||
| } |
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.
Since this can't change, maybe it's better to make it a constant?
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.
Yup I agree, this was from following other sibling views in the settings. Updated in 3a24acb.
|
|
||
| static let sidebarNavigationLocalCatalogTitle = NSLocalizedString( | ||
| "pointOfSaleSettingsView.sidebarNavigationLocalCatalogTitle", | ||
| value: "Local catalog", |
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.
Perhaps just Catalog is enough? We don't use the word local inside the settings detail.
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.
Yea, the "local" part could be confusing for merchants. Updated in 62a78d7.
… get this information in implementation for now.

Part of WOOMOB-1100
Description
This PR adds the UI for POS local catalog sync settings with mock data behind the i1 feature flag. It introduces a new settings detail view that allows merchants to view & configure local catalog sync options for POS. The sync status integration requires more changes which will be included in a future PR.
The implementation includes:
POSSettingsLocalCatalogDetailViewthat provides the interface for catalog settingsPointOfSaleSettingsViewto display the local catalog settings option behind the local catalog i1 feature flagSteps to reproduce
Testing information
I tested that when the feature flag is disabled, the catalog settings isn't shown. Tested in iPad A18 iOS 18.4 simulator.
Screenshots
RELEASE-NOTES.txtif necessary.