Adds "Draft" label below the image on the product editing screen

[iamgabrielma]

Fixes #4544

Description

When editing a draft product we want to show a “Draft” label below the product image for consistency with the Android app:

Changes

Draft products status	Other product status

Landscape View

Example of LTR screen & Localized strings

I've added a new UILabel to ProductImagesHeaderTableViewCell.xib via the Interface Builder . Then within the ProductFormTableViewDataSource I'm performing a check to see if the product status is a draft, so this label will only be displayed if the product status is set to draft, no label will be shown otherwise.

I used existing styles that we use elsewhere in order to apply some sort of design, in this case using the same style as completed orders via applyStyle(for: .completed) seemed appropriate as fits the the style shown in the Android version of the app.

Work in progress:

• [x] In order to fit as close as possible the Android one, I need to style it further by adding inner padding and changing the text colour.
• [x] Move the new label cell configuration outside the Data Source.
• [x] Fix UI constraints
• [x] Test for localized strings
• [x] Test for dark mode
• [x] Move the .cell appearance-only config to be called from awakeFromNib() instead, as these do not depend on data updates.

Testing steps

• Create a new product, or edit any existing product.
• If the product is currently a draft , the draft label will appear below the image cell.
• In the top right corner, tap on "..." > Product Settings > Status
• Switching between different status will make the label appear only when draft is selected, and disappear on other statuses.

Update release notes:

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

[peril-woocommerce[bot]]

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

[peril-woocommerce[bot]]

You can trigger an installable build for these changes by visiting CircleCI here.

[iamgabrielma]

With the latest commit the label is now more similar to its Android version (white text & grey background):

[iamgabrielma]

It seems the holder is using autoresizing masks instead of layout constraints, which seems to work initially, but it won't adapt to a different size when using accessibility font sizes or when the text is localized: It also doesn't look right on dark mode, but I'm not sure what the right colors should be for that, so I'd ping a designer for a UI review as well. For the layout, you probably want to set constraints to pin the view to the bottom leading edge (perhaps with some offset), but then let the holder grow as the text gets larger. For that, the label should use one of the dynamic fonts. Again, not sure of which one since I can't see this label in the design files, so maybe @woocommerce/mobile-designers can help with the exact font/color/dimensions here.

Thanks for the pointers! With the latest commit I believe to have resolved this by setting layout constraints indeed, rather than the existing autoresizing mask, for now I haven't modified the colours in dark mode as seems we're not modifying them neither in other places like Order statuses, however is true that the current grey/white combination is not great with a black background.

I'll give it a few more tests and wait for @woocommerce/mobile-designers feedback on styling the labels before requesting a new review

English label	Localized Label	Dark mode

[Garance91540]

Hi! @adamzelinski do you have some time to provide some designs for this issue this week?

[iamgabrielma]

Light mode	Dark mode	Dark mode with no images

Latest changes:

• Moved the cell configs that do not deal directly with data changes to its own function to be called from awakeFromNib()
• Fixed label and text constraints to fit localized strings and to look good in dark mode
• Applied design recommendations and now follows the "Order Status" pill design except for the positioning: While I adjusted it to the bottom of the view, is not fully under the image for various reasons:
  • It would involve modify the whole style of ProductImagesHeaderTableViewCell as well as push both the images to the top and the rest of the Order details to the bottom. This causes visibility problems on smaller devices and is inconsistent when the product is not a Draft, as the space reserved for this label is not used.
  • For consistency with the Android version, where the pill covers part of the image on purpose.

Happy to make any further modifications!

[ghost]

Hi, @iamgabrielma here is the design for adding the draft tag. Can we please just use the current status pills we use next to the Product name.

Thanks!

[iamgabrielma]

Thanks for the pointers @adamzelinski ! I have a question before proceeding: Would we consider to use an icon rather than a label with text?

While checking the current textfield that is being used for the product text I realized that there's already some code in place that is not being used and its purpose is precisely to show an icon when necessary.

This is the current structure of the field:

If we add the label with a text, this is how it would be:

By using one of Apple's Human Interface Guideline icons here rather than label and text we could avoid the clutter as well as potential problems like the length of a localized text overlapping or pushing the product name.

This is roughly how it would look like ( I used the .square.and.pencil icon, and applied a generic grey just for testing ):

Let me know if that would make sense, or if we should stick to the original idea of using a label & text.

[ghost]

Hey @iamgabrielma, I think we should just use the label instead of an icon for now as it's an existing component we use to display the status of something. Definitely, something we will keep in mind though so thanks for looking into this :)

[iamgabrielma]

I'm moving this PR back to draft for the moment, as the latest changes require to redo it from zero 😅

At this stage I had to choices for the new implementation: To create a new cell, or reuse an existing one. TitleAndTextFieldTableViewCell seems to adapt well however this cell is already being used across the app and I can't modify it without affecting other places, so I opted to create a new cell for this: LabeledTextViewTableViewCell based on this one.

TODO:

• [x] Revert changes at ProductImagesHeaderTableViewCell.swift , ProductImagesHeaderTableViewCell.xib, TitleAndTextFieldTableViewCell.xib
• [x] Editing does not trigger onTextChange
• [x] Fix first responder when the new cell is tapped.
• [x] Layout should adapt to the screen when there's no label.
• [x] Fix cell styling consistency, and label autolayout with previous implementation.

[iamgabrielma]

Alright, I believe this should be good (or pretty close) now:

• In DefaultProductFormTableViewModel now we're passing the product status inside the primaryFieldRows when generating the .name row.
• This returns a new prototype cell LabeledTextViewTableViewCell rather than the previous TextViewTableViewCell
• This new cell contains a UILabel and a textfield ( using EnhancedTextView ). The label will display our product status if is set to draft, and nothing otherwise.

Draft Product	Draft Product (Dark Mode)	Localized	No label

Let me know if needs further iterations 👍

[ghost]

@iamgabrielma looking good so far thanks! Just noticed a few little things to tweak.

[koke]

Also, I'm seeing some auto layout errors:

2021-08-31 09:27:22.362020+0200 WooCommerce[1094:716536] [LayoutConstraints] Unable to simultaneously satisfy constraints.
    Probably at least one of the constraints in the following list is one you don't want. 
    Try this: 
        (1) look at each constraint and try to figure out which you don't expect; 
        (2) find the code that added the unwanted constraint or constraints and fix it. 
(
    "<NSLayoutConstraint:0x282189180 H:[UIView:0x11e5b5a90]-(5)-[WooCommerce.EnhancedTextView:0x10c93c000'Wapuu Plush Toy']   (active)>",
    "<NSLayoutConstraint:0x28218a080 'UISV-spacing' H:[UIView:0x11e5b5a90]-(42)-[WooCommerce.EnhancedTextView:0x10c93c000'Wapuu Plush Toy']   (active)>"
)

It looks like there is a manual constraint for the spacing between the status holder and the product name (5), but the stack view that contains them would already manage the spacing between children, so there's a conflict there. You can remove that constraint and adjust the stack view's spacing to 5 to achieve the same effect.

[iamgabrielma]

Changes:

• Fixed the missing background colour in dark mode
• Reduced the label size to make it similar to the order status labels
• Removed the top divider. This is actually the bottom divider from the ProductImagesHeaderTableViewCell, I also added more space to the bottom of the new cell so looks it looks centered and aligned correctly after removing the divider:

• Reverted noise for ProductImagesHeaderTableViewCell.xib, ProductFormViewController.xib, and TitleAndTextFieldTableViewCell.xib to their own respective previous commits.
• Refactored productLabel to productStatus
• Changed NSLocalizedString to accept the String literal
• Fixed auto-layout errors by removing constraints from the child elements and just setting them on the StackView.
• Fixed label holder appearance logic. I have not used a isHidden check here as there's an open issue to potentially add other states to these labels: https://github.com/woocommerce/woocommerce-android/issues/4587

[iamgabrielma]

Latest changes:

• I set to none the cell's selection style, so the row is no longer highlighted when we tap outside the product field.
• I added applyStyle(style: viewModel.style) so the .headline style for product title is applied properly:

• For the alignment between the status label and product text I'll be opening a new issue that addresses this specifically. As discussed in Slack we're using a UITextView here because of the multi-line support, but this also causes the miss-alignment and may require manual alignment if we want to keep this implementation.
• Resolved merge conflict.

[iamgabrielma]

One thing I noticed is that the text doesn't left-align with the other cells when there's no badge, but we can handle that as we solve the other alignment issue.

Thanks for the review! I'll be sure to add this bit to the new issue as well :D