Skip to content

Make read receipt avatar list more compact#5970

Merged
bmarty merged 4 commits into
element-hq:developfrom
ofalvai:feature/ofa/read-receipts-design
May 31, 2022
Merged

Make read receipt avatar list more compact#5970
bmarty merged 4 commits into
element-hq:developfrom
ofalvai:feature/ofa/read-receipts-design

Conversation

@ofalvai
Copy link
Copy Markdown
Contributor

@ofalvai ofalvai commented May 7, 2022

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

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

Before After
Screenshot_20220507-201302__01 Screenshot_20220507-201249__01
Screenshot_20220507-201024__02 Screenshot_20220507-201027__02

The same read receipt state in Element Web:

image

Tests

  • Find message with 1 read receipt and check the single avatar
  • Find message with 2 or 3 read receipts and check order of avatars
  • Find message with 3+ read receipts and check order of avatars, as well as which 3 avatars are shown

Tested devices

  • Physical
  • Emulator
  • OS version(s): 11

Checklist

Signed-off-by: Olivér Falvai ofalvai@gmail.com

@ofalvai ofalvai changed the title Make read receipt avatar list compact Make read receipt avatar list more compact May 7, 2022
if (readReceipts.isNotEmpty()) {
isVisible = true
for (index in 0 until MAX_RECEIPT_DISPLAYED) {
val receiptData = readReceipts.getOrNull(index)
Copy link
Copy Markdown
Contributor Author

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.


<string name="generic_label_and_value">%1$s: %2$s</string>
<string name="x_plus">%d+</string>
<string name="x_plus">+%d</string>
Copy link
Copy Markdown
Contributor Author

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.

Copy link
Copy Markdown
Member

@bmarty bmarty May 9, 2022

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

@ouchadam ouchadam added X-Needs-Design May require input from the design team Z-Community-PR Issue is solved by a community member's PR labels May 9, 2022
Copy link
Copy Markdown
Member

@bmarty bmarty left a 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.

Copy link
Copy Markdown
Member

@bmarty bmarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@bmarty bmarty enabled auto-merge May 31, 2022 16:14
@bmarty bmarty merged commit e6beb73 into element-hq:develop May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

X-Needs-Design May require input from the design team Z-Community-PR Issue is solved by a community member's PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants