Skip to content

Conversation

@ThomazFB
Copy link
Contributor

@ThomazFB ThomazFB commented Mar 26, 2025

Summary

Fix issue #13841 by updating the Purchased screen with the Hazmat data.

The Hazmat was already defined within the UI, but it wasn't carrying any data from the Creation form. To fix this, this PR updates the data wiring between the Creation form and Purchased screen to contain the selected Hazmat category.

Also, it updates the Hazmat card inside the Purchased screen to follow the Figma designs.

Screen Capture

Hazmat.purchased.screen.mp4

How to Test

  1. Open the Shipping Label creation form
  2. Select a Hazmat category
  3. Create the label
  4. Verify that the Purchased screen correctly displays the selected Hazmat category

Update release notes:

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

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.

@ThomazFB ThomazFB added type: task An internally driven task. feature: shipping labels Related to creating, ordering, or printing shipping labels. labels Mar 26, 2025
@ThomazFB ThomazFB added this to the 22.1 milestone Mar 26, 2025
@ThomazFB ThomazFB linked an issue Mar 26, 2025 that may be closed by this pull request
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Mar 26, 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
Commitb2944cf
Direct Downloadwoocommerce-wear-prototype-build-pr13847-b2944cf.apk

@ThomazFB ThomazFB changed the title Issue/update purchase screen hazmat data [Shipping Labels Revamp] Update purchase screen with HAZMAT data Mar 26, 2025
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Mar 26, 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
Commitb2944cf
Direct Downloadwoocommerce-prototype-build-pr13847-b2944cf.apk

Base automatically changed from issue/add-hazmat-category-edit-option to trunk March 31, 2025 09:01
@codecov-commenter
Copy link

codecov-commenter commented Apr 2, 2025

Codecov Report

Attention: Patch coverage is 5.71429% with 66 lines in your changes missing coverage. Please review.

Project coverage is 38.29%. Comparing base (ed48b9f) to head (b2944cf).
Report is 31 commits behind head on trunk.

Files with missing lines Patch % Lines
...d/ui/orders/wooshippinglabels/hazmat/HazmatCard.kt 0.00% 66 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              trunk   #13847      +/-   ##
============================================
- Coverage     38.31%   38.29%   -0.03%     
  Complexity     9357     9357              
============================================
  Files          2088     2089       +1     
  Lines        115379   115448      +69     
  Branches      14743    14769      +26     
============================================
+ Hits          44206    44209       +3     
- Misses        67147    67213      +66     
  Partials       4026     4026              

☔ 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.

@ThomazFB ThomazFB marked this pull request as ready for review April 2, 2025 04:15
@ThomazFB
Copy link
Contributor Author

ThomazFB commented Apr 2, 2025

@irfano @atorresveiga during this PR progress, I've noticed that the Purchased screen contains a code repetition of the Purchased composable. One of them is used solely for Preview purposes. This can be verified by comparing the code of the WooShippingLabelPurchasedWithBottomSheetScreen and WooShippingLabelPurchasedScreen composables.

I understand the reasoning for the preview-only code, but this is causing a lot of confusion during development. More than once, I ran an entire Label purchase flow for tests and then noticed that I only updated the Preview composable, not the real one.

I'm inclined to refactor this in a separate PR so that the WooShippingLabelPurchasedWithBottomSheetScreen reuses the WooShippingLabelPurchasedScreen internally, so we remove this duplication. What do you folks think?

@ThomazFB ThomazFB requested review from atorresveiga and irfano April 2, 2025 04:21
@dangermattic
Copy link
Collaborator

dangermattic commented Apr 2, 2025

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

import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding
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.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding
import androidx.compose.material.Icon
import androidx.compose.material.MaterialTheme

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.material.Icon
import androidx.compose.material.MaterialTheme
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.MaterialTheme
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
@irfano irfano modified the milestones: 22.1, 22.2 Apr 4, 2025
@irfano irfano self-assigned this Apr 4, 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.

I verified the Hazmat selection on the purchased screen. LGTM! 👍🏻

@irfano
Copy link
Contributor

irfano commented Apr 7, 2025

I'm inclined to refactor this in a separate PR so that the WooShippingLabelPurchasedWithBottomSheetScreen reuses the WooShippingLabelPurchasedScreen internally, so we remove this duplication. What do you folks think?

This makes sense, and we can do it in a separate PR. Opened an issue: WOOMOB-266

@irfano irfano enabled auto-merge April 7, 2025 16:20
# Conflicts:
#	WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationScreen.kt
#	WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/WooShippingLabelCreationViewModel.kt
@irfano irfano force-pushed the issue/update-purchase-screen-hazmat-data branch from df4e6cb to b2944cf Compare April 7, 2025 16:41
@irfano irfano merged commit f0bd46f into trunk Apr 7, 2025
17 checks passed
@irfano irfano deleted the issue/update-purchase-screen-hazmat-data branch April 7, 2025 17:11
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.

[Hazmat] Update Purchase screen Hazmat section

6 participants