-
Notifications
You must be signed in to change notification settings - Fork 769
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make read receipt avatar list more compact #5970
Make read receipt avatar list more compact #5970
Conversation
if (readReceipts.isNotEmpty()) { | ||
isVisible = true | ||
for (index in 0 until MAX_RECEIPT_DISPLAYED) { | ||
val receiptData = readReceipts.getOrNull(index) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I deleted this null check because I verified the call chain and List<ReadReceiptData>
can never have null elements.
@@ -1418,7 +1418,7 @@ | |||
<string name="merged_events_collapse">collapse</string> | |||
|
|||
<string name="generic_label_and_value">%1$s: %2$s</string> | |||
<string name="x_plus">%d+</string> | |||
<string name="x_plus">+%d</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this okay to modify an existing string in such a simple case? I checked all current translations and every instance is the same string as in the default locale, there is no custom translation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, this is OK. But with the new form, I think it would be better to have some avatars displayed, and after the "+3" text. Will check with design
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR.
Can you add a file for the changelog please?
See more info here: https://github.com/vector-im/element-android/blob/main/CONTRIBUTING.md#changelog
Then I think this PR can be merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Type of change
Content
Make the read receipt avatar list more compact by only displaying 3 avatars and have some negative spacing.
Motivation and context
Similar to how Element Web displays read receipts now: matrix-org/matrix-react-sdk#8389
Screenshots / GIFs
The same read receipt state in Element Web:
Tests
Tested devices
Checklist
Signed-off-by: Olivér Falvai [email protected]