Left hand navigation #3263
Left hand navigation #3263
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new navigation drawer UI for the Android application. It adds several XML layout files for the drawer's base, header, footer, list items, and sublist items, along with new vector drawable icons for navigation elements. Multiple string resources are added for English and localized into Spanish, French, Hindi, Lithuanian, Norwegian, Portuguese, Swahili, and Tigrinya. A new style for circular image views is defined. The Java and Kotlin codebase is extended with a new abstract Sequence Diagram(s)sequenceDiagram
participant User
participant Activity (BaseDrawerActivity)
participant NavDrawerAdapter
participant AnalyticsUtil
User->>Activity (BaseDrawerActivity): Launches Activity
Activity->>Activity: injectScreenLayout()
Activity->>NavDrawerAdapter: Setup with parent and child items
User->>Activity: Opens navigation drawer
Activity->>AnalyticsUtil: reportNavDrawerOpen()
User->>NavDrawerAdapter: Clicks parent or child item
NavDrawerAdapter->>Activity: onDrawerItemClicked()
Activity->>AnalyticsUtil: reportNavDrawerItemSelected(selectedItem)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 10
🧹 Nitpick comments (11)
app/res/drawable/baseline_help_outline_24.xml (1)
1-5: Avoid hard-coded tint and duplicate colouring attributes
android:tint="#FFFFFF"together withfillColor="@android:color/white"bakes a fixed colour into the asset and makes runtime theming harder. Prefer:
- Removing the
android:tint(let callers apply tint if needed), or- Referencing a theme/colour resource (
@color/cc_icon_tint) instead of#FFFFFF.Also make sure all drawer icons follow the same convention (see the other vectors added in this PR).
app/res/drawable/baseline_grid_view_24.xml (1)
1-5: Keep icon attributes consistent across the new vector setUnlike
baseline_help_outline_24.xml, this file omitsandroid:autoMirrored="true". For RTL support, add it (or remove from the other file) so the whole icon pack behaves consistently. While here, apply the same tint advice (avoid hard-coded#FFFFFF).app/res/values/strings.xml (2)
634-634: Fix spacing & casing: “Sign in / Register”- <string name="signin_register"><u>Signin / Register</u></string> + <string name="signin_register"><u>Sign in / Register</u></string>Tiny copy-text tweak for readability and to match Google Material wording.
638-638: Capitalise brand correctly- <string name="commcare_apps">Commcare Apps</string> + <string name="commcare_apps">CommCare Apps</string>Keeps product name consistent with the rest of the UI.
app/res/drawable/baseline_chat_bubble_outline_24.xml (1)
1-5: Apply the same RTL & tint conventions as other iconsAdd
android:autoMirrored="true"and replace the hard-coded tint with a colour resource to stay consistent with the rest of the new drawable set.app/res/drawable/baseline_credit_card_24.xml (1)
1-4: Remove redundant colour attributes & prefer theme-aware tintingBoth the root
<vector>and the<path>explicitly force white.
This (a) duplicates information and (b) prevents the icon from adapting to dark/light themes or brand colour changes.-<vector android:height="24dp" android:tint="#FFFFFF" +<vector android:height="24dp" android:tint="?attr/colorOnSurface" @@ - <path android:fillColor="@android:color/white" + <path android:fillColor="?attr/colorOnSurface"Using
?attr/colorOnSurface(or another semantic colour) keeps the resource design-system friendly.
Remove one of the two colour declarations if you want a fixed colour instead.app/res/drawable/baseline_clock.xml (1)
1-6: Same colour rigidity issue as the credit-card iconThe vector is hard-wired to white in two different places. Consider the same fix recommended above to make the drawable theme-aware.
app/res/layout/nav_drawer_sublist_item.xml (1)
7-13: Add ripple foreground for tactile feedbackThe row is
clickable/focusablebut has no ripple/pressed state, so users get no visual feedback.android:focusable="true" +android:foreground="?attr/selectableItemBackground"On API 21+ this gives a native ripple with zero extra work.
app/res/layout/nav_drawer_list_item.xml (1)
7-11: Missing ripple / pressed stateSame issue as the sub-list item – add
android:foreground="?attr/selectableItemBackground".app/res/layout/nav_drawer_footer.xml (1)
32-37:android:textFontWeightignored < API 28
textFontWeightdoes not render on Lollipop–Oreo. You already setsans-serif-medium; droptextFontWeightto avoid confusion.-android:textFontWeight="400"app/res/layout/nav_drawer_header.xml (1)
55-61: Hard-coded “John Doe” should be a translatable stringHard-coding defeats i18n & dynamic profile updates.
- android:text="John Doe" + android:text="@string/profile_default_user_name"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/res/drawable/nav_person_avatar.pngis excluded by!**/*.png
📒 Files selected for processing (12)
app/res/drawable/baseline_chat_bubble_outline_24.xml(1 hunks)app/res/drawable/baseline_clock.xml(1 hunks)app/res/drawable/baseline_credit_card_24.xml(1 hunks)app/res/drawable/baseline_grid_view_24.xml(1 hunks)app/res/drawable/baseline_help_outline_24.xml(1 hunks)app/res/drawable/logged_out_avatar.xml(1 hunks)app/res/layout/left_hand_base_drawer.xml(1 hunks)app/res/layout/nav_drawer_footer.xml(1 hunks)app/res/layout/nav_drawer_header.xml(1 hunks)app/res/layout/nav_drawer_list_item.xml(1 hunks)app/res/layout/nav_drawer_sublist_item.xml(1 hunks)app/res/values/strings.xml(1 hunks)
🧠 Learnings (12)
📓 Common learnings
Learnt from: shubham1g5
PR: dimagi/commcare-android#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: Jignesh-dimagi
PR: dimagi/commcare-android#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.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#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.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/attrs.xml:98-113
Timestamp: 2025-05-07T06:49:29.295Z
Learning: In Android resource files, attributes like `editTextHintColor` and `editTextHintSize` can be defined once globally with format declarations and then reused in multiple styleables without repeating the format. This is the proper pattern for attribute reuse in Android's resource system.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3093
File: app/res/layout/view_country_code_edittext.xml:2-7
Timestamp: 2025-05-09T06:50:50.863Z
Learning: The CommCare Android app uses minSdkVersion 21 (Android 5.0 Lollipop), so API 21+ attributes like paddingVertical and paddingHorizontal can be used without compatibility concerns.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/strings.xml:73-73
Timestamp: 2025-05-07T06:50:52.518Z
Learning: The empty `remote_form_payload_url` string resource in strings.xml is intentional legacy code and should be preserved as-is.
app/res/drawable/baseline_clock.xml (2)
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
Learnt from: Jignesh-dimagi
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.
app/res/drawable/baseline_chat_bubble_outline_24.xml (2)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/drawable/baseline_help_outline_24.xml (2)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/drawable/baseline_credit_card_24.xml (3)
Learnt from: Jignesh-dimagi
PR: #3093
File: app/res/layout/dialog_payment_confirmation.xml:14-17
Timestamp: 2025-05-09T07:00:45.949Z
Learning: Ignore false positives about unsupported shadow attributes (android:shadowColor, shadowDx, shadowDy, and shadowRadius) in CardView elements when they don't actually exist in the code.
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/drawable/baseline_grid_view_24.xml (2)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/drawable/logged_out_avatar.xml (2)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/layout/nav_drawer_header.xml (3)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3093
File: app/res/layout/view_country_code_edittext.xml:2-7
Timestamp: 2025-05-09T06:50:50.863Z
Learning: The CommCare Android app uses minSdkVersion 21 (Android 5.0 Lollipop), so API 21+ attributes like paddingVertical and paddingHorizontal can be used without compatibility concerns.
app/res/layout/nav_drawer_footer.xml (2)
Learnt from: Jignesh-dimagi
PR: #3093
File: app/res/layout/view_country_code_edittext.xml:2-7
Timestamp: 2025-05-09T06:50:50.863Z
Learning: The CommCare Android app uses minSdkVersion 21 (Android 5.0 Lollipop), so API 21+ attributes like paddingVertical and paddingHorizontal can be used without compatibility concerns.
Learnt from: Jignesh-dimagi
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.
app/res/layout/nav_drawer_list_item.xml (4)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3093
File: app/res/layout/view_country_code_edittext.xml:2-7
Timestamp: 2025-05-09T06:50:50.863Z
Learning: The CommCare Android app uses minSdkVersion 21 (Android 5.0 Lollipop), so API 21+ attributes like paddingVertical and paddingHorizontal can be used without compatibility concerns.
app/res/values/strings.xml (1)
Learnt from: OrangeAndGreen
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.
app/res/layout/left_hand_base_drawer.xml (2)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
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.
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: shubham1g5
PR: dimagi/commcare-android#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: Jignesh-dimagi
PR: dimagi/commcare-android#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.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#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.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/attrs.xml:98-113
Timestamp: 2025-05-07T06:49:29.295Z
Learning: In Android resource files, attributes like `editTextHintColor` and `editTextHintSize` can be defined once globally with format declarations and then reused in multiple styleables without repeating the format. This is the proper pattern for attribute reuse in Android's resource system.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3093
File: app/res/layout/view_country_code_edittext.xml:2-7
Timestamp: 2025-05-09T06:50:50.863Z
Learning: The CommCare Android app uses minSdkVersion 21 (Android 5.0 Lollipop), so API 21+ attributes like paddingVertical and paddingHorizontal can be used without compatibility concerns.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/strings.xml:73-73
Timestamp: 2025-05-07T06:50:52.518Z
Learning: The empty `remote_form_payload_url` string resource in strings.xml is intentional legacy code and should be preserved as-is.
app/res/drawable/baseline_clock.xml (2)
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
Learnt from: Jignesh-dimagi
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.
app/res/drawable/baseline_chat_bubble_outline_24.xml (2)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/drawable/baseline_help_outline_24.xml (2)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/drawable/baseline_credit_card_24.xml (3)
Learnt from: Jignesh-dimagi
PR: #3093
File: app/res/layout/dialog_payment_confirmation.xml:14-17
Timestamp: 2025-05-09T07:00:45.949Z
Learning: Ignore false positives about unsupported shadow attributes (android:shadowColor, shadowDx, shadowDy, and shadowRadius) in CardView elements when they don't actually exist in the code.
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/drawable/baseline_grid_view_24.xml (2)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/drawable/logged_out_avatar.xml (2)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/layout/nav_drawer_header.xml (3)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3093
File: app/res/layout/view_country_code_edittext.xml:2-7
Timestamp: 2025-05-09T06:50:50.863Z
Learning: The CommCare Android app uses minSdkVersion 21 (Android 5.0 Lollipop), so API 21+ attributes like paddingVertical and paddingHorizontal can be used without compatibility concerns.
app/res/layout/nav_drawer_footer.xml (2)
Learnt from: Jignesh-dimagi
PR: #3093
File: app/res/layout/view_country_code_edittext.xml:2-7
Timestamp: 2025-05-09T06:50:50.863Z
Learning: The CommCare Android app uses minSdkVersion 21 (Android 5.0 Lollipop), so API 21+ attributes like paddingVertical and paddingHorizontal can be used without compatibility concerns.
Learnt from: Jignesh-dimagi
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.
app/res/layout/nav_drawer_list_item.xml (4)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3093
File: app/res/layout/view_country_code_edittext.xml:2-7
Timestamp: 2025-05-09T06:50:50.863Z
Learning: The CommCare Android app uses minSdkVersion 21 (Android 5.0 Lollipop), so API 21+ attributes like paddingVertical and paddingHorizontal can be used without compatibility concerns.
app/res/values/strings.xml (1)
Learnt from: OrangeAndGreen
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.
app/res/layout/left_hand_base_drawer.xml (2)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
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.
🔇 Additional comments (2)
app/res/drawable/logged_out_avatar.xml (1)
1-19: 135 dp intrinsic size is unusually large – verify memory impactThis drawable declares
android:width|height="135dp". On most screens the ImageView will down-scale it, but the decoder still keeps the full path data in memory. Unless the avatar is truly rendered at that size, consider reducing the intrinsic size to 48 dp/64 dp and letting the ImageView upscale when necessary.app/res/layout/left_hand_base_drawer.xml (1)
1-12: VerifyDrawerLayoutusage – only one child present
DrawerLayoutis expected to host (1) a content view and (2) one or more drawer views withlayout_gravity="start|end". Here it wraps only the drawer panel, which may break drawer gestures and scrim behaviour unless this file is itself inflated inside anotherDrawerLayout.Please confirm the layout hierarchy or switch the root to a simple
LinearLayout.
| <ImageView | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:layout_gravity="center" | ||
| android:src="@drawable/logged_out_avatar"/> | ||
| <TextView |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avatar inside sign-out view lacks contentDescription
<ImageView
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_gravity="center"
- android:src="@drawable/logged_out_avatar"/>
+ android:src="@drawable/logged_out_avatar"
+ android:contentDescription="@string/logged_out_avatar"/>📝 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:layout_width="wrap_content" | |
| android:layout_height="wrap_content" | |
| android:layout_gravity="center" | |
| android:src="@drawable/logged_out_avatar"/> | |
| <TextView | |
| <ImageView | |
| android:layout_width="wrap_content" | |
| android:layout_height="wrap_content" | |
| android:layout_gravity="center" | |
| android:src="@drawable/logged_out_avatar" | |
| android:contentDescription="@string/logged_out_avatar"/> | |
| <TextView |
🤖 Prompt for AI Agents
In app/res/layout/left_hand_base_drawer.xml between lines 24 and 29, the
ImageView for the avatar inside the sign-out view is missing a
contentDescription attribute. Add a meaningful contentDescription to this
ImageView to improve accessibility, describing the image purpose for screen
readers.
| <androidx.recyclerview.widget.RecyclerView | ||
| android:visibility="gone" | ||
| android:id="@+id/signin_view" | ||
| android:layout_weight="1" | ||
| android:layout_width="match_parent" | ||
| android:layout_height="wrap_content" | ||
| android:background="@color/cc_brand_color"/> |
There was a problem hiding this comment.
RecyclerView weight won’t apply with layout_height="wrap_content"
Weight is ignored unless the height is 0dp. This view therefore collapses instead of filling remaining space.
- android:layout_height="wrap_content"
+ android:layout_height="0dp"📝 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.
| <androidx.recyclerview.widget.RecyclerView | |
| android:visibility="gone" | |
| android:id="@+id/signin_view" | |
| android:layout_weight="1" | |
| android:layout_width="match_parent" | |
| android:layout_height="wrap_content" | |
| android:background="@color/cc_brand_color"/> | |
| <androidx.recyclerview.widget.RecyclerView | |
| android:visibility="gone" | |
| android:id="@+id/signin_view" | |
| android:layout_weight="1" | |
| android:layout_width="match_parent" | |
| - android:layout_height="wrap_content" | |
| + android:layout_height="0dp" | |
| android:background="@color/cc_brand_color"/> |
🤖 Prompt for AI Agents
In app/res/layout/left_hand_base_drawer.xml between lines 48 and 54, the
RecyclerView has layout_height set to "wrap_content" which causes the
layout_weight to be ignored and the view to collapse. Change the layout_height
to "0dp" to enable the weight to properly allocate remaining space and make the
RecyclerView fill as intended.
app/res/layout/nav_drawer_header.xml
Outdated
| <ImageButton | ||
| android:id="@+id/close_button" | ||
| android:layout_width="24dp" | ||
| android:layout_height="24dp" | ||
| android:background="@color/transparent" | ||
| android:tint="@color/white" | ||
| android:src="@drawable/ic_connect_close"/> | ||
| <ImageView |
There was a problem hiding this comment.
Add contentDescription and ripple feedback to the close button for accessibility & UX parity
The ImageButton lacks an a11y label and uses a non-interactive background. Screen-reader users won’t know what the button does, and there will be no ripple on touch.
<ImageButton
android:id="@+id/close_button"
android:layout_width="24dp"
android:layout_height="24dp"
- android:background="@color/transparent"
- android:tint="@color/white"
- android:src="@drawable/ic_connect_close"/>
+ android:background="?attr/selectableItemBackgroundBorderless"
+ android:tint="@color/white"
+ android:src="@drawable/ic_connect_close"
+ android:contentDescription="@string/accessibility_close_drawer"/>Remember to add the accessibility_close_drawer string to strings.xml.
🤖 Prompt for AI Agents
In app/res/layout/nav_drawer_header.xml lines 15 to 22, the ImageButton with id
close_button lacks a contentDescription and ripple feedback, which affects
accessibility and user experience. Add the attribute
android:contentDescription="@string/accessibility_close_drawer" to provide an
accessible label, and replace android:background="@color/transparent" with
android:background="?attr/selectableItemBackgroundBorderless" to enable ripple
effect on touch. Also, ensure the string accessibility_close_drawer is defined
in strings.xml.
app/res/layout/nav_drawer_header.xml
Outdated
| <ImageView | ||
| android:id="@+id/image_user_profile" | ||
| android:layout_width="64dp" | ||
| android:layout_height="wrap_content" | ||
| android:src="@drawable/nav_person_avatar"/> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avatar is missing an accessibility label
<ImageView
android:id="@+id/image_user_profile"
android:layout_width="64dp"
android:layout_height="wrap_content"
- android:src="@drawable/nav_person_avatar"/>
+ android:src="@drawable/nav_person_avatar"
+ android:contentDescription="@string/profile_picture"/>🤖 Prompt for AI Agents
In app/res/layout/nav_drawer_header.xml around lines 43 to 47, the ImageView for
the user profile avatar lacks an accessibility label. Add the
android:contentDescription attribute to the ImageView with a meaningful
description such as "User profile avatar" to improve accessibility for screen
readers.
| android:text="CommCare Apps" | ||
| android:textColor="@android:color/white" | ||
| android:textSize="14sp" | ||
| android:layout_marginStart="12dp"/> |
There was a problem hiding this comment.
Hard-coded label prevents localisation
Move "CommCare Apps" to a string resource.
-android:text="CommCare Apps"
+android:text="@string/commcare_apps"📝 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.
| android:text="CommCare Apps" | |
| android:textColor="@android:color/white" | |
| android:textSize="14sp" | |
| android:layout_marginStart="12dp"/> | |
| android:text="@string/commcare_apps" | |
| android:textColor="@android:color/white" | |
| android:textSize="14sp" | |
| android:layout_marginStart="12dp"/> |
🤖 Prompt for AI Agents
In app/res/layout/nav_drawer_list_item.xml around lines 24 to 27, the text
attribute is hard-coded as "CommCare Apps", which prevents localization. To fix
this, move the string "CommCare Apps" into the strings.xml resource file and
reference it here using the @string resource identifier instead of the
hard-coded text.
| <TextView | ||
| android:id="@+id/sublist_title" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:paddingStart="16dp" | ||
| android:text="Sample Job One" | ||
| android:textColor="@android:color/white" | ||
| android:textSize="14sp"/> |
There was a problem hiding this comment.
Hard-coded text blocks localisation
"Sample Job One" should come from strings.xml, otherwise it never gets translated.
-android:text="Sample Job One"
+android:text="@string/sample_job_one"(remember to add the new string resource).
📝 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/sublist_title" | |
| android:layout_width="wrap_content" | |
| android:layout_height="wrap_content" | |
| android:paddingStart="16dp" | |
| android:text="Sample Job One" | |
| android:textColor="@android:color/white" | |
| android:textSize="14sp"/> | |
| <TextView | |
| android:id="@+id/sublist_title" | |
| android:layout_width="wrap_content" | |
| android:layout_height="wrap_content" | |
| android:paddingStart="16dp" | |
| - android:text="Sample Job One" | |
| + android:text="@string/sample_job_one" | |
| android:textColor="@android:color/white" | |
| android:textSize="14sp"/> |
🤖 Prompt for AI Agents
In app/res/layout/nav_drawer_sublist_item.xml around lines 20 to 27, the
TextView's text attribute is hard-coded as "Sample Job One". To enable
localization, move this string to the strings.xml resource file by adding a new
string entry with the value "Sample Job One". Then replace the android:text
attribute in the XML with a reference to this new string resource using
@string/your_string_name.
📝 WalkthroughWalkthroughThis change introduces several new XML resource files to the Android project, including vector drawable icons and layout files for a navigation drawer UI. Specifically, it adds vector assets for chat bubble, clock, credit card, grid view, help outline, and a logged-out avatar. New layout resources define the structure for a left-hand navigation drawer, including its header, footer, list items, and sublist items. Additionally, new string resources are added for UI elements related to authentication, help, and user sections. No existing resources or code are modified or removed. Estimated code review effort1 (~8 minutes) Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (14)
app/res/drawable/baseline_help_outline_24.xml (1)
1-4: Remove hard-coded tint and reference local color resource for consistencyThe drawable hard-codes
android:tint="#FFFFFF"while each path already setsandroid:fillColor="@android:color/white".
Keeping both is redundant and prevents the theme / hostingImageViewfrom tinting the asset dynamically.
Prefer:
- Drop the
android:tintattribute.- Use an app-level color resource (
@color/white) instead of the framework one to stay in control of theming.- Align
android:autoMirroredusage with the rest of the new icons – either add it everywhere or remove it here to stay consistent.-<vector android:autoMirrored="true" android:height="24dp" - android:tint="#FFFFFF" android:viewportHeight="24" +<vector android:autoMirrored="true" + android:height="24dp" android:viewportHeight="24" - android:viewportWidth="24" android:width="24dp" xmlns:android="http://schemas.android.com/apk/res/android"> - <path android:fillColor="@android:color/white" ... + android:viewportWidth="24" android:width="24dp" + xmlns:android="http://schemas.android.com/apk/res/android"> + <path android:fillColor="@color/white" ...app/res/drawable/logged_out_avatar.xml (1)
14-18: Minor: prefer whole-number stroke widths
android:strokeWidth="1.5"renders fine, yet using integer dp (1 dp or 2 dp) keeps visual weight predictable across densities.
No action needed if the 1.5 dp weight was chosen deliberately.app/res/drawable/baseline_clock.xml (1)
1-5: Avoid redundanttint– rely on themingSame concern as in
baseline_help_outline_24.xml: the icon pins bothandroid:tintand per-pathfillColorto white, blocking runtime tinting.-<vector android:height="24dp" android:tint="#FFFFFF" +<vector android:height="24dp"Also consider referencing
@color/whiteforfillColor.app/res/drawable/baseline_chat_bubble_outline_24.xml (1)
1-4: Make tinting strategy consistentRecommend dropping
"android:tint"and switchingfillColorto an app color resource, mirroring the previous comments for other icons.
This keeps all new vectors themable.app/res/drawable/baseline_credit_card_24.xml (1)
1-4: Unify color handling with other vector assetsSame feedback: remove fixed
android:tint, reference@color/whiteforfillColor.
Uniform handling simplifies day-night theme support later.app/res/drawable/baseline_grid_view_24.xml (1)
1-5: Avoid hard-coded tint + duplicate white fill; keep vector themableRoot–level
android:tint="#FFFFFF"together with an explicitandroid:fillColor="@android:color/white"locks the icon to white and defeats theming. Prefer leaving the vector un-tinted and let callers supply tint viaandroid:tint(orapp:tint) on theImageView.-<vector android:height="24dp" android:tint="#FFFFFF" +<vector android:height="24dp"Optionally replace
@android:color/whitewith a theme attr (?attr/colorOnPrimary) to keep it style-able.app/res/layout/nav_drawer_sublist_item.xml (2)
1-13: Add ripple feedback for accessibility & UXClickable containers should expose visual feedback. Add a foreground ripple:
android:focusable="true" +android:foreground="?attr/selectableItemBackground"
15-18: MissingcontentDescriptionon iconScreen-reader users get no context for this image.
- <ImageView + <ImageView + android:contentDescription="@string/nav_item_icon_desc"app/res/layout/nav_drawer_header.xml (1)
43-47: Profile avatar missing description- <ImageView + <ImageView android:id="@+id/image_user_profile" @@ android:src="@drawable/nav_person_avatar"/> + android:contentDescription="@string/profile_picture"app/res/layout/nav_drawer_list_item.xml (2)
1-13: Add ripple to clickable list itemsandroid:focusable="true" +android:foreground="?attr/selectableItemBackground"
13-18: Icon needscontentDescription- <ImageView + <ImageView android:id="@+id/list_icon" @@ - app:tint="@android:color/white" /> + app:tint="@android:color/white" + android:contentDescription="@string/nav_item_icon_desc"/>app/res/layout/left_hand_base_drawer.xml (1)
24-28: Avatar lackscontentDescription- <ImageView + <ImageView android:layout_width="wrap_content" @@ - android:src="@drawable/logged_out_avatar"/> + android:src="@drawable/logged_out_avatar" + android:contentDescription="@string/logged_out_avatar"/>app/res/values/strings.xml (2)
634-634: Typo & mixed casing in “Signin / Register”“Sign in” is two words; “Register” uses sentence-case elsewhere. Suggested fix:
-<string name="signin_register"><u>Signin / Register</u></string> +<string name="signin_register"><u>Sign in / Register</u></string>Also confirm whether the underline markup is really required; TextView will render literal HTML only when
android:textIsSelectable/LinkMovementMethodis set.
638-639: Brand capitalization inconsistency
Commcare Appsshould follow existing branding (“CommCare”).-<string name="commcare_apps">Commcare Apps</string> +<string name="commcare_apps">CommCare Apps</string>Ensures uniform branding across UI and translations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/res/drawable/nav_person_avatar.pngis excluded by!**/*.png
📒 Files selected for processing (12)
app/res/drawable/baseline_chat_bubble_outline_24.xml(1 hunks)app/res/drawable/baseline_clock.xml(1 hunks)app/res/drawable/baseline_credit_card_24.xml(1 hunks)app/res/drawable/baseline_grid_view_24.xml(1 hunks)app/res/drawable/baseline_help_outline_24.xml(1 hunks)app/res/drawable/logged_out_avatar.xml(1 hunks)app/res/layout/left_hand_base_drawer.xml(1 hunks)app/res/layout/nav_drawer_footer.xml(1 hunks)app/res/layout/nav_drawer_header.xml(1 hunks)app/res/layout/nav_drawer_list_item.xml(1 hunks)app/res/layout/nav_drawer_sublist_item.xml(1 hunks)app/res/values/strings.xml(1 hunks)
🧠 Learnings (12)
📓 Common learnings
Learnt from: shubham1g5
PR: dimagi/commcare-android#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: Jignesh-dimagi
PR: dimagi/commcare-android#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.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#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.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/attrs.xml:98-113
Timestamp: 2025-05-07T06:49:29.295Z
Learning: In Android resource files, attributes like `editTextHintColor` and `editTextHintSize` can be defined once globally with format declarations and then reused in multiple styleables without repeating the format. This is the proper pattern for attribute reuse in Android's resource system.
app/res/drawable/baseline_clock.xml (2)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/drawable/baseline_chat_bubble_outline_24.xml (2)
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
Learnt from: Jignesh-dimagi
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.
app/res/drawable/baseline_help_outline_24.xml (2)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/drawable/baseline_grid_view_24.xml (2)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/drawable/baseline_credit_card_24.xml (3)
Learnt from: Jignesh-dimagi
PR: #3093
File: app/res/layout/dialog_payment_confirmation.xml:14-17
Timestamp: 2025-05-09T07:00:45.949Z
Learning: Ignore false positives about unsupported shadow attributes (android:shadowColor, shadowDx, shadowDy, and shadowRadius) in CardView elements when they don't actually exist in the code.
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/layout/nav_drawer_header.xml (3)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3093
File: app/res/layout/view_country_code_edittext.xml:2-7
Timestamp: 2025-05-09T06:50:50.863Z
Learning: The CommCare Android app uses minSdkVersion 21 (Android 5.0 Lollipop), so API 21+ attributes like paddingVertical and paddingHorizontal can be used without compatibility concerns.
app/res/layout/nav_drawer_footer.xml (2)
Learnt from: Jignesh-dimagi
PR: #3093
File: app/res/layout/view_country_code_edittext.xml:2-7
Timestamp: 2025-05-09T06:50:50.863Z
Learning: The CommCare Android app uses minSdkVersion 21 (Android 5.0 Lollipop), so API 21+ attributes like paddingVertical and paddingHorizontal can be used without compatibility concerns.
Learnt from: Jignesh-dimagi
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.
app/res/layout/left_hand_base_drawer.xml (1)
Learnt from: Jignesh-dimagi
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.
app/res/drawable/logged_out_avatar.xml (2)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/values/strings.xml (1)
Learnt from: OrangeAndGreen
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.
app/res/layout/nav_drawer_list_item.xml (4)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3093
File: app/res/layout/view_country_code_edittext.xml:2-7
Timestamp: 2025-05-09T06:50:50.863Z
Learning: The CommCare Android app uses minSdkVersion 21 (Android 5.0 Lollipop), so API 21+ attributes like paddingVertical and paddingHorizontal can be used without compatibility concerns.
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: shubham1g5
PR: dimagi/commcare-android#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: Jignesh-dimagi
PR: dimagi/commcare-android#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.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#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.
Learnt from: Jignesh-dimagi
PR: dimagi/commcare-android#3070
File: app/res/values/attrs.xml:98-113
Timestamp: 2025-05-07T06:49:29.295Z
Learning: In Android resource files, attributes like `editTextHintColor` and `editTextHintSize` can be defined once globally with format declarations and then reused in multiple styleables without repeating the format. This is the proper pattern for attribute reuse in Android's resource system.
app/res/drawable/baseline_clock.xml (2)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/drawable/baseline_chat_bubble_outline_24.xml (2)
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
Learnt from: Jignesh-dimagi
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.
app/res/drawable/baseline_help_outline_24.xml (2)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/drawable/baseline_grid_view_24.xml (2)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/drawable/baseline_credit_card_24.xml (3)
Learnt from: Jignesh-dimagi
PR: #3093
File: app/res/layout/dialog_payment_confirmation.xml:14-17
Timestamp: 2025-05-09T07:00:45.949Z
Learning: Ignore false positives about unsupported shadow attributes (android:shadowColor, shadowDx, shadowDy, and shadowRadius) in CardView elements when they don't actually exist in the code.
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/layout/nav_drawer_header.xml (3)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3093
File: app/res/layout/view_country_code_edittext.xml:2-7
Timestamp: 2025-05-09T06:50:50.863Z
Learning: The CommCare Android app uses minSdkVersion 21 (Android 5.0 Lollipop), so API 21+ attributes like paddingVertical and paddingHorizontal can be used without compatibility concerns.
app/res/layout/nav_drawer_footer.xml (2)
Learnt from: Jignesh-dimagi
PR: #3093
File: app/res/layout/view_country_code_edittext.xml:2-7
Timestamp: 2025-05-09T06:50:50.863Z
Learning: The CommCare Android app uses minSdkVersion 21 (Android 5.0 Lollipop), so API 21+ attributes like paddingVertical and paddingHorizontal can be used without compatibility concerns.
Learnt from: Jignesh-dimagi
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.
app/res/layout/left_hand_base_drawer.xml (1)
Learnt from: Jignesh-dimagi
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.
app/res/drawable/logged_out_avatar.xml (2)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
app/res/values/strings.xml (1)
Learnt from: OrangeAndGreen
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.
app/res/layout/nav_drawer_list_item.xml (4)
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3070
File: app/res/values/themes.xml:80-80
Timestamp: 2025-05-07T06:48:32.621Z
Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
Learnt from: Jignesh-dimagi
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.
Learnt from: Jignesh-dimagi
PR: #3093
File: app/res/layout/view_country_code_edittext.xml:2-7
Timestamp: 2025-05-09T06:50:50.863Z
Learning: The CommCare Android app uses minSdkVersion 21 (Android 5.0 Lollipop), so API 21+ attributes like paddingVertical and paddingHorizontal can be used without compatibility concerns.
🔇 Additional comments (3)
app/res/drawable/logged_out_avatar.xml (1)
1-5: Confirm the need for a 135 dp intrinsic sizeUnlike the other 24 dp icons, this vector declares
android:width|height="135dp".
That’s fine if you intend to show it at (or close to) that physical size, but if the view will scale it down (e.g. to 48 dp), consider:
- Setting a more typical default (48 dp) and letting the layout request a larger size when required, or
- Relying solely on
android:viewport*and sizing it via layout params.Larger intrinsic sizes slightly increase XML inflation overhead.
app/res/layout/nav_drawer_footer.xml (2)
22-27: MissingcontentDescriptionon decorative iconsAll
ImageViews should declare an explicitandroid:contentDescription.
If purely decorative:+android:contentDescription="@null" +android:importantForAccessibility="no"Otherwise supply a meaningful string resource.
Also applies to: 48-51
34-35:android:textFontWeightwill crash on API < 28
android:textFontWeightis only part of the public SDK starting API 28. WithminSdkVersion=21the layout will fail at runtime on Android 21-27.
Use a compatible alternative, e.g.:-android:textFontWeight="400" +android:textStyle="normal" +android:fontFamily="sans-serif-medium"If a true weight-specific font is required, bundle it as a custom font resource and reference it via
android:fontFamily.Also applies to: 59-60
⛔ Skipped due to learnings
Learnt from: Jignesh-dimagi PR: dimagi/commcare-android#3093 File: app/res/layout/view_country_code_edittext.xml:2-7 Timestamp: 2025-05-09T06:50:50.863Z Learning: The CommCare Android app uses minSdkVersion 21 (Android 5.0 Lollipop), so API 21+ attributes like paddingVertical and paddingHorizontal can be used without compatibility concerns.Learnt from: Jignesh-dimagi PR: dimagi/commcare-android#3070 File: app/res/values/attrs.xml:98-113 Timestamp: 2025-05-07T06:49:29.295Z Learning: In Android resource files, attributes like `editTextHintColor` and `editTextHintSize` can be defined once globally with format declarations and then reused in multiple styleables without repeating the format. This is the proper pattern for attribute reuse in Android's resource system.Learnt from: Jignesh-dimagi PR: dimagi/commcare-android#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.Learnt from: Jignesh-dimagi PR: dimagi/commcare-android#3070 File: app/res/values/themes.xml:80-80 Timestamp: 2025-05-07T06:48:32.621Z Learning: The CommCare Android app doesn't require backward compatibility for support library features like the vector drawable compatibility provided by app:srcCompat. It uses android:src directly for drawable references in themes.
| <androidx.recyclerview.widget.RecyclerView | ||
| android:visibility="gone" | ||
| android:id="@+id/signin_view" | ||
| android:layout_weight="1" | ||
| android:layout_width="match_parent" | ||
| android:layout_height="wrap_content" | ||
| android:background="@color/cc_brand_color"/> |
There was a problem hiding this comment.
Weight used with wrap_content height – view won’t expand
RecyclerView should have layout_height="0dp" when using weight, otherwise the weight is ignored and scrolling area may collapse.
- android:layout_height="wrap_content"
+ android:layout_height="0dp"🤖 Prompt for AI Agents
In app/res/layout/left_hand_base_drawer.xml between lines 48 and 54, the
RecyclerView uses layout_weight="1" but has layout_height="wrap_content", which
causes the weight to be ignored and the view not to expand properly. Change the
RecyclerView's layout_height attribute from "wrap_content" to "0dp" to ensure
the weight is applied correctly and the view expands as intended.
app/res/layout/nav_drawer_footer.xml
Outdated
| android:layout_width="wrap_content" | ||
| android:layout_height="48dp" | ||
| android:paddingStart="16dp" | ||
| android:paddingEnd="16dp" | ||
| android:orientation="horizontal" | ||
| android:gravity="center_vertical"> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Rows should be match_parent width & clickable to meet UX/accessibility expectations
LinearLayout rows are wrap_content, so only the icon/text area is touchable.
Recommended:
-android:layout_width="wrap_content"
+android:layout_width="match_parent"
+android:clickable="true"
+android:focusable="true"
+android:foreground="?attr/selectableItemBackground"This enlarges the hit-target, adds ripple feedback, and aligns with Material guidelines.
Also applies to: 40-46
🤖 Prompt for AI Agents
In app/res/layout/nav_drawer_footer.xml around lines 15 to 20, change the
LinearLayout's android:layout_width from "wrap_content" to "match_parent" to
ensure the entire row is touchable. Also, make the row clickable and add ripple
feedback by setting android:clickable="true" and
android:foreground="?attr/selectableItemBackground". Apply the same changes to
lines 40 to 46 for consistency with UX and accessibility guidelines.
app/res/layout/nav_drawer_header.xml
Outdated
| <ImageButton | ||
| android:id="@+id/close_button" | ||
| android:layout_width="24dp" | ||
| android:layout_height="24dp" | ||
| android:background="@color/transparent" | ||
| android:tint="@color/white" | ||
| android:src="@drawable/ic_connect_close"/> | ||
| <ImageView |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Close button lacks a11y description + uses non-system transparent color
Provide a contentDescription and rely on the platform transparent color.
- <ImageButton
+ <ImageButton
android:id="@+id/close_button"
@@
- android:background="@color/transparent"
+ android:background="@android:color/transparent"
+ android:contentDescription="@string/close_drawer"🤖 Prompt for AI Agents
In app/res/layout/nav_drawer_header.xml lines 15 to 22, the ImageButton with id
close_button lacks an accessibility contentDescription and uses a custom
transparent color. Add a meaningful contentDescription attribute to describe the
button's purpose for screen readers, and replace the android:background color
with the system default transparent color by using @android:color/transparent
instead of @color/transparent.
app/res/layout/nav_drawer_header.xml
Outdated
| android:id="@+id/user_name" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:text="John Doe" | ||
| android:textColor="@color/white" | ||
| android:textSize="14sp"/> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Placeholder name hard-coded
"John Doe" should be populated programmatically or from a <string> placeholder for localisation/testing.
- android:text="John Doe"
+ android:text="@string/user_name_placeholder"Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/res/layout/nav_drawer_header.xml between lines 55 and 60, the TextView
with id user_name has a hard-coded placeholder name "John Doe". Replace this
hard-coded string with a reference to a string resource defined in strings.xml
to support localization and easier testing. Update the android:text attribute to
use the string resource instead of a literal string.
| <TextView | ||
| android:id="@+id/list_title" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:text="CommCare Apps" | ||
| android:textColor="@android:color/white" | ||
| android:textSize="14sp" | ||
| android:layout_marginStart="12dp"/> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hard-coded label blocks localisation
Use the new @string/commcare_apps.
- android:text="CommCare Apps"
+ android:text="@string/commcare_apps"📝 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/list_title" | |
| android:layout_width="wrap_content" | |
| android:layout_height="wrap_content" | |
| android:text="CommCare Apps" | |
| android:textColor="@android:color/white" | |
| android:textSize="14sp" | |
| android:layout_marginStart="12dp"/> | |
| <TextView | |
| android:id="@+id/list_title" | |
| android:layout_width="wrap_content" | |
| android:layout_height="wrap_content" | |
| android:text="@string/commcare_apps" | |
| android:textColor="@android:color/white" | |
| android:textSize="14sp" | |
| android:layout_marginStart="12dp"/> |
🤖 Prompt for AI Agents
In app/res/layout/nav_drawer_list_item.xml between lines 20 and 27, the
TextView's android:text attribute is hard-coded as "CommCare Apps", which blocks
localization. Replace the hard-coded string with a reference to the string
resource @string/commcare_apps to enable proper localization support.
| <ImageView | ||
| android:id="@+id/arroe_icon" | ||
| android:layout_width="30dp" | ||
| android:layout_height="30dp" | ||
| android:paddingStart="16dp" | ||
| android:src="@drawable/ic_blue_forward" | ||
| app:tint="@android:color/white" /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Typo in view ID & missing description
arroe_icon → arrow_icon, add description.
- <ImageView
- android:id="@+id/arroe_icon"
+ <ImageView
+ android:id="@+id/arrow_icon"
android:layout_width="30dp"
@@
- app:tint="@android:color/white" />
+ app:tint="@android:color/white"
+ android:contentDescription="@string/arrow_forward"/>📝 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/arroe_icon" | |
| android:layout_width="30dp" | |
| android:layout_height="30dp" | |
| android:paddingStart="16dp" | |
| android:src="@drawable/ic_blue_forward" | |
| app:tint="@android:color/white" /> | |
| <ImageView | |
| android:id="@+id/arrow_icon" | |
| android:layout_width="30dp" | |
| android:layout_height="30dp" | |
| android:paddingStart="16dp" | |
| android:src="@drawable/ic_blue_forward" | |
| app:tint="@android:color/white" | |
| android:contentDescription="@string/arrow_forward"/> |
🤖 Prompt for AI Agents
In app/res/layout/nav_drawer_list_item.xml between lines 29 and 35, the
ImageView has a typo in its ID attribute: change "arroe_icon" to "arrow_icon".
Additionally, add a contentDescription attribute to this ImageView to improve
accessibility by describing the image's purpose.
| android:paddingStart="16dp" | ||
| android:text="Sample Job One" | ||
| android:textColor="@android:color/white" | ||
| android:textSize="14sp"/> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hard-coded text breaks localisation
Move the label to strings.xml and reference it.
- android:text="Sample Job One"
+ android:text="@string/sample_job_one"🤖 Prompt for AI Agents
In app/res/layout/nav_drawer_sublist_item.xml around lines 24 to 27, the text
attribute is hard-coded as "Sample Job One", which breaks localization. To fix
this, move the string "Sample Job One" to the strings.xml resource file and
replace the android:text attribute with a reference to this string resource
using @string/your_string_name.
shubham1g5
left a comment
shubham1g5
left a comment
There was a problem hiding this comment.
We should make sure to rename all xml resources (strings/layout names etc) to use a common prefix like left_navigation_menu_x to highlight that they are all used in left hand nav menu.
app/res/values/strings.xml
Outdated
| <string name="signin_register"><u>Signin / Register</u></string> | ||
| <string name="help">Help</string> | ||
| <string name="not_signed_in_to_personal_id">You are not signed in to Personal ID</string> | ||
| <string name="opportunities">Opportunities</string> | ||
| <string name="commcare_apps">Commcare Apps</string> | ||
| <string name="work_history">Work History</string> | ||
| <string name="payments">Payments</string> | ||
| <string name="manage_profile">Manage Profile</string> |
There was a problem hiding this comment.
- language support is missing
There was a problem hiding this comment.
not sure why is this required ?
There was a problem hiding this comment.
This is for when person pic is not available
shubham1g5
left a comment
shubham1g5
left a comment
There was a problem hiding this comment.
- drawables are still names without
nav_drawer_prefix as other layouts - I see you named a file as
left_hand_base_drawerwhile others asnav_drawer_x, can we be consistent here. - Can you also take a pass on drawables names and make sure it's easy to know from their names where they are getting used in nav drawer. Names like
chat_bubble_outlinedoesn't really make sense to me as I don't really see any chat bubbles or in screenshot.
@shubham1g5 for the drawables i have not changed the name because it was named same as android vector because if someone else will take a look he can find that with the same name otherwise we will make are own name then someone else what not able to easily find that...at end ur call i can change the name as well |
What do you mean by android vector here ? |
Directly imported from the vector asset in android |
…droid into pm_ccct_1514
Pm ccct 1514
…nto pm_nav_design_change # Conflicts: # app/res/values-lt/strings.xml # app/res/values-no/strings.xml # app/src/org/commcare/activities/StandardHomeActivity.java
…nto pm_CCCT-1515 # Conflicts: # app/res/values-lt/strings.xml # app/res/values-no/strings.xml
…roid into pm_nav_design_change # Conflicts: # app/res/values-no/strings.xml
- implement the nav drawer based on Controller
pm-dimagi
dismissed stale reviews from Jignesh-dimagi and shubham1g5
via
2497591
|
@damagatchi retest this please |
Product Description
https://www.figma.com/design/YDgUWZJ9DZPNA6XCZCyO8i/Sidebar-Nav--CCC-PID---DRAFT-?node-id=355-1143&t=a5jo5D5F82j1hr2N-1
Jira ticket:- CCCT-1515
https://dimagi.atlassian.net/browse/CCCT-1514
https://dimagi.atlassian.net/browse/CCCT-1517
Design Doc:- https://docs.google.com/document/d/1UXgjCAB8H_33YDJTZ0QN9z5f3-WDdHeyWeMiGJlSPsE/edit?usp=sharing
Things left to setup
1- click on individual page( cannot be done because it will be done by end of phase4 merge)
2- adding it to setupactivity(will do once pass the initial review)
These are the xml files for the left hand nav drawer and assets related to that
image 1
image 2
Screen_Recording_20250731_184334_CommCare.Debug.mp4
Technical Summary
Feature Flag
Safety Assurance
Safety story
Automated test coverage
QA Plan
Left hand nav with all the single click lands to particular page from the menu
Labels and Review