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

Author: justjanne

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: (
+            <>
+                <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,
     });
 
@@ -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;

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",

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 });
+        });
+    });
+});