-
Notifications
You must be signed in to change notification settings - Fork 136
[Shipping Labels] Make selected hazmat box tappable #14576
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Shipping Labels] Make selected hazmat box tappable #14576
Conversation
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
|
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #14576 +/- ##
============================================
- Coverage 38.31% 38.31% -0.01%
Complexity 9659 9659
============================================
Files 2056 2056
Lines 114953 114955 +2
Branches 15252 15254 +2
============================================
- Hits 44044 44042 -2
- Misses 66881 66885 +4
Partials 4028 4028 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
hichamboushaba
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks @irfano.
| onClick: OnClick? = null, | ||
| modifier: Modifier = Modifier |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
np, Modifier should be the first optional parameter, The IDE is also showing a warning about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done: 3c1f1a1
| Box( | ||
| modifier = Modifier | ||
| .fillMaxWidth() | ||
| .then(if (onClick == null) Modifier else Modifier.clickable { onClick() }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
np, I would suggest changing the modifiers a bit to improve the click ripple effect, the following patch achieves a better effect:
patch
Index: WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/hazmat/HazmatSelectionCard.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/hazmat/HazmatSelectionCard.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/hazmat/HazmatSelectionCard.kt
--- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/hazmat/HazmatSelectionCard.kt (revision 95f3732633abe38fc6ec987e84d512a469a741a8)
+++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/hazmat/HazmatSelectionCard.kt (date 1757341157252)
@@ -12,6 +12,7 @@
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
+import androidx.compose.ui.draw.clip
import androidx.compose.ui.res.colorResource
import androidx.compose.ui.res.dimensionResource
import androidx.compose.ui.res.stringResource
@@ -30,11 +31,9 @@
Box(
modifier = Modifier
.fillMaxWidth()
+ .clip(RoundedCornerShape(dimensionResource(R.dimen.corner_radius_large)))
.then(if (onClick == null) Modifier else Modifier.clickable { onClick() })
- .background(
- color = colorResource(R.color.light_colored_button_background),
- shape = RoundedCornerShape(dimensionResource(R.dimen.corner_radius_large))
- )
+ .background(color = colorResource(R.color.light_colored_button_background))
.padding(16.dp)
) {
Text(| Before | After |
|---|---|
![]() |
![]() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch, thank you! Applied your patch: 30982e1
| - [*] Automatically populate the weight field in the create shipment flow [https://github.com/woocommerce/woocommerce-android/pull/14525] | ||
| - [*] [Shipping Labels] Fixed displaying incorrect hazardous material option for purchased labels [https://github.com/woocommerce/woocommerce-android/pull/14571] | ||
| - [*] Shipping Labels: Updated the disabled button text on the package selection screen [https://github.com/woocommerce/woocommerce-android/pull/14558] | ||
| - [*] Enhance the shipping label creation screen by making the selected hazardous box tappable [https://github.com/woocommerce/woocommerce-android/pull/14576] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@irfano it seems we both missed this and the release note was included in 23.2 (as noted by Huong here p1757419135627609/1757414420.859729-slack-C03L1NF1EA3).
Can you create a small PR to fix the release notes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yes 🤦🏻♂️. I've opened a PR.


Part of WOOMOB-12
Description
This makes the selected hazmat box tappable. When tapped, it opens the hazmat screen, just like the "Are you shipping dangerous good or hazardous materials?" sentence above.
Steps to reproduce
The tests that have been performed
Steps above.
Images/gif
RELEASE-NOTES.txtif necessary. Use the "[Internal]" label for non-user-facing changes.