Skip to content

Commit

Permalink
read receipts: improve tooltips to show names of users (matrix-org#8438)
Browse files Browse the repository at this point in the history
  • Loading branch information
justjanne committed May 2, 2022
1 parent 4831129 commit 3e31fdb
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 12 deletions.
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 });
});
});
});

0 comments on commit 3e31fdb

Please sign in to comment.