-
Notifications
You must be signed in to change notification settings - Fork 131
[WOOMOB-271][POS] Minor Design Updates #13990
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
base: trunk
Are you sure you want to change the base?
[WOOMOB-271][POS] Minor Design Updates #13990
Conversation
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.
Pull Request Overview
This PR implements several minor design updates on the POS screens, including updating text colors and adding a chevron icon for variable product rows.
- Updates text colors for reader state titles and totals section using WooPosTheme.colors.onSurfaceVariantHighest.
- Introduces a chevron icon for variable product rows in the items list.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsScreen.kt | Updated text elements and TotalsGridRow parameters to include new color properties. |
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsList.kt | Added a chevron icon for variable products and adjusted layout via a Box with weight. |
Comments suppressed due to low confidence (1)
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/totals/WooPosTotalsScreen.kt:377
- [nitpick] Consider renaming the parameters 'colorOne' and 'colorTwo' in TotalsGridRow to more descriptive names (e.g. 'textColorOne' and 'textColorTwo') for improved clarity.
colorOne: Color = Color.Unspecified,
📲 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 ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #13990 +/- ##
============================================
- Coverage 38.30% 38.30% -0.01%
Complexity 9495 9495
============================================
Files 2118 2118
Lines 116395 116412 +17
Branches 14929 14930 +1
============================================
Hits 44586 44586
- Misses 67729 67746 +17
Partials 4080 4080 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
LGTM
@@ -223,7 +223,20 @@ fun WooPosProductCard( | |||
|
|||
Spacer(modifier = Modifier.width(WooPosSpacing.Medium.value)) | |||
|
|||
ProductInfo(item) | |||
Box(modifier = Modifier.weight(1f)) { | |||
ProductInfo(item) |
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.
💡 n.p.: I'm curious if you considered avoiding wrapping ProductInfo
with Box
. For example, can we introduce modifier: Modifier = Modifier
arg in ProductInfo
composable, and pass a Modifier with some matching state (.fillMaxWidth() maybe)? I didn't dig deep enough to be sure this would be a good solution here; it just caught my attention that we can try making ProductInfo
component configurable with Modifier instead of wrapping it.
Closes: #WOOMOB-271
Description
This PR updates:
Discount/Subtotal/Taxes
to grey.Testing information
Discount/Subtotal/Taxes
are greyThe tests that have been performed
I've tested the above.
Images/gif
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: