Additional tweaks to delivery progress text #3010
Conversation
…ent unit. Fixed a misidentified class (missed in change yesterday), calling a TextView a ConnectRegularTextView. Fixed a typo in a variable name.
OrangeAndGreen
requested review from
Jignesh-dimagi,
avazirna,
pm-dimagi and
shubham1g5
📝 WalkthroughWalkthroughThe pull request adds several new localized string resources for Portuguese, Swahili, Tigrinya, and the default English language to provide messages about remaining tasks or visits due by today or tomorrow and overall status updates. In addition, it refines the control flow of the Sequence Diagram(s)sequenceDiagram
participant VH as ViewHolder
participant AD as Adapter
participant RS as Resource Manager
VH->>AD: onBindViewHolder(data: pending, remainingDays)
AD->>AD: Check if pending > 0?
alt Pending > 0
AD->>AD: Switch on remainingDays value
alt Case: remainingDays (0, 1, 2, >2)
AD->>RS: Retrieve appropriate localized string (with pending value)
end
else Pending == 0
AD->>RS: Retrieve "visits done" string resource
end
AD->>VH: Update tvRemaining with computed string
Suggested reviewers
✨ Finishing Touches
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. 🪧 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/res/values-pt/strings.xml (1)
510-513: New Portuguese Delivery Progress Strings AddedThe new string resources for delivery progress—
connect_results_summary_remaining_tomorrow,connect_results_summary_remaining_today,connect_results_summary_visits_done, andconnect_results_summary_days_over—are clearly added. Please double-check that the translations (“%d até Amanhã”, “%d até Hoje”, “Todas as Visitas Realizadas”, “Dias Acima”) accurately capture the intended messaging and tone in Portuguese. Additionally, verify that the%dplaceholder usage is consistent with how it’s formatted in other parts of the app.app/res/values/strings.xml (1)
614-617: New English Delivery Progress Feedback Strings AddedThe new (or updated) string resources for delivery progress in English—
connect_results_summary_remaining_tomorrow,connect_results_summary_remaining_today,connect_results_summary_visits_done, andconnect_results_summary_days_over—provide clear messaging for the delivery scenarios. Please ensure that the wording “%d by tomorrow”, “%d by today”, “All Visits Done”, and “Days Over” fully aligns with the design and tone guidelines, and that “Days Over” is the best choice to indicate that the delivery period has passed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/res/values-pt/strings.xml(1 hunks)app/res/values-sw/strings.xml(1 hunks)app/res/values-ti/strings.xml(1 hunks)app/res/values/strings.xml(1 hunks)app/src/org/commcare/adapters/ConnectDeliveryProgressReportAdapter.java(1 hunks)app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.java(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.java (1)
Learnt from: pm-dimagi
PR: dimagi/commcare-android#2847
File: app/src/org/commcare/android/database/connect/models/ConnectJobRecordV4.java:123-133
Timestamp: 2025-04-02T13:59:29.672Z
Learning: In ConnectJobRecord and related classes, paymentAccrued is guaranteed to be a small integer value that will not overflow and does not need decimal/floating point support.
🔇 Additional comments (5)
app/src/org/commcare/adapters/ConnectDeliveryProgressReportAdapter.java (1)
48-66: Improved text handling for delivery progress statesThe new implementation enhances user experience by providing more specific messages based on time remaining and task status:
- Shows "by today" when only 1 day remains
- Shows "by tomorrow" when 2 days remain
- Indicates "Days Over" when the delivery period has ended
- Displays "All Visits Done" when all tasks are completed
This implementation aligns well with the PR objectives to provide better contextual information for users.
app/res/values-sw/strings.xml (1)
596-599: String resources properly added for Swahili localizationThese new string resources provide appropriate Swahili translations for the time-sensitive delivery notifications:
- "remaining_tomorrow" for tasks due tomorrow
- "remaining_today" for tasks due today
- "visits_done" for completed visits
- "days_over" for expired delivery periods
The translations are consistent with the functionality described in the PR objectives.
app/res/values-ti/strings.xml (1)
494-497: LGTM! Added localized strings for delivery progress messages.The new strings provide appropriate Tigrinya translations for the delivery progress indicators as described in the PR objectives:
- "By tomorrow" indicator for delivery timing
- "By today" for urgent deliveries
- "All visits done" completion status
- "Days over" for expired delivery periods
These additions enhance the user experience for Tigrinya-speaking users.
app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.java (2)
54-54: Good type simplification from ConnectRegularTextView to TextView.Using the more generic TextView type instead of the specialized ConnectRegularTextView simplifies the code while maintaining functionality. This change reduces unnecessary coupling to custom view implementations.
203-203: Fixed variable naming from "discrepation" to "description".The variable has been appropriately renamed from
tvJobDiscrepationtotvJobDescription, which better reflects its purpose of displaying the job description. The corresponding usage in line 211 has also been updated correctly.Also applies to: 211-211
| <string name="connect_results_summary_all">ኩሎም</string> | ||
| <string name="connect_results_summary_remaining">ዝተረፈ</string> | ||
| <string name="connect_results_summary_remaining_days">%d ኣብ ውሽጢ %d መዓልቲ</string> | ||
| <string name="connect_results_summary_remaining_tomorrow">%d ብ ጽባሕ</string> |
There was a problem hiding this comment.
you leave me speechless!
| ConnectBoldTextView hoursTitle = viewJobCard.findViewById(R.id.tvDailyVisitTitle); | ||
| ConnectBoldTextView tv_job_time = viewJobCard.findViewById(R.id.tv_job_time); | ||
| ConnectMediumTextView tvJobDiscrepation = viewJobCard.findViewById(R.id.tv_job_discrepation); | ||
| ConnectMediumTextView tvJobDescription = viewJobCard.findViewById(R.id.tv_job_discrepation); |
There was a problem hiding this comment.
seems like a typo in tv_job_discrepation
There was a problem hiding this comment.
Good call, I found a few other similar typos when I searched so corrected them all in this commit: dec55c1
https://dimagi.atlassian.net/browse/CCCT-897
cross-request: dimagi/commcare-core#1455
Product Description
Changed the displayed text in payment unit progress cards for special situations:
Technical Summary
Some additional logic and new strings to handle the additional displays.
Also saw a misidentified class that I should have change in a commit yesterday.
It was low-risk since all we call is setText and both classes implement the method the same way, but good to fix.
Feature Flag
Connect
Safety Assurance
Safety story
Tested display manually.
Automated test coverage
No automated tests for Connect yet.
QA Plan
Verify the text is displayed correctly for each of the scenarios described above.
Labels and Review