-
Notifications
You must be signed in to change notification settings - Fork 137
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
Issue/6585 onboarding sl banner display logic #6620
Issue/6585 onboarding sl banner display logic #6620
Conversation
…OnboardingRepository
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailFragment.kt
Show resolved
Hide resolved
...c/main/kotlin/com/woocommerce/android/ui/orders/details/ShippingLabelOnboardingRepository.kt
Outdated
Show resolved
Hide resolved
...c/main/kotlin/com/woocommerce/android/ui/orders/details/ShippingLabelOnboardingRepository.kt
Outdated
Show resolved
Hide resolved
...c/main/kotlin/com/woocommerce/android/ui/orders/details/ShippingLabelOnboardingRepository.kt
Outdated
Show resolved
Hide resolved
...c/main/kotlin/com/woocommerce/android/ui/orders/details/ShippingLabelOnboardingRepository.kt
Outdated
Show resolved
Hide resolved
...c/main/kotlin/com/woocommerce/android/ui/orders/details/ShippingLabelOnboardingRepository.kt
Outdated
Show resolved
Hide resolved
You can test the changes on this Pull Request by downloading an installable build, or scanning this QR code: |
…oarding-sl-banner-display-logic # Conflicts: # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailFragment.kt # WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/details/OrderDetailViewModel.kt # WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderDetailViewModelTest.kt
Codecov Report
@@ Coverage Diff @@
## trunk #6620 +/- ##
============================================
+ Coverage 45.27% 45.29% +0.01%
- Complexity 2890 2897 +7
============================================
Files 517 518 +1
Lines 28181 28196 +15
Branches 3706 3711 +5
============================================
+ Hits 12760 12770 +10
- Misses 14242 14243 +1
- Partials 1179 1183 +4
Continue to review full report at Codecov.
|
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.
Works great Thanks for moving the onboarding logic to a different repository and adding unit tests ❤️ It looks much nicer!!
Generated by 🚫 dangerJS |
Part of: #6585
Description
This PR adds the logic to determine if the WC Shipping banner should be displayed in order detail screen. The established criteria for displaying the banner is:
Additionally there is a feature flag in place that I've moved to
OrderDetailFragment
to be able to unit tests things without depending on its value.Testing instructions
Using a site eligible for shipping labels, disable the WC Shipping plugin from
wc-core
and test that the banner is shown if this conditions are met:Test that if any of those conditions for the given order is not satisfied, the banner is not displayed.
I've created a new repository to handle the banner's visibility as I'll need to reuse some of its logic for
Settings
screen in this task.Images/gif
The banner should appear below products section:
RELEASE-NOTES.txt
if necessary.