-
Notifications
You must be signed in to change notification settings - Fork 131
[WOOMOB-192] Update Empty Coupon Screen #14014
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
[WOOMOB-192] Update Empty Coupon Screen #14014
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 adds support for a "Create Coupon" feature by updating the empty coupon screen, introducing a new UI event, and updating the corresponding view model and UI components.
- Added a new UI event (CreateCouponClicked) to handle coupon creation.
- Updated the empty coupon screen to include a new action button and adjusted spacing.
- Modified the empty list composable to support an action label and callback.
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
WooPosCouponsViewModel.kt | Added handling for CreateCouponClicked which currently throws an error. |
WooPosCouponsUIEvent.kt | Introduced the new CreateCouponClicked event. |
WooPosCouponsScreen.kt | Updated the screen to include an action button for creating a coupon. |
WooPosItemsList.kt | Overloaded the empty list composable to include an action button and adjusted spacing. |
Files not reviewed (1)
- WooCommerce/src/main/res/values/strings.xml: Language not supported
...c/main/kotlin/com/woocommerce/android/ui/woopos/home/items/coupons/WooPosCouponsViewModel.kt
Show resolved
Hide resolved
📲 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.
|
} | ||
|
||
@Composable | ||
private fun WooPosItemsEmptyListInternal( |
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.
📓 I've considered creating a sealed class to ensure actionLabel
and onActionClicked
either need to be both null or both non-null, but in the end I opted for making this function private and include Internal
in its name instead and provide two overloads. Let me know what you think.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #14014 +/- ##
============================================
- Coverage 38.29% 38.28% -0.02%
Complexity 9496 9496
============================================
Files 2118 2118
Lines 116436 116467 +31
Branches 14937 14948 +11
============================================
Hits 44585 44585
- Misses 67774 67805 +31
Partials 4077 4077 ☔ 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!
onClick = onActionClicked, | ||
modifier = Modifier | ||
.fillMaxWidth(0.5f) | ||
.height(80.dp) |
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.: Text size in the button can be scaled up in the accessibility settings. Maybe let's use top padding instead of fixed height 🤔
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.
Hmm, I see what you mean and I tend to agree, however, this was copied and there are 5+ other buttons like this in the POS. Considering we tested other projects for accessibility and didn't report this an issue I believe it might look fine even with the biggest font. At the moment, it's imo not worth the effort to diving more into this.
Closes: #WOOMOB-192
Description
This PR updates the
Empty Coupons screen
Testing information
Open POS on a store without coupons
Tap on Coupons tab
Notice the empty screen
Open POS on a store with coupons
Tap on Coupons tab
Verify coupons are listed as expected
The tests that have been performed
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: