Align Android message status wording - #6133
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughMessage routing errors now use dedicated resources, clearer text, expanded retryability coverage, and updated message status presentation. Documentation and tests reflect the new error mappings and UI behavior. ChangesMessage Routing Error Feedback
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches 💡 2⚔️ Resolve merge conflicts 💡
🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hello @RCGV1 -- This is excellent. I have already volunteered to help documentation for the Android UI as soon as the details are sorted. |
|
A few things before this comes out of draft:
The core wording/logic change looks clean (no |
jamesarich
left a comment
There was a problem hiding this comment.
Requesting changes — three items to resolve before this leaves draft (detail in the comment above):
- Revert the 36 Crowdin-managed locale docs.
crowdin.ymlownsdocs/{code}/user/; hand-editing them corrupts localized content (English injected into non-English tables, e.g.docs/ar-rSA/user/messages-and-channels.md:80) and gets clobbered on the next sync. Keep onlydocs/en/user/messages-and-channels.md. This drops the PR from 44 files to ~6. - Commit the screenshot goldens for the new
MessageItemPreviews.ktstates —screenshot-checkis red without them. - Document the
NO_CHANNELbehavior change (Message.kt:166makes channel/key-mismatch failures retryable + warning-colored) in the description, and confirm it's intended — it's a behavior change, not just wording.
The core logic/wording change itself looks clean.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@feature/messaging/src/commonTest/kotlin/org/meshtastic/feature/messaging/component/MessageItemTest.kt`:
- Around line 207-231: Update channelKeyMismatch_isTerminalAndUsesErrorColor to
verify the rendered MessageItem styling rather than calling messageStatusColor
directly; use a retryable routing-error message and assert its warning color
through the UI, while keeping the NO_CHANNEL terminal-text assertion separate.
Ensure the test would fail if MessageItem stops passing isRetryableFailure.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 34530f3e-9419-4cc0-8246-1b2938259072
⛔ Files ignored due to path filters (2)
screenshot-tests/src/screenshotTestDebug/reference/org/meshtastic/screenshots/feature/MessagingScreenshotTestsKt/ScreenshotMessageItemStatusStates_Dark_d19fbf1f_0.pngis excluded by!**/*.png,!**/*.pngscreenshot-tests/src/screenshotTestDebug/reference/org/meshtastic/screenshots/feature/MessagingScreenshotTestsKt/ScreenshotMessageItemStatusStates_Light_b29dc7a7_0.pngis excluded by!**/*.png,!**/*.png
📒 Files selected for processing (8)
.skills/compose-ui/strings-index.txtcore/model/src/commonMain/kotlin/org/meshtastic/core/model/Message.ktcore/model/src/commonTest/kotlin/org/meshtastic/core/model/DataPacketTest.ktcore/resources/src/commonMain/composeResources/values/strings.xmldocs/en/user/messages-and-channels.mdfeature/messaging/src/commonMain/kotlin/org/meshtastic/feature/messaging/component/MessageItem.ktfeature/messaging/src/commonMain/kotlin/org/meshtastic/feature/messaging/component/MessageItemPreviews.ktfeature/messaging/src/commonTest/kotlin/org/meshtastic/feature/messaging/component/MessageItemTest.kt
…s-routing-followup # Conflicts: # feature/messaging/src/commonMain/kotlin/org/meshtastic/feature/messaging/component/MessageItem.kt # screenshot-tests/src/screenshotTestDebug/reference/org/meshtastic/screenshots/feature/MessagingScreenshotTestsKt/ScreenshotMessageItemStatusStates_Dark_d19fbf1f_0.png # screenshot-tests/src/screenshotTestDebug/reference/org/meshtastic/screenshots/feature/MessagingScreenshotTestsKt/ScreenshotMessageItemStatusStates_Light_b29dc7a7_0.png
Summary
Channel/key mismatchterminal and error-colored: resending unchanged content cannot repair a channel or PSK mismatch.Screenshots
Validation
core:modelandfeature:messagingJVM testsHardware verification
Not performed; the PR remains draft.
Summary by CodeRabbit
Bug Fixes
Documentation
Localization