Fix the layout issue in the Woo update dialog #13851
Conversation
This commit introduces the `WooUpgradeRequiredDialogFragment`, which is a custom DialogFragment used to display a dialog that informs the user about a required WooCommerce upgrade. This dialog is implemented using Jetpack Compose, and it allows the user to navigate to the WooCommerce upgrade instructions using a custom Chrome tab. It also includes an option to dismiss the dialog.
irfano
added
the
category: design
irfano
changed the title
📲 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.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/sitepicker/WooUpgradeRequiredDialog.kt
Dismissed
Show dismissed
Hide dismissed
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/sitepicker/WooUpgradeRequiredDialog.kt
Dismissed
Show dismissed
Hide dismissed
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/sitepicker/WooUpgradeRequiredDialog.kt
Dismissed
Show dismissed
Hide dismissed
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #13851 +/- ##
============================================
- Coverage 38.14% 38.12% -0.02%
- Complexity 9296 9297 +1
============================================
Files 2082 2083 +1
Lines 114936 114976 +40
Branches 14651 14662 +11
============================================
Hits 43839 43839
- Misses 67105 67144 +39
- Partials 3992 3993 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| return Dialog(requireContext()).apply { | ||
| setContentView(composeView) | ||
| setCancelable(false) |
There was a problem hiding this comment.
np, this would create two dialogs (ie two windows), Compose's AlertDialog will create its own dialog, and we are creating another one here.
I'm just sharing this, I don't think it's a blocker, and I think the simplicity of the current solution.
Also, since this dialog is rarely (if at all) is shown, it's not an issue.
There was a problem hiding this comment.
Good catch @hichamboushaba! I’ve implemented a better solution by using a ComposeView in the SitePickerFragment. The WooUpgradeDialog remains the same, I simply removed the dialog fragment and integrated this composable dialog directly into the SitePickerFragment.
hichamboushaba
left a comment
hichamboushaba
left a comment
There was a problem hiding this comment.
Thanks for fixing this @irfano 👍
Generated by 🚫 Danger |
Closes: #13843
Description
This fixes the layout issue in the Woo update dialog, which was broken on tablets. I migrated the dialog to Compose and resolved the issue. This PR also updates the dialog’s style to align with the current design used in the app.
Steps to reproduce
The tests that have been performed
Steps above and screens below.
Images/gif
RELEASE-NOTES.txtif 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: