Redesign opportunity card on connect landing page #3484
Conversation
jaypanchal-13
requested review from
Jignesh-dimagi,
OrangeAndGreen,
conroy-ricketts and
shubham1g5
📝 WalkthroughWalkthroughThis pull request redesigns the Connect job list item UI by replacing a CircleProgressBar component with a composite progress indicator arrangement (ProgressBar + ImageView + TextView), adding two new action buttons (Resume and View Info), and updating the job-type display to show expiration dates with visual warnings. The adapter logic is updated to handle date proximity checks, conditional progress visibility, simplified job-type rendering, and revised click-binding targets. Additionally, three new UI strings and three new color resources are introduced, with translations added across eight language locales. Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app/res/values-lt/strings.xml (2)
77-89: Critical: Incorrect language content detected.Lines 77-89 contain Italian text instead of Lithuanian translations (e.g., "Accedi / Registrati", "Aiuto", "Non hai effettuato l'accesso all\ID personale"). This appears to be a copy-paste error from the Italian locale file.
🔍 Verification: Find all mixed-language occurrences
#!/bin/bash # Search for known Italian phrases in Lithuanian locale file rg -n "Accedi|Registrati|Aiuto|Questa app|effettuato" app/res/values-lt/strings.xml
102-102: Critical: Italian text in Lithuanian locale.Line 102 contains Italian text: "Questa app non è aggiornata. Per procedere, aggiorna l'app sul Google Play Store." This should be translated to Lithuanian.
app/src/org/commcare/adapters/JobListConnectHomeAppsAdapter.java (1)
196-205: Missing click listener forbtn_view_infobutton.The layout defines a
btn_view_infobutton, but no click listener is attached here. Users will see the button but tapping it will do nothing.Do you want me to help implement the click listener for the "View Info" button?
🤖 Fix all issues with AI agents
In @app/res/layout/connect_job_list_item.xml:
- Around line 129-138: The ImageView with id imgJobType is using scalable text
units (30sp) for layout_width and layout_height; update those attributes to use
density pixels (30dp) instead (replace "30sp" with "30dp") so the image
dimensions remain consistent regardless of user font-size settings; locate the
ImageView tag with android:id="@+id/imgJobType" and change its layout_width and
layout_height attributes accordingly.
- Around line 61-71: tvDate is declared inside llOpportunity (a LinearLayout)
but has
app:layout_constraintTop_toTopOf/app:layout_constraintBottom_toBottomOf/app:layout_constraintEnd_toEndOf
attributes which are ignored; remove those app:layout_constraint* attributes
from the TextView (id tvDate) and, if alignment is required, replace them with
appropriate LinearLayout parameters such as android:layout_gravity,
android:gravity, or proper layout_margin* values, or alternatively move tvDate
into a ConstraintLayout if you intend to use constraints.
In @app/src/org/commcare/adapters/JobListConnectHomeAppsAdapter.java:
- Around line 241-261: The crash is caused by passing wrong arguments into
setJobType (signature: setJobType(Context,int iconResId,int
iconAlpha,ConnectJobListItemBinding)): replace the hardcoded 255 used as
iconResId for item.isNew() with a proper drawable resource (e.g.
R.drawable.ic_connect_new or another appropriate job icon) and pass 255 as the
iconAlpha; for the delivery branch do not pass a color resource as the icon
alpha—compute an iconAlpha (e.g. int iconAlpha = finished ? 128 : 255) and call
setJobType(context, iconId, iconAlpha, binding), and separately apply the text
color via binding (e.g.
binding.<textView>.setTextColor(ContextCompat.getColor(context, textColorId)));
keep learning branch as-is.
🧹 Nitpick comments (3)
app/res/values-es/strings.xml (1)
542-545: Spanish additions are correct; optional wording tweak onlyKeys and
%susage are correct and consistent with other locales, and this helps close earlier gaps where Spanish PersonalID strings fell back to English. If you ever revisit copy, you might considerCompletar antes del %sas a slightly more natural phrasing, but the current text is perfectly serviceable.Based on learnings, this also improves the localized Connect/PersonalID experience for Spanish users.
app/src/org/commcare/adapters/JobListConnectHomeAppsAdapter.java (1)
142-145: Consider reusingConnectDateUtils.formatDate()instead of duplicating.There's already a
formatDateutility inConnectDateUtils.kt(from relevant code snippets). Using it would reduce duplication and ensure consistent date formatting across the codebase.app/res/layout/connect_job_list_item.xml (1)
73-85: Redundantclickableandfocusableattributes on button.
AppCompatButtonis clickable and focusable by default. Lines 79-80 can be removed.🧹 Proposed cleanup
<androidx.appcompat.widget.AppCompatButton android:layout_width="wrap_content" android:layout_height="30dp" android:id="@+id/btn_resume" android:textSize="11sp" android:text="@string/personalid_resume" - android:clickable="true" - android:focusable="true" android:layout_marginTop="10dp"
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
app/res/drawable/bg_rounded_corner_lavender_70.xmlapp/res/layout/connect_job_list_item.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/colors.xmlapp/res/values/strings.xmlapp/src/org/commcare/adapters/JobListConnectHomeAppsAdapter.java
🧰 Additional context used
🧠 Learnings (5)
📓 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.
📚 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-es/strings.xmlapp/res/values-pt/strings.xml
📚 Learning: 2025-05-07T06:33:26.714Z
Learnt from: Jignesh-dimagi
Repo: dimagi/commcare-android PR: 3070
File: app/res/drawable/shape_connect_delivery_pending.xml:1-5
Timestamp: 2025-05-07T06:33:26.714Z
Learning: For drawables in the CommCare Android codebase, the preferred approach is to let consuming layouts explicitly specify required width and height rather than defining intrinsic sizes within shape drawables.
Applied to files:
app/res/drawable/bg_rounded_corner_lavender_70.xml
📚 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/adapters/JobListConnectHomeAppsAdapter.javaapp/res/layout/connect_job_list_item.xml
📚 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/res/layout/connect_job_list_item.xml
🧬 Code graph analysis (1)
app/src/org/commcare/adapters/JobListConnectHomeAppsAdapter.java (1)
app/src/org/commcare/connect/ConnectDateUtils.kt (1)
formatDate(16-20)
⏰ 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 (12)
app/res/values-ti/strings.xml (1)
533-536: Tigrinya strings: keys and placeholders look correctThe three new keys are well-named, match the existing PersonalID namespace, and the
%splaceholder usage aligns with the expected "complete by" pattern. No structural or i18n issues from the resource side.app/res/values-hi/strings.xml (1)
540-543: Hindi localization consistent with PersonalID usage
personalid_resume,personalid_view_info, andpersonalid_complete_bymirror the English intent and keep a single%splaceholder for date insertion, matching how the adapter will format this string.app/res/values-sw/strings.xml (1)
550-553: Swahili strings: correct placeholder usage and toneThe new Swahili labels ("Endelea", "Angalia maelezo", "Kamilisha ifikapo %s") fit prior terminology and preserve the single
%splaceholder expected by the calling code.app/res/values-fr/strings.xml (1)
545-548: French strings align with expected semantics and format
personalid_resume,personalid_view_info, andpersonalid_complete_byare idiomatic, andÀ compléter d'ici le %scorrectly models the "complete by" pattern with a single%splaceholder as required by the adapter.app/res/values-lt/strings.xml (1)
150-152: LGTM! New string resources added correctly.The three new Lithuanian translations for personalid_resume, personalid_view_info, and personalid_complete_by are properly formatted and consistent with the string additions in other locale files. The format specifier %s in personalid_complete_by is correctly placed for date string substitution.
app/res/drawable/bg_rounded_corner_lavender_70.xml (1)
1-16: LGTM! Drawable follows best practices.The shape drawable correctly omits intrinsic width/height, allowing consuming layouts to control sizing. The 70dp corner radius will create a heavily rounded or circular appearance depending on the element dimensions.
Based on learnings, this follows the preferred pattern of letting layouts specify required dimensions.
app/res/values/strings.xml (1)
720-722: LGTM! Base string resources added correctly.The three new English string resources follow the existing naming convention and support the redesigned opportunity card UI. The format specifier %s in personalid_complete_by is appropriate for date string substitution.
app/res/values-no/strings.xml (1)
150-152: LGTM! Norwegian translations added correctly.The three new Norwegian string resources are properly formatted and consistent with the string additions across other locales. The format specifier %s in personalid_complete_by is correctly positioned for date substitution.
app/res/values/colors.xml (1)
227-229: Color values are unique with no duplicates.The three new color resources are properly named and formatted. Each color value (#B3261E, #E7EAF7, #AFAFAF) appears exactly once in the file with no duplicates.
app/res/values-pt/strings.xml (1)
550-553: LGTM!The Portuguese translations are correct and the format string placeholder
%sfor the date is properly used.app/res/layout/connect_job_list_item.xml (1)
113-157: LGTM on progress area layout structure.Good use of
Group(line 153-157) to manage visibility of related progress elements together. The guideline-based layout organization is clean.app/src/org/commcare/adapters/JobListConnectHomeAppsAdapter.java (1)
161-170: Potential NPE ifgetDate()returns null.
shouldShowDateInRedis called without null-checkingconnectLoginJobListModel.getDate(). If the date is null, this will cause aNullPointerExceptionwhen accessingexpiryDate.getTime().🐛 Proposed fix
- private boolean shouldShowDateInRed(Date expiryDate) { + private boolean shouldShowDateInRed(Date expiryDate) { + if (expiryDate == null) { + return false; + } long now = System.currentTimeMillis(); long diffMillis = expiryDate.getTime() - now; long daysRemaining = TimeUnit.MILLISECONDS.toDays(diffMillis); return daysRemaining <= 5; }Also guard the date text binding:
+ Date date = connectLoginJobListModel.getDate(); - if (shouldShowDateInRed(connectLoginJobListModel.getDate())) { + if (date != null && shouldShowDateInRed(date)) { binding.tvDate.setTextColor( ContextCompat.getColor(mContext, R.color.dark_red_brick_red) ); binding.ivInfo.setColorFilter( ContextCompat.getColor(mContext, R.color.dark_red_brick_red), PorterDuff.Mode.SRC_IN ); } - binding.tvDate.setText(mContext.getString(R.string.personalid_complete_by,formatDate(connectLoginJobListModel.getDate()))); + binding.tvDate.setText(date != null + ? mContext.getString(R.string.personalid_complete_by, formatDate(date)) + : "");Also applies to: 178-185
⛔ Skipped due to learnings
Learnt from: conroy-ricketts Repo: dimagi/commcare-android PR: 3477 File: app/src/org/commcare/fragments/connect/ConnectResultsSummaryListFragment.java:265-284 Timestamp: 2026-01-02T16:07:46.905Z Learning: In ConnectJobPaymentRecord and related Connect payment sorting code, the date field returned by getDate() is guaranteed to be non-null through the application's initialization logic and data flow. Following the fail-fast philosophy used throughout the Connect feature, null pointer exceptions from date comparisons should be allowed to crash rather than adding defensive null checks, as this helps catch programming errors during development.
| <TextView | ||
| android:id="@+id/tvJobType" | ||
| android:id="@+id/tvDate" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_marginStart="4dp" | ||
| android:layout_marginEnd="10dp" | ||
| android:textColor="@color/connect_yellowish_orange_color" | ||
| android:textSize="12sp" | ||
| tools:text="@string/connect_new_opportunity" | ||
| android:textStyle="bold"/> | ||
| android:layout_marginStart="4dp" | ||
| android:fontFamily="sans-serif" | ||
| app:layout_constraintBottom_toBottomOf="@+id/llOpportunity" | ||
| app:layout_constraintEnd_toEndOf="parent" | ||
| app:layout_constraintTop_toTopOf="@+id/llOpportunity" /> | ||
| </LinearLayout> |
There was a problem hiding this comment.
ConstraintLayout attributes on tvDate are ignored inside LinearLayout.
tvDate is a child of llOpportunity which is a LinearLayout. The app:layout_constraint* attributes (lines 68-70) have no effect here and should be removed to avoid confusion.
🧹 Proposed fix
<TextView
android:id="@+id/tvDate"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:textSize="12sp"
android:layout_marginStart="4dp"
- android:fontFamily="sans-serif"
- app:layout_constraintBottom_toBottomOf="@+id/llOpportunity"
- app:layout_constraintEnd_toEndOf="parent"
- app:layout_constraintTop_toTopOf="@+id/llOpportunity" />
+ android:fontFamily="sans-serif" />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <TextView | |
| android:id="@+id/tvJobType" | |
| android:id="@+id/tvDate" | |
| android:layout_width="wrap_content" | |
| android:layout_height="wrap_content" | |
| android:layout_marginStart="4dp" | |
| android:layout_marginEnd="10dp" | |
| android:textColor="@color/connect_yellowish_orange_color" | |
| android:textSize="12sp" | |
| tools:text="@string/connect_new_opportunity" | |
| android:textStyle="bold"/> | |
| android:layout_marginStart="4dp" | |
| android:fontFamily="sans-serif" | |
| app:layout_constraintBottom_toBottomOf="@+id/llOpportunity" | |
| app:layout_constraintEnd_toEndOf="parent" | |
| app:layout_constraintTop_toTopOf="@+id/llOpportunity" /> | |
| </LinearLayout> | |
| <TextView | |
| android:id="@+id/tvDate" | |
| android:layout_width="wrap_content" | |
| android:layout_height="wrap_content" | |
| android:textSize="12sp" | |
| android:layout_marginStart="4dp" | |
| android:fontFamily="sans-serif" /> | |
| </LinearLayout> |
🤖 Prompt for AI Agents
In @app/res/layout/connect_job_list_item.xml around lines 61 - 71, tvDate is
declared inside llOpportunity (a LinearLayout) but has
app:layout_constraintTop_toTopOf/app:layout_constraintBottom_toBottomOf/app:layout_constraintEnd_toEndOf
attributes which are ignored; remove those app:layout_constraint* attributes
from the TextView (id tvDate) and, if alignment is required, replace them with
appropriate LinearLayout parameters such as android:layout_gravity,
android:gravity, or proper layout_margin* values, or alternatively move tvDate
into a ConstraintLayout if you intend to use constraints.
| <ImageView | ||
| android:id="@+id/imgJobType" | ||
| android:layout_width="30sp" | ||
| android:layout_height="30sp" | ||
| android:contentDescription="@null" | ||
| android:src="@drawable/ic_connect_new_opportunity" | ||
| app:layout_constraintTop_toTopOf="@id/progressBar" | ||
| app:layout_constraintStart_toStartOf="@id/progressBar" | ||
| app:layout_constraintBottom_toBottomOf="@id/progressBar" | ||
| app:layout_constraintEnd_toEndOf="@id/progressBar"/> |
There was a problem hiding this comment.
Use dp instead of sp for image dimensions.
imgJobType uses 30sp for width and height (lines 131-132). sp units scale with the user's font size preference, which is inappropriate for images and could cause layout issues. Use dp instead.
🐛 Proposed fix
<ImageView
android:id="@+id/imgJobType"
- android:layout_width="30sp"
- android:layout_height="30sp"
+ android:layout_width="30dp"
+ android:layout_height="30dp"
android:contentDescription="@null"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <ImageView | |
| android:id="@+id/imgJobType" | |
| android:layout_width="30sp" | |
| android:layout_height="30sp" | |
| android:contentDescription="@null" | |
| android:src="@drawable/ic_connect_new_opportunity" | |
| app:layout_constraintTop_toTopOf="@id/progressBar" | |
| app:layout_constraintStart_toStartOf="@id/progressBar" | |
| app:layout_constraintBottom_toBottomOf="@id/progressBar" | |
| app:layout_constraintEnd_toEndOf="@id/progressBar"/> | |
| <ImageView | |
| android:id="@+id/imgJobType" | |
| android:layout_width="30dp" | |
| android:layout_height="30dp" | |
| android:contentDescription="@null" | |
| android:src="@drawable/ic_connect_new_opportunity" | |
| app:layout_constraintTop_toTopOf="@id/progressBar" | |
| app:layout_constraintStart_toStartOf="@id/progressBar" | |
| app:layout_constraintBottom_toBottomOf="@id/progressBar" | |
| app:layout_constraintEnd_toEndOf="@id/progressBar"/> |
🤖 Prompt for AI Agents
In @app/res/layout/connect_job_list_item.xml around lines 129 - 138, The
ImageView with id imgJobType is using scalable text units (30sp) for
layout_width and layout_height; update those attributes to use density pixels
(30dp) instead (replace "30sp" with "30dp") so the image dimensions remain
consistent regardless of user font-size settings; locate the ImageView tag with
android:id="@+id/imgJobType" and change its layout_width and layout_height
attributes accordingly.
| if (item.isNew()) { | ||
| setJobType(context, R.drawable.connect_rounded_corner_orange_yellow, | ||
| ContextCompat.getString(context, R.string.connect_new_opportunity), | ||
| R.drawable.ic_connect_new_opportunity, | ||
| 255, R.color.connect_yellowish_orange_color, binding); | ||
| setJobType(context, 255, R.color.connect_yellowish_orange_color, binding); | ||
| } else if (item.isLearningApp()) { | ||
| boolean passedAssessment = item.getJob().passedAssessment(); | ||
| int textId = passedAssessment ? R.string.connect_learn_review : R.string.connect_learn; | ||
| int textColorId = passedAssessment ? R.color.connect_blue_color_50 : R.color.connect_blue_color; | ||
| int iconAlpha = passedAssessment ? 128 : 255; | ||
| setJobType(context, R.drawable.connect_rounded_corner_tealish_blue, | ||
| ContextCompat.getString(context, textId), R.drawable.ic_connect_learning, iconAlpha, | ||
| textColorId, binding); | ||
| setJobType(context, R.drawable.ic_connect_learning, iconAlpha, binding); | ||
| } else if (item.isDeliveryApp()) { | ||
| boolean finished = item.getJob().isFinished(); | ||
| int textId = finished ? R.string.connect_expired : R.string.connect_delivery; | ||
| int textColorId = finished ? R.color.connect_middle_grey : R.color.connect_green; | ||
| int drawableId = finished ? R.drawable.connect_rounded_corner_lighter_grey | ||
| : R.drawable.connect_rounded_corner_light_green; | ||
| int iconId = finished ? R.drawable.ic_connect_expired : R.drawable.ic_connect_delivery; | ||
|
|
||
| setJobType(context, drawableId, | ||
| ContextCompat.getString(context, textId), iconId, 255, | ||
| setJobType(context, iconId, | ||
| textColorId, binding); | ||
| } | ||
| } | ||
|
|
||
| private void setJobType( | ||
| Context context, | ||
| int backgroundResId, | ||
| String jobTypeText, | ||
| int iconResId, | ||
| int iconAlpha, | ||
| int textColorResId, | ||
| ConnectJobListItemBinding binding | ||
| ) { | ||
| binding.llOpportunity.setBackground(ContextCompat.getDrawable(context, backgroundResId)); | ||
| binding.tvJobType.setText(jobTypeText); | ||
| private void setJobType(Context context, | ||
| int iconResId, int iconAlpha, ConnectJobListItemBinding binding) { | ||
| binding.imgJobType.setImageDrawable(ContextCompat.getDrawable(context, iconResId)); | ||
| binding.imgJobType.setImageAlpha(iconAlpha); | ||
| binding.tvJobType.setTextColor(ContextCompat.getColor(context, textColorResId)); | ||
| } |
There was a problem hiding this comment.
Critical: Incorrect arguments passed to setJobType causing runtime crash.
The setJobType method signature expects (Context, int iconResId, int iconAlpha, binding), but:
- Line 242: Passes
255asiconResId— this is not a valid drawable resource and will crash when callingContextCompat.getDrawable(context, 255). - Lines 252-253: Passes
textColorId(a color resource ID) asiconAlpha— should be an integer value 0-255.
The logic for "new" jobs appears incomplete compared to learning/delivery jobs.
🐛 Proposed fix
private void configureJobType(
Context context,
ConnectLoginJobListModel item,
ConnectJobListItemBinding binding
) {
if (item.isNew()) {
- setJobType(context, 255, R.color.connect_yellowish_orange_color, binding);
+ setJobType(context, R.drawable.ic_connect_new_opportunity, 255, binding);
} else if (item.isLearningApp()) {
boolean passedAssessment = item.getJob().passedAssessment();
int iconAlpha = passedAssessment ? 128 : 255;
setJobType(context, R.drawable.ic_connect_learning, iconAlpha, binding);
} else if (item.isDeliveryApp()) {
boolean finished = item.getJob().isFinished();
- int textColorId = finished ? R.color.connect_middle_grey : R.color.connect_green;
+ int iconAlpha = finished ? 128 : 255;
int iconId = finished ? R.drawable.ic_connect_expired : R.drawable.ic_connect_delivery;
- setJobType(context, iconId,
- textColorId, binding);
+ setJobType(context, iconId, iconAlpha, binding);
}
}🤖 Prompt for AI Agents
In @app/src/org/commcare/adapters/JobListConnectHomeAppsAdapter.java around
lines 241 - 261, The crash is caused by passing wrong arguments into setJobType
(signature: setJobType(Context,int iconResId,int
iconAlpha,ConnectJobListItemBinding)): replace the hardcoded 255 used as
iconResId for item.isNew() with a proper drawable resource (e.g.
R.drawable.ic_connect_new or another appropriate job icon) and pass 255 as the
iconAlpha; for the delivery branch do not pass a color resource as the icon
alpha—compute an iconAlpha (e.g. int iconAlpha = finished ? 128 : 255) and call
setJobType(context, iconId, iconAlpha, binding), and separately apply the text
color via binding (e.g.
binding.<textView>.setTextColor(ContextCompat.getColor(context, textColorId)));
keep learning branch as-is.
|
@jaypanchal-13 Over all page looks very clean and decent. Just one comment from visual appearance that in progress icon is blurry 
|
|
@jaypanchal-13 Just a question: Why StagingOpp4 is shown in both |
|
We recently implemented similar "Resume" and "View Info" buttons in #3473. My initial thought was that these buttons should be identical in style so that there is consistency across the app UI. However, I did notice that the buttons are slightly bigger in the Figma mocks with a reversed order. @shubham1g5 based on previous discussions regarding UI consistency what are your thoughts here? Should we match the same "Resume" and "View Info" buttons that were implemented in #3473? By that I mean make them the same size, same style, same order |
|
@jaypanchal-13 I also noticed that the download button is a bit blurry, and also the "resume" and "view info" buttons have a weird shadow being cut off in the background 
|
Think that's good to flag to Brendon, @jaypanchal-13 can you share the screenshot on this in the demo channel and tag Brendon with this question there. |
app/src/org/commcare/adapters/JobListConnectHomeAppsAdapter.java
Outdated
Show resolved
Hide resolved
app/src/org/commcare/adapters/JobListConnectHomeAppsAdapter.java
Outdated
Show resolved
Hide resolved
app/src/org/commcare/adapters/JobListConnectHomeAppsAdapter.java
Outdated
Show resolved
Hide resolved
…portunity-card-design
|
|
|
Ok got it. I think it's worth looking into it @conroy-ricketts as seems like bug? |
|
|
@jaypanchal-13 @Jignesh-dimagi I do not think the items in the list are duplicates. I think that fyi @shubham1g5 |
@jaypanchal-13 The image is probably blurry because we are enlarging a PNG. Is there anyway we can get a vector image asset to replace that image? That would eliminate the blur issue The same thing applies to the download icon image |
@jaypanchal-13 Thanks for flagging the issue! Can we create a new bug ticket for this? fyi @shubham1g5 |
I think that's not the design we want here after this re-design, taking the conversation on slack here |
…958-update-opportunity-card-design
|
|
|
|
@conroy-ricketts |
@jaypanchal-13 Based on conversations with Brendon on Slack, no it should not show duplicate entires. We should combine those duplicates into one single card as a part of these changes
@jaypanchal-13 No worries, I can create the new bug ticket
@Jignesh-dimagi No, there is no buffer time. I think this issue will be resolved once the duplicates are eliminated and each learn/deliver app is triggered from just one single card in the opportunities list. Here's the current logic where we are sorting completed/expired jobs into the different categories. This function and its logic will definitely need to be refactored for this ticket (fyi @jaypanchal-13, @shubham1g5) |
|
I created a new ticket for the bug: CCCT-2035 |
…958-update-opportunity-card-design
|
@jaypanchal-13 It looks like that few issues which has came up during this PR review has separate tickets. Just trying to find if anything is remaining/blocked for this PR? |
|
|
@jaypanchal-13 I'm seeing a crash in the opportunities list: Screen_Recording_20260115_134312_One.UI.Home.mp4Here is the exception I pulled from Android Studio: |
conroy-ricketts
left a comment
conroy-ricketts
left a comment
There was a problem hiding this comment.
This is looking pretty good so far! My two biggest concerns are both the crash I mentioned in an earlier comment and that we need to refactor setJobListData() in ConnectJobsListsFragment to sort the jobs/opportunities
app/src/org/commcare/adapters/JobListConnectHomeAppsAdapter.java
Outdated
Show resolved
Hide resolved
app/src/org/commcare/adapters/JobListConnectHomeAppsAdapter.java
Outdated
Show resolved
Hide resolved
app/src/org/commcare/adapters/JobListConnectHomeAppsAdapter.java
Outdated
Show resolved
Hide resolved
|
@jaypanchal-13 No problem, did you get a chance to push the latest commit yet? I don't see it on my end |
…portunity-card-design
…portunity-card-design
|
|
I confirmed that the crash is fixed for me now 👍 |
conroy-ricketts
left a comment
conroy-ricketts
left a comment
There was a problem hiding this comment.
The code is looking pretty solid! Think there's just one more big concern that we should address before moving this across the finish line.
The "View Info" button seems to not be working for me. Here's a video of me tapping on several of the buttons and they do nothing:
Screen_Recording_20260120_141935_CommCare.Debug.mp4
Think the issue here is that there is no click listener set on the "View Info" button currently
|
conroy-ricketts
left a comment
conroy-ricketts
left a comment
There was a problem hiding this comment.
Yes because it is covered in this ticket. Will raise PR soon for it
@jaypanchal-13 Ah I see, sounds good! Updating my review to non-blocking.
@shubham1g5 I think going forward the team should be cautious of dividing up tickets like this if we want the master branch to always be in a production-ready state. Not sure if we want to disable the "View Info" button in the meantime.
|
@conroy-ricketts I agree that this PR must NOT be merged without the other one being ready to merge. |
Product Description
Ticket -> https://dimagi.atlassian.net/browse/CCCT-1958
Technical Summary
Screen shot ->
Feature Flag
Safety Assurance
Safety story
Automated test coverage
QA Plan
Labels and Review