Custom fields: Entry point logic in Product Details

[hafizrahman]

Closes: #14067

Please do not merge until base is trunk

Description

This PR adds various logic related to the Custom Fields entry point in Product Details.

1. The Custom Fields entry point in Product Details now actually opens to the Custom Fields list.
2. The Custom Fields entry point is now displayed for all type of products, not just Simple.
3. The Custom Fields entry point is added in the main settings area if the Product already has at least one Custom Field, or else it's put inside the "+ Add more details" bottom sheet action.
4. The Custom Fields entry point is now hidden during Product Creation, as adding them is not supported at this point.
5. The Custom Fields entry point now has a new icon.

Steps to reproduce

1. Prepare some products with custom fields added to them in the web,
2. Start app, then create a new product. Ensure that "Custom Fields" is not listed in the Product Details setting options.
3. Go back to the list of Product. Open a Product that has custom fields.
   • Ensure that "Custom Fields" is listed in the Product Details setting options.
   • Tap it, and ensure it opens the List screen, displaying the product's custom fields content.
4. Go back to the list of Product. Open a Product that has no custom fields.
   • Ensure that "Custom Fields" is not listed in the Product Details setting options
   • Ensure that "Custom Fields' is listed in "Add more details" bottom sheet
   • Tap it, and ensure it opens the List screen. It will show empty screen for now as Empty screen is still being worked on.
5. Smoke test different product types and ensure Custom Fields option appear either on the options or in the bottom sheet, depending on whether the product already has custom fields or not.

Testing information

I tested this in Simulator with iOS 17.5, with all different product types generated by Smooth Generator.

Screenshots

Existing product with custom fields	Existing product with no custom fields

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

• The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
• Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
• Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on all devices (phone/tablet) and no regressions are added.

[wpmobilebot]

📲 You can test the changes from this Pull Request in WooCommerce iOS by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce iOS
	Build Number	pr14068-fbcabcd
	Version	20.6
	Bundle ID	com.automattic.alpha.woocommerce
	Commit	fbcabcd
	App Center Build	WooCommerce - Prototype Builds #11096

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

[itsmeichigo]

I tested on simulator iPhone 15 Pro iOS 17.4 and confirmed that custom fields are available when editing and viewing readonly products of all types. I wonder if we should keep the custom field list non-editable if a product is readonly?

The navigation to the custom field list is not smooth due to the switch of the toolbar. I left a comment with more details and suggestions below.

[itsmeichigo]

The animation causes unwanted jiggling between transitions, so setting animated to false might help.

For a smoother experience, please consider adding a separate type CustomFieldsListHostingController and set the navigation items for the Save and Add buttons in that controller if it's editable. That way you can reuse the back button and avoid having a thinner chevron icon on this screen compared to the product form screen.

[hafizrahman]

Thanks, Paolo also raised something similar here #14059 (review) and I'll quote my reply:

I agree with that, though, and in hindsight it would've been more consistent to copy Blaze's method of creating navigation coordinator (BlazeCampaignCreationCoordinator) and use UIKit navigation still. While I do find that the experience of setting up navigation using SwiftUI's NavigationStack to be more intuitive and concise, it's also unfortunate that connecting it with UIKit's navigation isn't as smooth as possible.

An improvement here would be to refactor the navigation to use coordinator. Given what's already set up, this would be a decently sized task, so let's keep this for now. In the meanwhile I'll add a bit more things like turning off animation and making the back icon bolder, to make the back navigation look less odd.

[hafizrahman]

I added cfd6c47 to:

• remove animations
• make the back button thicker (good catch on this! This is also done in other parts of the code that uses the chevron)

[hafizrahman]

Something I'm not able to fix is that the placement of the back arrow has an extra leading padding in the list view, compared to the back button on the product details view or custom fields editor view:

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-10-01.at.12.23.03.mp4

I tried a few things:

• adding negative padding: this works but make the target area smaller and taps don't register on the left side
• using .topBarLeading instead of .cancellationAction as placement
• using ToolbarItemGroup

But none of them fixes the problem. It's odd because the back button generated automatically by SwiftUI (in the editor view) has the correct placement similar to UIKit's back button (in the product details view). It's just the the back button created with Toolbar that has a different leading padding. This feels like a SwiftUI bug to me, and as it's a bit odd but not breaking functionality, I think we can keep it like that for now.

[hafizrahman]

@itsmeichigo This is good for another review.

Thanks especially for your mention about readonly mode. I've tested it by finding an Order and refunding it, then going to the refund details and tapping the product. Whether the product has custom fields or not, the custom fields row was not shown in the readonly-mode product details. Please also test this use case 🙏🏼

[itsmeichigo]

Thanks for the updates Hafiz. I left a few comments following up with your latest changes below.

Regarding this response:

I agree with that, though, and in hindsight it would've been more consistent to copy Blaze's method of creating navigation coordinator (BlazeCampaignCreationCoordinator) and use UIKit navigation still. While I do find that the experience of setting up navigation using SwiftUI's NavigationStack to be more intuitive and concise, it's also unfortunate that connecting it with UIKit's navigation isn't as smooth as possible.

Can you explain why creating a coordinator would be relevant here? If you look closely at the BlazeCampaignCreationCoordinator, the navigateToNativeCampaignCreation method includes navigating to BlazeCampaignCreationFormHostingController, which is also what I suggested above to create a hosting controller and let it manage the navigation items instead of using SwiftUI toolbar.

[itsmeichigo]

I hope there's already a subtask for this, but I think we should display the custom field keys if the custom fields are available in the subtitle here. I don't think displaying "Custom fields" twice would be helpful.

[hafizrahman]

Can you explain why creating a coordinator would be relevant here?

I think we're talking about the same thing here but I didn't make it clear. I am aware that BlazeCampaignCreationCoordinator uses hosting controller. The thing is that the navigation to update here is not just for the navigation into the fields list, but also between fields list and field editor. So if I'm making hosting controller for the fields list, I'll need to do the same for the editor view, and at that point having a coordinator for all the custom fields context makes sense to me.

[dangermattic]

	1 Warning
⚠️	This PR is assigned to the milestone 20.8. This milestone is due in less than 2 days. Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

[hafizrahman]

I think we should display the custom field keys if the custom fields are available in the subtitle here. I don't think displaying "Custom fields" twice would be helpful.

Thanks for catching that. The duplicate writing was a bug that I fixed in aa0ccd5

The behavior should now match Android (see screenshots in woocommerce/woocommerce-android#12682 )

We discussed about what to show in the subtitle here p1726234051128759/1726231756.826709-slack-C03L1NF1EA3 and for now ended up deciding to just show a fixed string "View and edit ..." . Thanks for the feedback, though, I'll put it in for further discussion and future iteration.

[hafizrahman]

Good for another review @itsmeichigo . thank you!

[itsmeichigo]

Thanks again for the updates Hafiz!

The thing is that the navigation to update here is not just for the navigation into the fields list, but also between fields list and field editor. So if I'm making hosting controller for the fields list, I'll need to do the same for the editor view, and at that point having a coordinator for all the custom fields context makes sense to me.

I see your point. I think it would be simpler to only make the hosting controller for the field list, and present the editor view modally. The custom field editing feels like a separate flow so I think it makes sense to present it as a modal.

Please give my suggestion some consideration - I'm approving this PR anyway to avoid blocking development for too long.

[wpmobilebot]

Version 20.7 has now entered code-freeze, so the milestone of this PR has been updated to 20.8.

[hafizrahman]

The custom field editing feels like a separate flow so I think it makes sense to present it as a modal.

This is a good point and I see another flow in Order Details that's exactly like that. I'll work on this on a separate PR as I need to consolidate other work related to saving flow first, just to avoid conflicts.