CCCT-2002 Opportunity Card Redesign #3473
Conversation
Added a new warning icon for card messages.
Replaced message text sizes with dimens values.
Moved the job completion date to underneath the job title.
Reformatted the job completion date if its delivery is complete.
Hid warning icon if job is ready to transition to delivery.
Added non-english translations for the job completion date.
Added view info and resume buttons.
Added string resources, including translations, for the view info and resume buttons.
Hid some text (by default) on the learning progress screen that would always flash in the UI causing poor UX.
Fixed up some styling on the delivery progress page.
conroy-ricketts
requested review from
Jignesh-dimagi,
OrangeAndGreen,
jaypanchal-13 and
shubham1g5
📝 WalkthroughWalkthroughThis PR adds a warning icon to the Connect feature's job message display and introduces new navigation functionality. A new vector drawable asset is created, and multiple layout files are updated to include the warning icon with proper constraints and spacing. Three new string resources are added across 9 language variants for job completion dates and action buttons. Java and Kotlin code changes wire up icon visibility toggling based on job state, add a new date formatter for completed jobs, and refactor navigation paths to launch job detail views and resume jobs via the delivery and learning apps. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
app/res/layout/home_screen.xml (1)
66-76: Warning icon integration in home card is structurally soundThe new
ivConnectMessageWarningIconis correctly constrained andtvConnectMessagehas been reflowed to sit to its right with a dimension-based text size, which should make the warning state visually clear without affecting other cards.If you touch this area again, consider switching the
layout_constraintLeft/Rightattributes toStart/Endfor better RTL behavior, to keep it in line with newer layout conventions.Also applies to: 77-88
app/res/layout/fragment_connect_delivery_progress.xml (1)
6-7: Delivery progress message card updates are consistent and safeUsing
@color/connect_background_colorremoves hard-coded colors, the increased bottom margin oncvConnectMessageshould reduce crowding, and the new warning icon plus adjustedtvConnectMessageconstraints/text size mirror the home-screen treatment. Layout and constraints look correct.As with the home screen, you could eventually migrate
Left/Rightconstraints here toStart/Endfor improved RTL support, but it’s not blocking.Also applies to: 17-18, 102-111, 124-146
app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.java (1)
168-179: Consider extracting shared navigation and formatting logic.Both
ConnectDeliveryProgressFragmentandConnectLearningProgressFragmentcontain nearly identical date formatting logic (lines 168-179) and similar navigation methods (navigateToDeliverAppHomevsnavigateToLearnAppHome). Consider extracting this shared logic to reduce duplication:
- The date formatting pattern could be moved to a shared helper method or the base
ConnectJobFragmentclass.- The navigation methods could be consolidated into a single parameterized method.
This would improve maintainability and reduce the risk of divergence between the two fragments.
Also applies to: 269-273
app/src/org/commcare/fragments/connect/ConnectLearningProgressFragment.java (1)
12-12: Remove unused import.The
androidx.annotation.StringResimport on line 12 appears to be unused in this file.🔎 Proposed fix
-import androidx.annotation.StringRes;
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
app/res/drawable/ic_connect_warning.xmlapp/res/layout-land/home_screen.xmlapp/res/layout/fragment_connect_delivery_progress.xmlapp/res/layout/fragment_connect_learning_progress.xmlapp/res/layout/home_screen.xmlapp/res/layout/view_job_card.xmlapp/res/values-es/strings.xmlapp/res/values-fr/strings.xmlapp/res/values-hi/strings.xmlapp/res/values-lt/strings.xmlapp/res/values-no/strings.xmlapp/res/values-pt/strings.xmlapp/res/values-sw/strings.xmlapp/res/values-ti/strings.xmlapp/res/values/strings.xmlapp/src/org/commcare/activities/StandardHomeActivityUIController.javaapp/src/org/commcare/connect/ConnectDateUtils.ktapp/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.javaapp/src/org/commcare/fragments/connect/ConnectLearningProgressFragment.java
🧰 Additional context used
🧠 Learnings (18)
📓 Common learnings
Learnt from: shubham1g5
Repo: dimagi/commcare-android PR: 0
File: :0-0
Timestamp: 2025-05-08T11:08:18.530Z
Learning: PR #3048 "Phase 4 Connect PR" introduces a substantial feature called "Connect" to the CommCare Android app, which includes messaging, job management, delivery tracking, payment processing, authentication flows, and learning modules. It follows a modern architecture using Navigation Components with three navigation graphs, segregated business logic in Manager classes, and proper database persistence.
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3248
File: app/src/org/commcare/connect/MessageManager.java:0-0
Timestamp: 2025-07-29T14:54:47.940Z
Learning: User OrangeAndGreen organizes messaging-related changes into separate PRs rather than mixing them with other Connect work, which aligns with their preference for handling code issues in separate PRs when they are not directly related to the main changes.
📚 Learning: 2025-05-22T14:32:53.133Z
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3121
File: app/res/values-ti/strings.xml:350-350
Timestamp: 2025-05-22T14:32:53.133Z
Learning: PersonalID and Connect features haven't been translated to Spanish, Lithuanian, or Norwegian yet, so users with those language settings see the English strings by default.
Applied to files:
app/res/values-no/strings.xmlapp/res/values-es/strings.xml
📚 Learning: 2025-01-26T19:02:16.066Z
Learnt from: pm-dimagi
Repo: dimagi/commcare-android PR: 2847
File: app/src/org/commcare/android/database/connect/models/ConnectJobRecordV2.java:120-121
Timestamp: 2025-01-26T19:02:16.066Z
Learning: The date fields (projectEndDate, lastUpdate, lastLearnUpdate, lastDeliveryUpdate) in ConnectJobRecordV2 and its newer versions are guaranteed to be non-null through the application's initialization logic and data flow. The newer version (ConnectJobRecord) explicitly initializes these in the constructor, while V2 maintains this guarantee through database upgrades and JSON parsing.
Applied to files:
app/src/org/commcare/connect/ConnectDateUtils.kt
📚 Learning: 2025-01-26T19:02:16.066Z
Learnt from: pm-dimagi
Repo: dimagi/commcare-android PR: 2847
File: app/src/org/commcare/android/database/connect/models/ConnectJobRecordV2.java:120-121
Timestamp: 2025-01-26T19:02:16.066Z
Learning: The date fields in ConnectJobRecordV2 (projectEndDate, lastUpdate, lastLearnUpdate, lastDeliveryUpdate) are guaranteed to be non-null as part of the core database model design, despite not having explicit initialization in the constructor.
Applied to files:
app/src/org/commcare/connect/ConnectDateUtils.kt
📚 Learning: 2025-05-08T11:08:18.530Z
Learnt from: shubham1g5
Repo: dimagi/commcare-android PR: 0
File: :0-0
Timestamp: 2025-05-08T11:08:18.530Z
Learning: PR #3048 "Phase 4 Connect PR" introduces a substantial feature called "Connect" to the CommCare Android app, which includes messaging, job management, delivery tracking, payment processing, authentication flows, and learning modules. It follows a modern architecture using Navigation Components with three navigation graphs, segregated business logic in Manager classes, and proper database persistence.
Applied to files:
app/src/org/commcare/connect/ConnectDateUtils.ktapp/res/layout/view_job_card.xmlapp/src/org/commcare/activities/StandardHomeActivityUIController.javaapp/res/layout/home_screen.xmlapp/res/values-pt/strings.xmlapp/res/values-es/strings.xmlapp/res/values/strings.xmlapp/res/layout-land/home_screen.xmlapp/res/layout/fragment_connect_delivery_progress.xmlapp/res/values-hi/strings.xmlapp/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.javaapp/src/org/commcare/fragments/connect/ConnectLearningProgressFragment.java
📚 Learning: 2025-12-10T16:39:05.007Z
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3448
File: app/src/org/commcare/gis/EntityMapUtils.kt:214-234
Timestamp: 2025-12-10T16:39:05.007Z
Learning: In Kotlin files (here: app/src/org/commcare/gis/EntityMapUtils.kt), follow fail-fast guidance: treat conditions that indicate a programmer error or broken invariant as exceptions rather than defensive handling. If a condition represents a bug (e.g., internal API assumptions like array/list size mismatches), crash early to alert developers instead of attempting to recover. Use clear runtime exceptions (e.g., IllegalStateException, IllegalArgumentException) or assertions where appropriate, and avoid swallowing or masking bugs. Ensure invariants are checked and failures are surfaced with actionable messages for quick debugging.
Applied to files:
app/src/org/commcare/connect/ConnectDateUtils.kt
📚 Learning: 2025-05-22T14:28:35.959Z
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3121
File: app/src/org/commcare/activities/CommCareSetupActivity.java:360-364
Timestamp: 2025-05-22T14:28:35.959Z
Learning: In CommCareSetupActivity.java, the call to installFragment.showConnectErrorMessage() after fragment transactions is intentionally unguarded with null checks. This follows the app's design pattern where critical error paths prefer immediate crashes over silent failures, making potential issues immediately visible during development rather than hiding them with defensive programming.
Applied to files:
app/src/org/commcare/activities/StandardHomeActivityUIController.javaapp/res/layout/fragment_connect_delivery_progress.xmlapp/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.javaapp/src/org/commcare/fragments/connect/ConnectLearningProgressFragment.java
📚 Learning: 2025-05-22T14:26:41.341Z
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3121
File: app/src/org/commcare/fragments/SelectInstallModeFragment.java:201-205
Timestamp: 2025-05-22T14:26:41.341Z
Learning: In SelectInstallModeFragment.java, the showConnectErrorMessage method intentionally omits null checks because it's called at a specific point in the startup flow where UI is guaranteed to be loaded. It's designed to crash if activity or view is null to make potential issues immediately visible rather than hiding them with defensive programming.
Applied to files:
app/src/org/commcare/activities/StandardHomeActivityUIController.java
📚 Learning: 2025-05-09T10:57:41.073Z
Learnt from: Jignesh-dimagi
Repo: dimagi/commcare-android PR: 3093
File: app/res/navigation/nav_graph_connect_messaging.xml:41-45
Timestamp: 2025-05-09T10:57:41.073Z
Learning: In the CommCare Android codebase, the navigation graph for Connect messaging (`nav_graph_connect_messaging.xml`) intentionally uses `channel_id` as the argument name in the connectMessageFragment, despite using `channelId` in other parts of the same navigation graph. This naming difference is by design in the refactored code.
Applied to files:
app/res/layout/home_screen.xmlapp/res/layout-land/home_screen.xmlapp/res/layout/fragment_connect_delivery_progress.xmlapp/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.javaapp/src/org/commcare/fragments/connect/ConnectLearningProgressFragment.java
📚 Learning: 2025-05-09T13:45:18.661Z
Learnt from: Jignesh-dimagi
Repo: dimagi/commcare-android PR: 3093
File: app/res/layout/activity_connect_messaging.xml:0-0
Timestamp: 2025-05-09T13:45:18.661Z
Learning: In Android layouts, use `layout_constraintStart_toStartOf` and `layout_constraintEnd_toEndOf` instead of the deprecated `layout_constraintLeft_toLeftOf` and `layout_constraintRight_toRightOf` to ensure proper RTL (Right-to-Left) layout support.
Applied to files:
app/res/layout-land/home_screen.xml
📚 Learning: 2025-04-18T20:13:29.655Z
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3040
File: app/src/org/commcare/android/database/connect/models/ConnectJobDeliveryFlagRecord.java:39-55
Timestamp: 2025-04-18T20:13:29.655Z
Learning: In the CommCare Android Connect feature, the JSON object passed to `ConnectJobDeliveryFlagRecord.fromJson()` method should never be null based on the implementation design.
Applied to files:
app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.java
📚 Learning: 2025-06-06T19:54:26.428Z
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3108
File: app/src/org/commcare/fragments/connect/ConnectDownloadingFragment.java:74-78
Timestamp: 2025-06-06T19:54:26.428Z
Learning: In ConnectDownloadingFragment.java and similar Connect-related code, the team prefers to let "should never happen" scenarios like null app records crash rather than add defensive null checks, following a fail-fast philosophy to catch programming errors during development.
Applied to files:
app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.javaapp/src/org/commcare/fragments/connect/ConnectLearningProgressFragment.java
📚 Learning: 2025-06-20T15:51:14.157Z
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3108
File: app/src/org/commcare/models/connect/ConnectLoginJobListModel.java:79-92
Timestamp: 2025-06-20T15:51:14.157Z
Learning: The ConnectLoginJobListModel class in app/src/org/commcare/models/connect/ConnectLoginJobListModel.java does not need to implement Parcelable interface as it is not passed between Android activities or fragments.
Applied to files:
app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.javaapp/src/org/commcare/fragments/connect/ConnectLearningProgressFragment.java
📚 Learning: 2025-06-04T19:17:21.213Z
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3108
File: app/src/org/commcare/fragments/connect/ConnectUnlockFragment.java:62-64
Timestamp: 2025-06-04T19:17:21.213Z
Learning: In ConnectUnlockFragment.java, the user prefers to let getArguments() potentially throw NullPointerException rather than adding null checks, as the arguments are required for proper navigation flow and their absence indicates a programming error that should fail fast.
Applied to files:
app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.javaapp/src/org/commcare/fragments/connect/ConnectLearningProgressFragment.java
📚 Learning: 2025-06-06T20:15:21.134Z
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3108
File: app/src/org/commcare/fragments/connect/ConnectJobIntroFragment.java:65-71
Timestamp: 2025-06-06T20:15:21.134Z
Learning: In the CommCare Android Connect module, job.getLearnAppInfo() and getLearnModules() should never return null according to the system design. The team prefers to let the code crash if these values are unexpectedly null rather than adding defensive null checks, following a fail-fast philosophy to catch bugs early rather than masking them.
Applied to files:
app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.javaapp/src/org/commcare/fragments/connect/ConnectLearningProgressFragment.java
📚 Learning: 2025-02-19T15:15:01.935Z
Learnt from: pm-dimagi
Repo: dimagi/commcare-android PR: 2956
File: app/src/org/commcare/fragments/connectMessaging/ConnectMessageChannelListFragment.java:58-60
Timestamp: 2025-02-19T15:15:01.935Z
Learning: Error handling for message retrieval in ConnectMessageChannelListFragment's retrieveMessages callback is not required as per user preference.
Applied to files:
app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.java
📚 Learning: 2025-07-29T14:14:07.954Z
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3248
File: app/src/org/commcare/adapters/ConnectMessageAdapter.java:0-0
Timestamp: 2025-07-29T14:14:07.954Z
Learning: In the CommCare Android Connect messaging system (ConnectMessageAdapter.java), the team prefers to let getMessage() potentially return null and crash rather than adding defensive null checks. This follows the fail-fast philosophy used throughout the Connect feature to catch programming errors immediately during development.
Applied to files:
app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.java
📚 Learning: 2025-01-21T17:27:58.754Z
Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 2912
File: app/src/org/commcare/android/database/connect/models/ConnectUserRecord.java:81-89
Timestamp: 2025-01-21T17:27:58.754Z
Learning: In the CommCare Android codebase, use org.jetbrains.annotations.NotNull for null-safety annotations. Empty strings are acceptable for unspecified values, but null values should be prevented using NotNull.
Applied to files:
app/src/org/commcare/fragments/connect/ConnectLearningProgressFragment.java
🧬 Code graph analysis (2)
app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.java (3)
app/src/org/commcare/activities/connect/ConnectActivity.java (1)
ConnectActivity(59-296)app/src/org/commcare/connect/ConnectDateUtils.kt (2)
formatDateForCompletedJob(26-30)formatDate(17-21)app/src/org/commcare/connect/ConnectAppUtils.kt (1)
launchApp(117-125)
app/src/org/commcare/fragments/connect/ConnectLearningProgressFragment.java (3)
app/src/org/commcare/connect/ConnectDateUtils.kt (2)
formatDateForCompletedJob(26-30)formatDate(17-21)app/src/org/commcare/CommCareApplication.java (1)
CommCareApplication(158-1255)app/src/org/commcare/connect/ConnectAppUtils.kt (1)
launchApp(117-125)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Lint Code Base
🔇 Additional comments (23)
app/res/values-no/strings.xml (1)
136-138: Norwegian translations look good; request QA verification.The three new string resources are properly formatted with correct XML syntax and appropriate use of the format placeholder in
connect_job_ended.Per the PR objectives' safety/QA notes, please ensure translation verification is completed for these Norwegian strings.
app/res/values-hi/strings.xml (1)
311-311: Hindi translations are properly formatted; request QA verification.The string additions are technically correct with proper XML encoding and appropriate format placeholder usage in
connect_job_ended.Per the PR objectives' safety/QA notes, please ensure translation verification is completed for these Hindi strings.
Also applies to: 390-391
app/res/values-fr/strings.xml (1)
311-311: French translations are well-formed; request QA verification.The string resources are properly formatted with correct French accents and natural phrasing (e.g., "terminée le %s"). XML encoding is correct.
Per the PR objectives' safety/QA notes, please ensure translation verification is completed for these French strings.
Also applies to: 391-392
app/res/values-lt/strings.xml (1)
136-138: Lithuanian translations are correctly formatted; request QA verification.The string additions are technically sound with proper encoding of Lithuanian special characters (ė, ū, ž) and correct format placeholder usage.
Per the PR objectives' safety/QA notes, please ensure translation verification is completed for these Lithuanian strings.
app/res/drawable/ic_connect_warning.xml (1)
1-13: LGTM! Warning icon drawable is properly structured.The vector drawable follows Android standards with correct dimensions, viewport, and path data. The dark fill color (#1F2937) is appropriate for a warning icon.
app/res/layout/fragment_connect_learning_progress.xml (1)
314-314: LGTM! Visibility change aligns with PR objectives.Setting
android:visibility="gone"onconnect_learning_ended_textis consistent with the PR objective to hide flashing text on the learning progress screen. The end-state messaging is now handled through other UI elements as described in the PR summary.app/res/layout-land/home_screen.xml (1)
67-76: LGTM! Warning icon properly added.The new
ivConnectMessageWarningIconImageView is correctly positioned with appropriate constraints and padding. ThecontentDescription="@null"is acceptable for this decorative warning indicator that accompanies explanatory text.app/src/org/commcare/connect/ConnectDateUtils.kt (1)
23-30: LGTM! Date formatter follows established patterns.The new
completedJobDateFormatandformatDateForCompletedJobmethod are implemented correctly:
- Proper thread-safety with synchronized block (required for SimpleDateFormat)
- Consistent with existing formatters in the file
- Clear date pattern ("dd MMM, yyyy") appropriate for display
- Descriptive naming
app/src/org/commcare/activities/StandardHomeActivityUIController.java (1)
49-50: Warning icon wiring and visibility logic look solidThe new
connectMessageWarningIconis correctly initialized from the home screen layout and its visibility is kept in sync with the message card: shown only when there is a message and the job is not ready to transition to delivery, and hidden otherwise. No correctness issues spotted.Also applies to: 82-86, 152-158
app/res/values-ti/strings.xml (1)
304-305: New Connect strings are format-safe and consistent
connect_job_endedpreserves the single%splaceholder, and the new button labels use the expected keys and structure. Looks good from a resource/format perspective.Also applies to: 393-395
app/res/values-sw/strings.xml (1)
314-315: Swahili translations align with base keys and formattingThe added Swahili strings map 1:1 to the new Connect keys and keep the single
%splaceholder where required. No issues.Also applies to: 394-395
app/res/values-es/strings.xml (1)
249-250: Spanish Connect strings are correctly localized and keyed
connect_job_endedand the “Ver información” / “Reanudar” labels map cleanly to the new UI behavior and keep placeholders consistent with the default resources.Also applies to: 393-394
app/res/values/strings.xml (1)
415-416: New base strings correctly support the redesigned cards
connect_job_endedcleanly expresses the completed-date message with a single%s, and the “View Info” / “Resume” labels match the new buttons on the opportunity card. No problems here.Also applies to: 572-573
app/res/values-pt/strings.xml (1)
320-321: Portuguese Connect strings are correctly added and formattedThe three new keys tie into the Connect job-completion and action-button flows, with placeholder usage matching the default locale. No concerns.
Also applies to: 410-411
app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.java (5)
161-164: LGTM!The new button click handlers are properly wired up. The
mbViewInfonavigates to the job detail bottom sheet, andmbResumetriggers the navigation to the deliver app home.
168-179: Date formatting logic is clear and correct.The conditional formatting based on
deliveryComplete()appropriately uses different string resources and date formatters. The completed job usesformatDateForCompletedJobwhile in-progress jobs use the standardformatDate.
187-192: UI visibility updates are well-coordinated.The visibility changes properly hide the old UI elements (
tvJobDescription,connectJobEndDate,tvViewMore) and show the new ones (connectJobEndDateSubHeading,mbViewInfo,mbResume). This ensures a clean transition in the redesign.
230-230: Warning icon visibility logic is correct.The warning icon is shown when a card message exists and hidden otherwise. This provides a clear visual indicator for users.
Also applies to: 233-233
269-273: Navigation logic follows established patterns.The
navigateToDeliverAppHome()method correctly closes the user session and launches the deliver app usingConnectAppUtils.launchApp(). Note thatjob.getDeliveryAppInfo().getAppId()is called without null checks, following the fail-fast philosophy used throughout the Connect feature.app/src/org/commcare/fragments/connect/ConnectLearningProgressFragment.java (4)
182-182: Button click handlers properly updated.The click handlers for the assessment and learning buttons now correctly delegate to
navigateToLearnAppHome(), providing a consistent navigation experience.Also applies to: 187-187
241-252: Date formatting logic is correct and consistent.The conditional formatting based on
deliveryComplete()matches the pattern used inConnectDeliveryProgressFragment, ensuring consistent date display across both learning and delivery contexts.
260-267: UI visibility and click handler setup looks good.The visibility changes and click handler assignments properly integrate the new card design. The
mbViewInfobutton navigates to the job detail bottom sheet, whilembResumelaunches the learn app.
279-282: Navigation method correctly implemented.The
navigateToLearnAppHome()method follows the same pattern as the delivery fragment, closing the user session and launching the learn app. The call tojob.getLearnAppInfo().getAppId()is intentionally unguarded, following the fail-fast philosophy used throughout the Connect feature.Based on learnings, the team prefers to let null values crash rather than add defensive checks in the Connect module.
Made some adjustments in XML layouts for RTL support.
Adjusted width of buttons on the job card to support longer non-English translations.
Made some additional adjustments in XML layouts for RTL support.
Made some more additional adjustments in XML layouts for RTL support.
Optimized imports.
Checked for if app is installed for resume buttons.
OrangeAndGreen
left a comment
OrangeAndGreen
left a comment
There was a problem hiding this comment.
Nice changes! All looks pretty good to me, left one nit but nothing blocking.
app/src/org/commcare/fragments/connect/ConnectLearningProgressFragment.java
Show resolved
Hide resolved
shubham1g5
left a comment
shubham1g5
left a comment
There was a problem hiding this comment.
Thanks for the great description on the PR.
The new redesign has us move the opportunity completion date to a different location on the card, so I added a 2nd text view element rather than deleting the old one so that we don't mess up the UI on the app home screens. That's why you see both connect_job_end_date and connect_job_end_date_sub_heading in view_job_card.xml.
That's also why I added a new utility function for getting the new date format in ConnectDateUtils.kt rather than tweaking the original utility function.
Given the 2 components are quite similar here, I think it's alright for changes in one widget to reflect into other, i.e. I think we can use the same Ui components for date here to favor consistency between these widgets.
| connectMessageWarningIcon.setVisibility( | ||
| job.readyToTransitionToDelivery() ? View.GONE : View.VISIBLE | ||
| ); |
There was a problem hiding this comment.
Should it be job.readyToTransitionToDelivery() || job.deliveryComplete() , I see that we only set warning UI state above for this condition. If so, it would be nice to have a variable like isWarning based on which we set the visibility here so that we don't duplicate logic to show warning in 2 different places inside same method.
There was a problem hiding this comment.
The reason for not including job.deliveryComplete() is because we want to hide the icon only when the message looks something like this (I assume):
On the other hand, I like the idea of isWarning!
There was a problem hiding this comment.
Ah, on second look, adding a isWarning variable may make things unnecessarily complex unless I'm missing something. There's essentially 3 different states we need to worry about:
- Green background & no warning icon (
job.readyToTransitionToDelivery()) - Blue background & warning icon (
job.deliveryComplete()) - Orange background & warning icon (all other scenarios - a catch-all)
There was a problem hiding this comment.
that makes sense to me, and fine to keep it as it is. Although I would slightly prefer for this logic to be part of the above if-else logic so that all the UI state calculation is part of single logic flow.
There was a problem hiding this comment.
Ah I see what you mean. I agree!
@shubham1g5 I'm a bit weary of doing big changes to other connect job cards. For example, it feels weird replacing the job's description on a learn app home page with the job's completion date: 
Unless what you mean is moving the completion date to above the description? |
Refactored code to use one version of date format.
Added back in some date formatting logic that was removed by mistake.
Added similar date formatting logic to app home page.
|
@conroy-ricketts I didn't realise there is an additional description there which is not present on the progress page cards, I was hoping we can standardize the 2 views better to make them more consistent with each other. I am not sure why we chose to show description on one card and not on another and think it might just be something that got overlooked by us and product. So think it's good to flag this to product but otherwise I am happy with the current state based on this PR. |
Moved logic for setting visibility of the warning icon on the app home pages to the state conditionals.
Ran Ktlint on ConnectDateUtils. Renamed ignored exception to _.
OrangeAndGreen
left a comment
OrangeAndGreen
left a comment
There was a problem hiding this comment.
I added a couple thoughts but nothing blocking
app/src/org/commcare/activities/StandardHomeActivityUIController.java
Outdated
Show resolved
Hide resolved
OrangeAndGreen
left a comment
OrangeAndGreen
left a comment
There was a problem hiding this comment.
Meant to approve...
Did some minor code reformatting in StandardHomeActivityUIController.
…nto story/CCCT-2002-opportunity-card-redesign
|
@damagatchi retest this please |
…nto story/CCCT-2002-opportunity-card-redesign
|
@shubham1g5 @OrangeAndGreen Update from Product: they want us to keep the UI changes here as is and revisit once these changes go live to see how things are going. I'll merge this in once it gets another approval |
CCCT-2002
Product Description
I redesigned the connect cards in the opportunity progress pages.
Here's some screenshots of the card in various different states:
Here's a video demonstrating the behavior of the new buttons:
Screen_Recording_20251226_143659_CommCare.Debug.mp4
Technical Summary
android:visibility="gone"by default so that we don't mess up the UI on other screens.connect_job_end_dateandconnect_job_end_date_sub_headinginview_job_card.xml.ConnectDateUtils.ktrather than tweaking the original utility function.Safety Assurance
Safety story
QA Plan