Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

read receipts: improve tooltips to show names of users #8438

Merged
merged 4 commits into from
May 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 42 additions & 12 deletions src/components/views/rooms/ReadReceiptGroup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,11 @@ interface IAvatarPosition {
position: number;
}

function determineAvatarPosition(index: number, count: number, max: number): IAvatarPosition {
const firstVisible = Math.max(0, count - max);

if (index >= firstVisible) {
export function determineAvatarPosition(index: number, count: number, max: number): IAvatarPosition {
if (index < max) {
return {
hidden: false,
position: index - firstVisible,
position: Math.min(count, max) - index - 1,
};
} else {
return {
Expand All @@ -68,12 +66,49 @@ function determineAvatarPosition(index: number, count: number, max: number): IAv
}
}

export function readReceiptTooltip(members: string[], hasMore: boolean): string | null {
if (hasMore) {
return _t("%(members)s and more", {
members: members.join(", "),
});
} else if (members.length > 1) {
return _t("%(members)s and %(last)s", {
last: members.pop(),
members: members.join(", "),
});
} else if (members.length) {
return members[0];
} else {
return null;
}
}

export function ReadReceiptGroup(
{ readReceipts, readReceiptMap, checkUnmounting, suppressAnimation, isTwelveHour }: Props,
) {
const [menuDisplayed, button, openMenu, closeMenu] = useContextMenu();

// If we are above MAX_READ_AVATARS, we’ll have to remove a few to have space for the +n count.
const hasMore = readReceipts.length > MAX_READ_AVATARS;
const maxAvatars = hasMore
? MAX_READ_AVATARS_PLUS_N
: MAX_READ_AVATARS;

const tooltipMembers: string[] = readReceipts.slice(0, maxAvatars)
.map(it => it.roomMember?.name ?? it.userId);
const tooltipText = readReceiptTooltip(tooltipMembers, hasMore);

const [{ showTooltip, hideTooltip }, tooltip] = useTooltip({
label: _t("Seen by %(count)s people", { count: readReceipts.length }),
label: (
<>
<div className="mx_Tooltip_title">
{ _t("Seen by %(count)s people", { count: readReceipts.length }) }
</div>
<div className="mx_Tooltip_sub">
{ tooltipText }
</div>
</>
),
alignment: Alignment.TopRight,
});

Expand All @@ -97,11 +132,6 @@ export function ReadReceiptGroup(
);
}

// If we are above MAX_READ_AVATARS, we’ll have to remove a few to have space for the +n count.
const maxAvatars = readReceipts.length > MAX_READ_AVATARS
? MAX_READ_AVATARS_PLUS_N
: MAX_READ_AVATARS;

const avatars = readReceipts.map((receipt, index) => {
const { hidden, position } = determineAvatarPosition(index, readReceipts.length, maxAvatars);

Expand Down Expand Up @@ -130,7 +160,7 @@ export function ReadReceiptGroup(
showTwelveHour={isTwelveHour}
/>
);
});
}).reverse();

let remText: JSX.Element;
const remainder = readReceipts.length - maxAvatars;
Expand Down
2 changes: 2 additions & 0 deletions src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -1766,6 +1766,8 @@
"Preview": "Preview",
"View": "View",
"Join": "Join",
"%(members)s and more": "%(members)s and more",
"%(members)s and %(last)s": "%(members)s and %(last)s",
"Seen by %(count)s people|other": "Seen by %(count)s people",
"Seen by %(count)s people|one": "Seen by %(count)s person",
"Recently viewed": "Recently viewed",
Expand Down
80 changes: 80 additions & 0 deletions test/components/views/rooms/ReadReceiptGroup-test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { determineAvatarPosition, readReceiptTooltip } from "../../../../src/components/views/rooms/ReadReceiptGroup";

describe("ReadReceiptGroup", () => {
describe("TooltipText", () => {
it("returns '...and more' with hasMore", () => {
expect(readReceiptTooltip(["Alice", "Bob", "Charlie", "Dan", "Eve"], true))
.toEqual("Alice, Bob, Charlie, Dan, Eve and more");
expect(readReceiptTooltip(["Alice", "Bob", "Charlie", "Dan"], true))
.toEqual("Alice, Bob, Charlie, Dan and more");
expect(readReceiptTooltip(["Alice", "Bob", "Charlie"], true))
.toEqual("Alice, Bob, Charlie and more");
expect(readReceiptTooltip(["Alice", "Bob"], true))
.toEqual("Alice, Bob and more");
expect(readReceiptTooltip(["Alice"], true))
.toEqual("Alice and more");
expect(readReceiptTooltip([], false))
.toEqual(null);
});
it("returns a pretty list without hasMore", () => {
expect(readReceiptTooltip(["Alice", "Bob", "Charlie", "Dan", "Eve"], false))
.toEqual("Alice, Bob, Charlie, Dan and Eve");
expect(readReceiptTooltip(["Alice", "Bob", "Charlie", "Dan"], false))
.toEqual("Alice, Bob, Charlie and Dan");
expect(readReceiptTooltip(["Alice", "Bob", "Charlie"], false))
.toEqual("Alice, Bob and Charlie");
expect(readReceiptTooltip(["Alice", "Bob"], false))
.toEqual("Alice and Bob");
expect(readReceiptTooltip(["Alice"], false))
.toEqual("Alice");
expect(readReceiptTooltip([], false))
.toEqual(null);
});
});
describe("AvatarPosition", () => {
// The avatar slots are numbered from right to left
// That means currently, we’ve got the slots | 3 | 2 | 1 | 0 | each with 10px distance to the next one.
// We want to fill slots so the first avatar is in the left-most slot without leaving any slots at the right
// unoccupied.
it("to handle the non-overflowing case correctly", () => {
expect(determineAvatarPosition(0, 1, 4))
.toEqual({ hidden: false, position: 0 });

expect(determineAvatarPosition(0, 2, 4))
.toEqual({ hidden: false, position: 1 });
expect(determineAvatarPosition(1, 2, 4))
.toEqual({ hidden: false, position: 0 });

expect(determineAvatarPosition(0, 3, 4))
.toEqual({ hidden: false, position: 2 });
expect(determineAvatarPosition(1, 3, 4))
.toEqual({ hidden: false, position: 1 });
expect(determineAvatarPosition(2, 3, 4))
.toEqual({ hidden: false, position: 0 });

expect(determineAvatarPosition(0, 4, 4))
.toEqual({ hidden: false, position: 3 });
expect(determineAvatarPosition(1, 4, 4))
.toEqual({ hidden: false, position: 2 });
expect(determineAvatarPosition(2, 4, 4))
.toEqual({ hidden: false, position: 1 });
expect(determineAvatarPosition(3, 4, 4))
.toEqual({ hidden: false, position: 0 });
});

it("to handle the overflowing case correctly", () => {
expect(determineAvatarPosition(0, 6, 4))
.toEqual({ hidden: false, position: 3 });
expect(determineAvatarPosition(1, 6, 4))
.toEqual({ hidden: false, position: 2 });
expect(determineAvatarPosition(2, 6, 4))
.toEqual({ hidden: false, position: 1 });
expect(determineAvatarPosition(3, 6, 4))
.toEqual({ hidden: false, position: 0 });
expect(determineAvatarPosition(4, 6, 4))
.toEqual({ hidden: true, position: 0 });
expect(determineAvatarPosition(5, 6, 4))
.toEqual({ hidden: true, position: 0 });
});
});
});