From 3e31fdb6a71f43774420e8da32452861296a263a Mon Sep 17 00:00:00 2001 From: Janne Mareike Koschinski Date: Mon, 2 May 2022 11:46:11 +0200 Subject: [PATCH] read receipts: improve tooltips to show names of users (#8438) --- .../views/rooms/ReadReceiptGroup.tsx | 54 ++++++++++--- src/i18n/strings/en_EN.json | 2 + .../views/rooms/ReadReceiptGroup-test.tsx | 80 +++++++++++++++++++ 3 files changed, 124 insertions(+), 12 deletions(-) create mode 100644 test/components/views/rooms/ReadReceiptGroup-test.tsx diff --git a/src/components/views/rooms/ReadReceiptGroup.tsx b/src/components/views/rooms/ReadReceiptGroup.tsx index 31a2a59dd3..c271d9f9ca 100644 --- a/src/components/views/rooms/ReadReceiptGroup.tsx +++ b/src/components/views/rooms/ReadReceiptGroup.tsx @@ -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 { @@ -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: ( + <> +
+ { _t("Seen by %(count)s people", { count: readReceipts.length }) } +
+
+ { tooltipText } +
+ + ), alignment: Alignment.TopRight, }); @@ -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); @@ -130,7 +160,7 @@ export function ReadReceiptGroup( showTwelveHour={isTwelveHour} /> ); - }); + }).reverse(); let remText: JSX.Element; const remainder = readReceipts.length - maxAvatars; diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 17d730b421..31c6f3383f 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -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", diff --git a/test/components/views/rooms/ReadReceiptGroup-test.tsx b/test/components/views/rooms/ReadReceiptGroup-test.tsx new file mode 100644 index 0000000000..28f1caa511 --- /dev/null +++ b/test/components/views/rooms/ReadReceiptGroup-test.tsx @@ -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 }); + }); + }); +});