Domain settings: feature flag, entry point from settings, barebone UI #8581

jaclync · 2023-01-09T02:56:13Z

Part of #8558

Description

Why

After the user creates a store, they might want to choose a proper domain for their new store. This is part of the onboarding plan for the future, until then we're showing the entry point in Menu tab > Settings > Store settings.

Implementation

This is the first PR of the domain settings, with a feature flag, an entry point in the settings, and a basic UI with mock data. The entry point is only shown when the feature is enabled and the site is a WPCOM site. Some barebone data models in the Networking layer were created to streamline the UI integration, and the Yosemite actions just return mock data for now.

Data layer

Two endpoints are required to show the UI on this screen:

DomainAction.loadDomains: this loads the site's domains, including the free staging domain
PaymentAction.loadSiteCurrentPlan: this loads the site's WPCOM plans, and returns the current plan to determine if the site has domain credit so that the user can pick a domain for free (available from certain WPCOM plans)

UI

The main SwiftUI view is DomainSettingsView, which shows the free *.wpcomstaging.com domain at the top with FreeStagingDomainView. Below the free domain, different content can be shown with the following logic:

If the site has a domain credit, a domain credit section is shown for the user to pick a domain for free using their domain credit (not implemented in this PR)
If the site has existing domains other than the free staging domain, a list of domains is shown. Otherwise, a CTA Search for a domain is shown (the CTA is implemented in this PR without the details text below)

Testing instructions

Prerequisite: a WC store with a WPCOM plan is required

Log in if needed, and continue with a WPCOM store
Go to the Menu tab
Tap on the settings CTA --> there should be a Domain row under the store settings section
Tap the Domain row --> a modal should be shown with a free staging domain at the top (mock data) and a CTA at the bottom

Screenshots

light	dark

I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

* trunk: (38 commits) Hide card present payments related actions for countries where IPP is not supported. Fix filename in comment. EntityIDMapper - Add unit tests. Rename error for clarity. EntityIDMapper - Handle JSON with/without data envelope. ProductIDMapper - Add unit tests. ProductIDMapper - Handle JSON with/without data envelope. Product IDs - Add mock json without data envelope. Rename type alias. `ProductSkuMapperTests` - Test mock json file without data envelope. `ProductSkuMapper` - Handle JSON with/without data envelope. Add product sku mock JSON without data envelope. Update unit tests to use a for loop to handle array of array. Parse both JSON and return an array of array. Introduce parsing error. Make ProductListMapper parse JSON with/without data envelope. Add mock product list JSON without data envelope. Update unit tests to test an array of products. Make `mapLoadProductResponse` return an array of parsed products from with/without data mock JSON files. Add an error for parsing failure. ... # Conflicts: # WooCommerce/WooCommerce.xcodeproj/project.pbxproj

wpmobilebot · 2023-01-09T03:11:27Z

You can test the changes from this Pull Request by:

Clicking here or scanning the QR code below to access App Center
Then installing the build number pr8581-d3f71c6 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

itsmeichigo

This works as expected ✅

itsmeichigo · 2023-01-09T05:44:14Z

WooCommerce/Classes/ViewRelated/Dashboard/Settings/Domains/DomainSettingsView.swift

+                    // TODO: 8558 - show domain list with search domain action
+                }
+            }
+            .padding(.init(top: 39, leading: 16, bottom: 16, trailing: 16))


Nit: should we make these numbers constants?

I feel like it's more useful making layout constants for shared styles within the view or reusable components, while these one-off constants are just from the design for this particular screen. Do you feel like having these layout constants helps with readability and future maintenance? If so I can make constants for these, would just like to hear more thoughts.

I'm under the impression that our convention has been to keep layout measurements at one place for quickly adjustments when needed. I don't have a strong opinion on this, just want to make sure that we are consistent with other parts of the app 😅

I see, I didn't do enough SwiftUI to know that we have this convention. I'll ask the team later but just moved them to constants in any case d3f71c6.

itsmeichigo · 2023-01-09T05:44:38Z

WooCommerce/Classes/ViewRelated/Dashboard/Settings/Domains/DomainSettingsView.swift

+
+    var body: some View {
+        ScrollView {
+            VStack(alignment: .leading, spacing: 36) {


Nit: should we make this number constant as well?

itsmeichigo · 2023-01-09T05:46:02Z

WooCommerce/Classes/ViewRelated/Dashboard/Settings/Domains/DomainSettingsView.swift

+            }
+        }
+        .navigationBarTitle(Localization.title)
+        .navigationBarTitleDisplayMode(.large)


❓ Do we need to do something to make this title appears large? Currently, it looks like the default style.

My bad, this screen isn't meant to have a large title based on the latest design VyLr7LvKodHE4kINfBE7Lw-fi-682%3A40227&t=gKEdaIp9JraK2sED-0. Updated in c2c3e9b.

To answer the question about why the large title isn't working: it's because the SwiftUI view is embedded in a UIHostingController, and large titles need to be enabled for its navigation controller's navigation bar like navigationBar.prefersLargeTitles = true.

itsmeichigo · 2023-01-09T05:47:55Z

WooCommerce/Classes/ViewRelated/Dashboard/Settings/Domains/DomainSettingsView.swift

+            if viewModel.domains.isEmpty {
+                VStack {
+                    Divider()
+                        .frame(height: Layout.dividerHeight)


❓Non-blocking: do we really need to set a height or the divider? I usually keep the frame as-is, as I think the default style works well.

The default height isn't mentioned in the official documentation 🤔 I feel like it's safer setting a height just in case the default behavior changes in future iOS versions so that there are no surprises.

itsmeichigo · 2023-01-09T05:53:10Z

WooCommerce/Classes/ViewRelated/Dashboard/Settings/Domains/FreeStagingDomainView.swift

+    let domain: DomainSettingsViewModel.FreeStagingDomain
+
+    var body: some View {
+        VStack(alignment: .leading, spacing: 8) {


Nit: should we make this number a constant too?

jaclync added 9 commits January 5, 2023 21:33

Create a feature flag for domain settings.

8ccab35

Create domain settings views with mock domains data source.

cd69f9f

Add PaymentAction.loadSiteCurrentPlan to load a site's current plan.

971b570

Show domain settings from settings > store settings.

e65dae1

Move DomainSettingsViewModel to a separate file. Add comments.

15cd478

Remove networking layer details.

1766c0d

Add comments to FreeStagingDomainView.

7ee526a

Add a test case on SettingsViewModel when a site is not a wpcom site.

0bda1c3

jaclync added type: task An internally driven task. status: feature-flagged Behind a feature flag. Milestone is not strongly held. labels Jan 9, 2023

jaclync added this to the 11.9 milestone Jan 9, 2023

jaclync requested review from itsmeichigo and selanthiraiyan January 9, 2023 02:56

itsmeichigo self-assigned this Jan 9, 2023

itsmeichigo approved these changes Jan 9, 2023

View reviewed changes

jaclync added 2 commits January 9, 2023 15:57

Disable large title for the domain settings view.

c2c3e9b

Move layout numbers to constants from CR suggestions.

d3f71c6

jaclync enabled auto-merge January 9, 2023 13:55

jaclync merged commit 11bf8fc into trunk Jan 10, 2023

jaclync deleted the feat/8558-domain-settings branch January 10, 2023 01:19

jaclync mentioned this pull request Jan 10, 2023

Settings: domain update #8558

Open

35 tasks

Domain settings: feature flag, entry point from settings, barebone UI #8581

Domain settings: feature flag, entry point from settings, barebone UI #8581

Uh oh!

Conversation

jaclync commented Jan 9, 2023

Description

Why

Implementation

Data layer

UI

Testing instructions

Screenshots

Uh oh!

wpmobilebot commented Jan 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

itsmeichigo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

itsmeichigo Jan 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

wpmobilebot commented Jan 9, 2023 •

edited

Loading

itsmeichigo Jan 9, 2023 •

edited

Loading