-
Notifications
You must be signed in to change notification settings - Fork 131
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] Error handling for load shipping rates fails #13653
Conversation
📝 WalkthroughWalkthroughThis PR refactors error handling in the WooCommerce shipping labels module. The Changes
Sequence Diagram(s)sequenceDiagram
participant S as ShippingRatesSection
participant E as ErrorMessageWithButton
participant U as User
S->>E: Invoke ErrorMessageWithButton (pass error message & retry handler)
U->>E: Clicks retry button
E->>S: Trigger retry logic
Possibly related PRs
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
📲 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 #13653 +/- ##
=========================================
Coverage 38.03% 38.04%
Complexity 9089 9089
=========================================
Files 2063 2063
Lines 113096 113088 -8
Branches 14351 14350 -1
=========================================
Hits 43021 43021
+ Misses 66164 66156 -8
Partials 3911 3911 ☔ View full report in Codecov by Sentry. |
@coderabbitai review |
✅ Actions performedReview triggered.
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/packages/components/ErrorMessageWithButton.kt (1)
45-45
: Improved text readability with centered alignment.Adding
textAlign = TextAlign.Center
enhances the user experience by ensuring the error message is properly centered, making it more readable and visually balanced, especially for multi-line text messages.Consider adding a content padding modifier to provide some spacing around the text for longer error messages:
Text( text = stringResource(message), style = MaterialTheme.typography.subtitle1, textAlign = TextAlign.Center, + modifier = Modifier.padding(horizontal = 16.dp), color = MaterialTheme.colors.onSurface, )
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/packages/components/ErrorMessageWithButton.kt
(2 hunks)WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/rates/ui/ShippingRatesSection.kt
(2 hunks)WooCommerce/src/main/res/values/strings.xml
(1 hunks)
🔇 Additional comments (4)
WooCommerce/src/main/res/values/strings.xml (1)
4462-4462
: Well-phrased error message for shipping rates loading.This new string resource provides a clear error message when shipping rates cannot be loaded, which helps improve the user experience by explaining what went wrong. The wording is consistent with other error messages in the application.
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/packages/components/ErrorMessageWithButton.kt (1)
17-17
: Added import for TextAlign to enable text centering.The TextAlign import supports the new text alignment feature added to the error message.
WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/wooshippinglabels/rates/ui/ShippingRatesSection.kt (2)
8-11
: Added imports for error handling refactoring.Appropriate imports added to support the ErrorMessageWithButton component implementation.
31-35
:✅ Verification successful
Improved error handling with reusable component.
Replaced inline error UI code with the reusable
ErrorMessageWithButton
component, which improves code maintainability and consistency across the app. The specific error message and minimum height constraint ensure a good user experience.
🏁 Script executed:
#!/bin/bash # Check if the string resource is properly defined for the error message # Look for the string resource definition used in this component grep -r "woo_shipping_labels_package_creation_shipping_rates_loading_error" --include="*.xml" .Length of output: 273
Error Message UI Component Approved – Resource Validated
The reusable
ErrorMessageWithButton
usage for improved error handling was verified. The error message resource (woo_shipping_labels_package_creation_shipping_rates_loading_error
) is correctly defined in the strings resource file, ensuring consistency and a good user experience.
A mistake Irfan, I wanted to trigger this on my own PRs while looking at yours, just because I wanted to look at something unrelated (Buildkite), apologies for the unnecessary noise... 😊 |
Tip For best results, initiate chat on the files or code changes. @ParaskP7 No problem at all! These things happen. I understand you were just looking at this PR while trying to check something related to Buildkite on your own PRs. No worries about the accidental review trigger - it's not an issue. |
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 as described!
LGTM!
Closes: #13327
Description
This updates the error view for load shipping rates.
Steps to reproduce
To create fake error for loading shipping rates
Error view
The tests that have been performed
Steps 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: