Woocommerce-ios: Edit Products: A better way to create product settings rows and bottom sheet actions

Created on 19 May 2020  路  4Comments  路  Source: woocommerce/woocommerce-ios

Right now, we just have a thin logic isVisible(product:) on ProductFormBottomSheetAction/ProductFormSection.SettingsRow to decide which settings rows / bottom sheet actions are to be displayed. It might be nice to have a factory that creates both sets of product editing actions since they are based on the same logic whenever the product is updated.

adedit products technical debt

Most helpful comment

Sorry I didn't get to this yesterday! 馃檱


So it's probably going to look like this:

My only concern is the SettingsRow is slightly complicated to create:

https://github.com/woocommerce/woocommerce-ios/blob/472adfa9709b6d1f36304c2d64c77cfe20680879/WooCommerce/Classes/ViewRelated/Products/Edit%20Product/DefaultProductFormTableViewModel.swift#L57-L63

And this may not belong in ProductFormActionsFactory.

But this feels like a good iteration. Let's try it. 馃檪

All 4 comments

Great! Not sure how the solution should look like. My initial not-so-well-informed idea is there can be an isolated data structure that defines the available actions. Then both the ProductFormBottomSheetAction and ProductFormSection can use that data structure.

I'd be happy to discuss some ideas if you'd like to. 馃憤

I was planning on taking this task today, but realized the ProductFormBottomSheetActionsFactory is only in release/4.3 branch 馃槵

my current thought for this task is pretty simple - in ProductFormViewController, we create a ProductFormActionsFactory whenever the product changes and the factory stays fixed for the same input parameters (product and feature flags). the factory not only has a method to return a list of bottom sheet actions (the current actions() -> [ProductFormBottomSheetAction]), but also a list of product form settings rows (like settingsRows() -> [ProductFormSection.SettingsRow]) since these two sets of actions are directly related.

so the action plan above is like:

  • Rename ProductFormBottomSheetActionsFactory to ProductFormActionsFactory
  • Rename the current actions() -> [ProductFormBottomSheetAction] to bottomSheetActions() -> [ProductFormBottomSheetAction]
  • Create a new function settingsRows() -> [ProductFormSection.SettingsRow]
  • In ProductFormViewController, add a private variable actionsFactory: ProductFormActionsFactory that is regenerated whenever the product changes. Whenever we need the settings rows or bottom sheet actions, we just ask the ProductFormActionsFactory

what do you think of the plan above? any other ideas are welcome as always!

release/4.3 was just merged to develop today! @shiki does the plan above sound good to you?

Sorry I didn't get to this yesterday! 馃檱


So it's probably going to look like this:

My only concern is the SettingsRow is slightly complicated to create:

https://github.com/woocommerce/woocommerce-ios/blob/472adfa9709b6d1f36304c2d64c77cfe20680879/WooCommerce/Classes/ViewRelated/Products/Edit%20Product/DefaultProductFormTableViewModel.swift#L57-L63

And this may not belong in ProductFormActionsFactory.

But this feels like a good iteration. Let's try it. 馃檪

Was this page helpful?
0 / 5 - 0 ratings