Preview of message in messages channel changed #2976
Conversation
…e string for received messages
Jignesh-dimagi
changed the title
📝 WalkthroughWalkthroughThis pull request modifies how message previews are handled within the messaging channel components. In the Sequence Diagram(s)sequenceDiagram
participant DBHelper as ConnectDatabaseHelper
participant ChannelRecord as ConnectMessagingChannelRecord
participant StringUtil as StringUtils
%% Preview construction flow in ConnectDatabaseHelper
DBHelper->>ChannelRecord: Initialize preview
alt Unconsented Channel
ChannelRecord-->>DBHelper: Returns SpannableString from resource
else Channel with messages
DBHelper->>DBHelper: Check last message properties
alt Outgoing and confirmed message
DBHelper->>StringUtil: convertDpToPixel(dp)
DBHelper->>ChannelRecord: Construct SpannableString with drawable icon
else
DBHelper->>ChannelRecord: Construct simple SpannableString preview
end
end
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/src/org/commcare/utils/StringUtils.java (1)
55-59: Good implementation of the DP to pixel conversion utility.This is a standard and correct implementation for converting density-independent pixels to raw pixels in Android. The method properly retrieves system display metrics and applies the standard formula, including rounding to ensure integer pixel values.
However, consider adding a brief documentation comment explaining the purpose of this method and how it handles device density.
+ /** + * Converts a value in density-independent pixels (dp) to pixels (px) + * based on the device's screen density. + * + * @param dp The value in density-independent pixels to convert + * @return The equivalent value in pixels + */ public static float convertDpToPixel(float dp){ DisplayMetrics metrics = Resources.getSystem().getDisplayMetrics(); float px = dp * (metrics.densityDpi / 160f); return Math.round(px); }app/src/org/commcare/connect/ConnectDatabaseHelper.java (1)
864-883: Enhanced message preview with read/unread indicators for outgoing messages.The implementation correctly handles different preview scenarios:
- For unconsented channels - displays appropriate text
- For outgoing messages - adds read/unread icon based on message status
- For incoming messages - displays only the message text
- For empty channels - creates an empty preview
The code effectively uses SpannableString and ImageSpan to create a richer UI that distinguishes between read and unread outgoing messages, aligning with the PR objectives.
A few suggestions for improvement:
- Extract the magic number 14dp (line height) to a constant for better maintainability:
+ private static final float MESSAGE_ICON_SIZE_DP = 14f; // Then in the method: - float lineHeight = convertDpToPixel(14); + float lineHeight = convertDpToPixel(MESSAGE_ICON_SIZE_DP);
- Consider handling potential edge cases like very long messages more gracefully, perhaps by respecting word boundaries when trimming.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/org/commcare/android/database/connect/models/ConnectMessagingChannelRecord.java(3 hunks)app/src/org/commcare/connect/ConnectDatabaseHelper.java(2 hunks)app/src/org/commcare/utils/StringUtils.java(2 hunks)
🔇 Additional comments (6)
app/src/org/commcare/utils/StringUtils.java (1)
4-6: LGTM: Imports added to support the new utility method.The imports for
ResourcesandDisplayMetricsare appropriately added to support the new conversion utility method.app/src/org/commcare/connect/ConnectDatabaseHelper.java (2)
3-11: New imports added correctly to support the modified preview functionality.The imports have been properly organized and include all necessary dependencies for handling spannable text and drawables within the message preview.
874-880: Well-implemented drawable integration for message status indicators.The code correctly sets up the drawable based on the message confirmation status and applies appropriate sizing and positioning. The use of the newly created
convertDpToPixelmethod ensures consistent visual appearance across different device densities.app/src/org/commcare/android/database/connect/models/ConnectMessagingChannelRecord.java (3)
3-4: Added required import for SpannableString type.The SpannableString import is correctly added to support the field type change.
67-67: Field type changed from String to SpannableString to support rich text previews.This change is essential to support the new preview rendering approach with read/unread icons.
158-164:Details
✅ Verification successful
Updated getter and setter methods to use SpannableString type.
The getter and setter methods have been properly updated to match the new field type. This change ensures type consistency throughout the codebase when working with message previews.
Run the following script to verify that all uses of this getter method are compatible with the new return type:
🏁 Script executed:
#!/bin/bash # Search for uses of the getPreview method to ensure they can handle SpannableString rg -A 3 "getPreview\(\)" --type javaLength of output: 876
Verification Successful: SpannableString Update Approved
- The updated getter and setter in
app/src/org/commcare/android/database/connect/models/ConnectMessagingChannelRecord.javacorrectly use theSpannableStringtype.- The usage in
app/src/org/commcare/adapters/ChannelAdapter.java(i.e.,binding.tvChannelDescription.setText(channel.getPreview());) is compatible sinceSpannableStringimplementsCharSequence.No further changes are needed.
| preview = new SpannableString(lastMessage.getIsOutgoing()? " "+trimmed:trimmed); | ||
| if(lastMessage.getIsOutgoing()){ | ||
| Drawable drawable = lastMessage.getConfirmed() ? ContextCompat.getDrawable(context, R.drawable.ic_connect_message_read) : ContextCompat.getDrawable(context, R.drawable.ic_connect_message_unread); | ||
| float lineHeight = convertDpToPixel(14); | ||
| drawable.setBounds(0,0,(int) lineHeight, (int) lineHeight); | ||
| preview.setSpan(new ImageSpan(drawable), 0, 1, SPAN_EXCLUSIVE_EXCLUSIVE); | ||
| } | ||
| } else { | ||
| preview = new SpannableString(""); | ||
| } |
There was a problem hiding this comment.
can we make sure to format the code here as per our style file.
| return MarkupUtil.styleSpannable(c, ret); | ||
| } | ||
|
|
||
| public static float convertDpToPixel(float dp){ |
There was a problem hiding this comment.
Maybe better to create a separate Util class , something like -
public class DimensionUtils {
// I am a little worried about performance here if we end up calling this too many times in a loop or something and therefore might be best retained as a static variable
private static final float density = Resources.getSystem().getDisplayMetrics(). densityDpi;
public static int convertDpToPixel(float dp) { }
}
There was a problem hiding this comment.
Yeah I was thinking of creating new but I can see lots of Util files so thought to keep in StringUtils but anyways created new DimensionUtils file
Product Description
For Outgoing message: read/unread icon + message body
For incoming message: message body
When last message was outgoing:
When last message was incoming:
Technical Summary
https://dimagi.atlassian.net/browse/CCCT-711
Feature Flag
This is kind of feature where display of message preview changed
Safety Assurance
Safety story
Limited effect on safety, UI change
QA Plan
Need general review for message preview