Skip to content

Conversation

@atorresveiga
Copy link
Contributor

@atorresveiga atorresveiga commented Mar 21, 2025

Part of: #13699

Description

This PR adds support for displaying selectable items in the split shipment flow. The changes included in this PR are:

  • Update the args used to open the split shipment flow
  • Move all shippable item transformation functions to ShippableItemsMapper
  • Create the SelectableShippingProduct and ExpandableSelectableShippingProduct composables

The changes in this PR focus only on the UI. The selection and expandable state will be added in a different PR

Testing information

  1. Open the orders tab
  2. Tap on an order eligible for shipping label creation without a shipping address
  3. Tap on Create shipping label
  4. Check that the Split shipment button is visible
  5. Tap on Split shipment
  6. Check that the app navigates to the split shipment screen
  7. Check that the shippable product lists are accurate

The tests that have been performed

  • Checked the UI is displayed as expected

Images/gif

Screen_recording_20250321_161104.mp4
  • I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

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 big (tablet) and small (phone) in case of UI changes, and no regressions are added.

@atorresveiga atorresveiga added type: task An internally driven task. feature: shipping labels Related to creating, ordering, or printing shipping labels. labels Mar 21, 2025
@atorresveiga atorresveiga added this to the 22.1 milestone Mar 21, 2025
@atorresveiga atorresveiga requested review from ThomazFB and irfano March 21, 2025 22:12
@dangermattic
Copy link
Collaborator

1 Warning
⚠️ This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

SplitShipmentArgs(
orderId = navArgs.orderId,
storeOptions = currentStoreOptions,
shipments = mapOf(0 to currentShippableItems)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be updated after adding support for multiple shipments

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Mar 21, 2025

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
App Name WooCommerce-Wear Android
Platform⌚️ Wear OS
FlavorJalapeno
Build TypeDebug
Commit99dd285
Direct Downloadwoocommerce-wear-prototype-build-pr13827-99dd285.apk

}
if (isExpanded) {
repeat(quantity.toInt()) {
var selection by remember { mutableStateOf(false) }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The view model will handle the selection and expandable state in the next PR. I added a local state here to test the UI

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Mar 21, 2025

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

App Name WooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Commit99dd285
Direct Downloadwoocommerce-prototype-build-pr13827-99dd285.apk

import androidx.compose.foundation.layout.sizeIn
import androidx.compose.foundation.shape.CircleShape
import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.material.Divider

Check warning

Code scanning / Android Lint

material and material3 are separate, incompatible design system libraries Warning

Using a material import while also using the material3 library
import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.material.Divider
import androidx.compose.material.Icon
import androidx.compose.material.IconButton

Check warning

Code scanning / Android Lint

material and material3 are separate, incompatible design system libraries Warning

Using a material import while also using the material3 library
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.foundation.lazy.items
import androidx.compose.material.Icon

Check warning

Code scanning / Android Lint

material and material3 are separate, incompatible design system libraries Warning

Using a material import while also using the material3 library
import androidx.compose.foundation.lazy.LazyColumn
import androidx.compose.foundation.lazy.items
import androidx.compose.material.Icon
import androidx.compose.material.IconButton

Check warning

Code scanning / Android Lint

material and material3 are separate, incompatible design system libraries Warning

Using a material import while also using the material3 library
import androidx.compose.foundation.lazy.items
import androidx.compose.material.Icon
import androidx.compose.material.IconButton
import androidx.compose.material.Surface

Check warning

Code scanning / Android Lint

material and material3 are separate, incompatible design system libraries Warning

Using a material import while also using the material3 library
import androidx.compose.material.Icon
import androidx.compose.material.IconButton
import androidx.compose.material.Surface
import androidx.compose.material.Text

Check warning

Code scanning / Android Lint

material and material3 are separate, incompatible design system libraries Warning

Using a material import while also using the material3 library
import androidx.compose.material.IconButton
import androidx.compose.material.Surface
import androidx.compose.material.Text
import androidx.compose.material.TopAppBar

Check warning

Code scanning / Android Lint

material and material3 are separate, incompatible design system libraries Warning

Using a material import while also using the material3 library
@codecov-commenter
Copy link

codecov-commenter commented Mar 21, 2025

Codecov Report

Attention: Patch coverage is 26.85185% with 237 lines in your changes missing coverage. Please review.

Project coverage is 38.08%. Comparing base (e80e478) to head (3f2a686).

Files with missing lines Patch % Lines
...rders/wooshippinglabels/WooShippingProductsCard.kt 0.00% 212 Missing ⚠️
...hippinglabels/WooShippingLabelCreationViewModel.kt 27.77% 13 Missing ⚠️
...glabels/split/WooShippingSplitShipmentViewModel.kt 84.61% 4 Missing and 2 partials ⚠️
...i/orders/wooshippinglabels/ShippableItemsMapper.kt 95.91% 1 Missing and 1 partial ⚠️
...stoms/products/WooShippingCustomsProductUIModel.kt 0.00% 0 Missing and 2 partials ⚠️
...glabels/customs/WooShippingCustomsFormViewModel.kt 0.00% 1 Missing ⚠️
...ders/wooshippinglabels/models/StoreOptionsModel.kt 0.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              trunk   #13827      +/-   ##
============================================
- Coverage     38.10%   38.08%   -0.03%     
  Complexity     9256     9256              
============================================
  Files          2079     2079              
  Lines        114560   114821     +261     
  Branches      14593    14637      +44     
============================================
+ Hits          43657    43728      +71     
- Misses        66936    67122     +186     
- Partials       3967     3971       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@irfano irfano self-assigned this Mar 24, 2025
Copy link
Contributor

@irfano irfano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are a few issues I noticed:

  • I believe the checkbox for the product group item is intended to select all items. Currently, it’s not functioning that way.
  • I think the total item count should reflect the count of all individual items. However, this behavior is the same on the previous screen as well. We can address it for both screens in a separate PR.
  • Left padding issue: The left padding of the expanded item is larger than intended. As a result, the expanded and product items appear at different x positions on the screen. Could you please check the Figma file and adjust the paddings accordingly?
Figma PR
  • There’s a minor bottom padding issue.

@atorresveiga
Copy link
Contributor Author

I believe the checkbox for the product group item is intended to select all items. Currently, it’s not functioning that way.

This PR focused only on displaying the UI. This more advanced selection behavior is tackled in this PR #13833

@atorresveiga
Copy link
Contributor Author

I think the total item count should reflect the count of all individual items. However, this behavior is the same on the previous screen as well. We can address it for both screens in a separate PR.

I will fix this in a different PR

Alejo added 2 commits March 24, 2025 14:32
…ipment-products-ui

# Conflicts:
#	WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt
#	WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/models/ShippableItemModel.kt
@atorresveiga
Copy link
Contributor Author

The left padding of the expanded item is larger than intended. As a result, the expanded and product items appear at different x positions on the screen. Could you please check the Figma file and adjust the paddings accordingly?

Fixed here 02256d0

@atorresveiga
Copy link
Contributor Author

There’s a minor bottom padding issue.

Fixed here cfb7f30

@atorresveiga
Copy link
Contributor Author

I was facing some conflicts due to the total value being a Float. I refactored this to use BigDecimal, considering that we use BigDecimal for the price edbb6d0.

Do you have any concerns with this refactor? CC @ThomazFB

@atorresveiga
Copy link
Contributor Author

@irfano PR is ready for another round

Copy link
Contributor

@irfano irfano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your comments and addressing my feedback. Great work on this feature. 🚀
I'm approving now but not merging to wait for Thomaz's feedback on the BigDecimal change.

@ThomazFB
Copy link
Contributor

Do you have any concerns with this refactor?

Not at all! I think the BigDecimal adoption here is the best move. Thanks for adding it, @atorresveiga.

…ipment-products-ui

# Conflicts:
#	WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt
#	WooCommerce/src/main/res/navigation/nav_graph_orders.xml
@atorresveiga atorresveiga enabled auto-merge April 1, 2025 22:21
@atorresveiga atorresveiga merged commit 37377b7 into trunk Apr 1, 2025
17 checks passed
@atorresveiga atorresveiga deleted the issue/13699-split-shipment-products-ui branch April 1, 2025 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: shipping labels Related to creating, ordering, or printing shipping labels. type: task An internally driven task.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants