Allow Re-creating Shipping Label #3961

hafizrahman · 2021-05-04T08:36:16Z

What this PR does

Allow user to re-purchase Shipping Label on the same Order, and modify the Order Details screen's design to accommodate it.

Testing Instructions

Prepare two Orders: one with no Shipping Labels created yet (but eligible for creation), and one with Shipping Labels already created.

1. Order with no Shipping Label

Open Order Details screen
Make sure the "Create shipping label" button is shown.
Make sure that the "Product" panel at the top does not display triple-dot menu button on the upper right side.

Screenshot	Screenshot with notes

2. Order with One Shipping Label

Open Order Details screen
Make sure the "Create shipping label" button is not shown
Make sure that the "Product" panel at the to displays triple-dot menu button on the upper right side.
Tap the triple-dot menu button and make sure "Create new shipping label" menu item shows up.
Tap the "Create new shipping label" menu item and make sure the Shipping Label creation screen is shown and working.
Come back to Order Details screen, scroll down to the "Package 1" section.
Make sure it shows "X item(s)" under the "Package 1" heading, with an arrow to the right side.
Make sure the list of products is not yet shown on the "Package 1" section
Tap the "X items(s)" label or its arrow, and make sure it shows the list of products below it.
Tap it again and make sure it hides the list of products below it.

Order details	Order details product menu item

Package panel toggle: off	Package panel toggle: on

Update release notes:

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

…ng labels existence.

…bel" menu item to it.

…ing shipping label. If there are shipping label(s), we want to move the option to re-create shipping label in the Products area's menu.

…g label(s).

peril-woocommerce · 2021-05-04T08:44:18Z

You can test the changes on this Pull Request by downloading the APK here.

Also refactor viewmodel to make the code more testable.

hafizrahman · 2021-05-05T07:32:18Z

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailViewModel.kt

            isShipmentTrackingAvailable = shipmentTracking.isVisible,
            isProductListVisible = orderProducts.isVisible,
-            areShippingLabelsVisible = shippingLabels.isVisible
+            areShippingLabelsVisible = shippingLabels.isVisible,
+            isProductListMenuVisible = shippingLabels.isVisible


Previously I used areShippingLabelsVisible as checker for displaying/hiding the Products list menu button. However, I found that it made it not super obvious for unit testing it. So while this is a bit redundant (i.e.: isProductListMenuVisible will always have the same value as areShippingLabelsVisible), it helps make it be more obvious during unit testing, like so:

assertThat(shippingLabels).isNotEmpty assertThat(isCreateShippingLabelButtonVisible).isFalse() assertThat(isProductListMenuVisible).isTrue()

hichamboushaba

Really great job @hafizrahman 👏
I left some comments about the usage of the Woo.Card.ExpanderButton style, as it's not matching the mockups, and some other small tips.

hichamboushaba · 2021-05-05T11:56:30Z

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailViewModel.kt

@@ -545,7 +538,8 @@ class OrderDetailViewModel @AssistedInject constructor(
        val refreshedProductId: Long? = null,
        val isCreateShippingLabelButtonVisible: Boolean? = null,
        val isProductListVisible: Boolean? = null,
-        val areShippingLabelsVisible: Boolean? = null
+        val areShippingLabelsVisible: Boolean? = null,
+        val isProductListMenuVisible: Boolean? = null


It's nice to have a an explicit variable about this as you said in https://github.com/woocommerce/woocommerce-android/pull/3961/files#r626326768, I like it.
But as its value is always same as areShippingLabelsVisible, we can simplify the logic a bit, and avoid any unwanted bugs (for example changing isProductListMenuVisible in some other areas of the viewmodel) by making it a property without a backing field, similar to the below property isMarkOrderCompleteButtonVisible, we can do this like this:

val isProductListMenuVisible: Boolean? get() = areShippingLabelsVisible

Thank you, that's a great tips!

hichamboushaba · 2021-05-05T12:06:13Z

...kotlin/com/woocommerce/android/ui/orders/details/adapter/OrderDetailShippingLabelsAdapter.kt

+                if (isChecked)
+                    WooAnimUtils.fadeIn(viewBinding.shippingLabelListProducts)
+                else
+                    WooAnimUtils.fadeOut(viewBinding.shippingLabelListProducts)


Normally we should avoid using single-line statement if blocks without braces, so either move the statement to the same line:

if (isChecked) WooAnimUtils.fadeIn(viewBinding.shippingLabelListProducts) else WooAnimUtils.fadeOut(viewBinding.shippingLabelListProducts)

or add curly braces to those statements.

Awesome, learned something new here. Thanks.

hichamboushaba · 2021-05-05T13:17:41Z

WooCommerce/src/main/res/values/strings.xml

@@ -300,6 +300,7 @@
    <string name="orderdetail_shipping_notice">This order is using extensions to calculate shipping. The shipping methods shown might be incomplete.</string>
    <string name="orderdetail_issue_refund_button">Issue refund</string>
    <string name="orderdetail_collect_payment_button">Collect Payment</string>
+    <string name="orderdetail_products_more_button">Create new shipping label</string>


np, I think in our last session we were planning to rename this string resource to something more meaningful, and that this was temporary 🤔, can you please rename it to something like orderdetail_products_recreate_label_button

Thanks for reminding me! I missed this part.

hichamboushaba · 2021-05-05T13:20:39Z

WooCommerce/src/main/res/layout/order_detail_shipping_label_list_item.xml

+        <ToggleButton
+            android:id="@+id/shippingLabelList_countExpander"
+            style="@style/Woo.Card.ExpanderButton"
+            android:layout_width="match_parent"
+            android:layout_height="wrap_content"
+            android:paddingTop="@dimen/major_85"
+            android:paddingBottom="@dimen/major_85" />


I don't think we can use the style Woo.Card.ExpanderButton here, as the arrow image there uses a smaller size 12dp, while in the designs we have 24dp, and also the color is different.
I think you can achieve similar effect using the same technique as the button shippingLabelItem_viewMoreButtonTitle below, and with more customizability.

Nice attention to details, I didn't realize the button size and behavior are different. I've refactored this to be something similar to shippingLabelItem_viewMoreButtonTitle instead.

Refer to https://softwareengineering.stackexchange.com/a/16530 for more discussions, concluding that putting things in-line with the "if"/"else" statement is better.

It should now match the "Show Shipment Details" toggle as well.

hafizrahman · 2021-05-06T10:22:36Z

Thanks for your feedbacks @hichamboushaba . This is now ready for another review.

hichamboushaba

Really good job @hafizrahman, looks good to me

hafizrahman added 5 commits April 30, 2021 16:24

Make Products area in Order details be permanent regardless of Shippi…

b4839dc

…ng labels existence.

Add triple-dot menu to Products list, and add "Create new shipping la…

f898ccd

…bel" menu item to it.

Only show the "Create shipping label" main button if there's no exist…

9d1a77e

…ing shipping label. If there are shipping label(s), we want to move the option to re-create shipping label in the Products area's menu.

Only show the Products list menu button if there are existing shippin…

89bf6a9

…g label(s).

Make the products list in each Shipping Label detail be collapsible.

1c47e14

hafizrahman added the feature: shipping labels Related to creating, ordering, or printing shipping labels. label May 4, 2021

hafizrahman added this to the 6.7 milestone May 4, 2021

Add/fix tests related to Add Shipping Label button/menu display.

570e632

Also refactor viewmodel to make the code more testable.

hafizrahman commented May 5, 2021

View reviewed changes

hafizrahman added 2 commits May 5, 2021 14:53

Update and add tests related to button/menu show/hiding.

a7413bb

Lint fixes.

45b137f

hafizrahman modified the milestones: 6.7, 6.6 ❄️ May 5, 2021

hafizrahman added 2 commits May 5, 2021 15:51

Release notes update.

9501c64

Merge branch 'develop' into issue/3856-allow-recreating-shipping-label

7c06916

hafizrahman modified the milestones: 6.6 ❄️, 6.7 May 5, 2021

hafizrahman marked this pull request as ready for review May 5, 2021 09:24

hafizrahman requested a review from a team May 5, 2021 09:25

hichamboushaba self-assigned this May 5, 2021

hichamboushaba requested changes May 5, 2021

View reviewed changes

hafizrahman added 6 commits May 6, 2021 16:23

Simplify single-line if blocks.

88fb519

Refer to https://softwareengineering.stackexchange.com/a/16530 for more discussions, concluding that putting things in-line with the "if"/"else" statement is better.

Simplify usage for "isProductListMenuVisible".

cee2993

Clarify string name for menu item "Create new shipping label".

1ca3b1d

String fix.

a1c0674

Fix test.

4ebecda

Refactor toggle button for better styling and animation.

09cecc1

It should now match the "Show Shipment Details" toggle as well.

hichamboushaba approved these changes May 6, 2021

View reviewed changes

hichamboushaba merged commit 24a1834 into develop May 6, 2021

hichamboushaba deleted the issue/3856-allow-recreating-shipping-label branch May 6, 2021 14:32

hichamboushaba mentioned this pull request May 18, 2021

Shipping Labels: shipping labels re-creation is allowed for non eligible orders #4052

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow Re-creating Shipping Label #3961

Allow Re-creating Shipping Label #3961

hafizrahman commented May 4, 2021 •

edited

Loading

peril-woocommerce bot commented May 4, 2021 •

edited

Loading

hafizrahman May 5, 2021 •

edited

Loading

hichamboushaba left a comment

hichamboushaba May 5, 2021

hafizrahman May 6, 2021

hichamboushaba May 5, 2021

hafizrahman May 6, 2021

hichamboushaba May 5, 2021

hafizrahman May 6, 2021

hichamboushaba May 5, 2021

hafizrahman May 6, 2021

hafizrahman commented May 6, 2021

hichamboushaba left a comment

Allow Re-creating Shipping Label #3961

Allow Re-creating Shipping Label #3961

Conversation

hafizrahman commented May 4, 2021 • edited Loading

What this PR does

Testing Instructions

1. Order with no Shipping Label

2. Order with One Shipping Label

peril-woocommerce bot commented May 4, 2021 • edited Loading

hafizrahman May 5, 2021 • edited Loading

Choose a reason for hiding this comment

hichamboushaba left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hafizrahman commented May 6, 2021

hichamboushaba left a comment

Choose a reason for hiding this comment

hafizrahman commented May 4, 2021 •

edited

Loading

peril-woocommerce bot commented May 4, 2021 •

edited

Loading

hafizrahman May 5, 2021 •

edited

Loading