diff --git a/desktop/src/features/messages/lib/reactionGlyphPresentation.test.mjs b/desktop/src/features/messages/lib/reactionGlyphPresentation.test.mjs new file mode 100644 index 0000000000..a6e3ae096a --- /dev/null +++ b/desktop/src/features/messages/lib/reactionGlyphPresentation.test.mjs @@ -0,0 +1,36 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { reactionGlyphPresentation } from "./reactionGlyphPresentation.ts"; + +test("uses compact layout only for one native emoji cluster", () => { + for (const emoji of ["πŸ˜€", "❀️", "πŸ‘πŸ½", "πŸ‘¨β€πŸ‘©β€πŸ‘§β€πŸ‘¦", "πŸ‡ΊπŸ‡Έ"]) { + assert.deepEqual(reactionGlyphPresentation(emoji), { + kind: "native", + text: emoji, + }); + } + + for (const text of ["a", "ship it", "πŸ˜€πŸ˜€", "πŸ‘©β€a"]) { + assert.deepEqual(reactionGlyphPresentation(text), { kind: "text", text }); + } +}); + +test("only unwraps valid outer shortcode delimiters for text fallbacks", () => { + assert.deepEqual(reactionGlyphPresentation(":missing_reaction:"), { + kind: "text", + text: "missing_reaction", + }); + assert.deepEqual(reactionGlyphPresentation(":party_parrot:"), { + kind: "text", + text: "party_parrot", + }); + for (const text of [ + ":ship it:", + "::", + ":missing_reaction", + "missing_reaction:", + ]) { + assert.deepEqual(reactionGlyphPresentation(text), { kind: "text", text }); + } +}); diff --git a/desktop/src/features/messages/lib/reactionGlyphPresentation.ts b/desktop/src/features/messages/lib/reactionGlyphPresentation.ts new file mode 100644 index 0000000000..dc71215c4e --- /dev/null +++ b/desktop/src/features/messages/lib/reactionGlyphPresentation.ts @@ -0,0 +1,22 @@ +import { isSingleNativeEmoji } from "@/shared/lib/emojiOnly"; + +const WRAPPED_SHORTCODE = /^:([a-z0-9_-]+):$/i; + +export type ReactionGlyphPresentation = + | { kind: "native"; text: string } + | { kind: "text"; text: string }; + +/** + * Chooses the no-image reaction fallback. A native emoji gets the compact glyph + * treatment; every other relay-valid reaction value gets text layout instead. + */ +export function reactionGlyphPresentation( + emoji: string, +): ReactionGlyphPresentation { + if (isSingleNativeEmoji(emoji)) { + return { kind: "native", text: emoji }; + } + + const shortcode = emoji.match(WRAPPED_SHORTCODE)?.[1]; + return { kind: "text", text: shortcode ?? emoji }; +} diff --git a/desktop/src/features/messages/ui/MessageReactions.tsx b/desktop/src/features/messages/ui/MessageReactions.tsx index d4bec8db6c..0cf2c650ff 100644 --- a/desktop/src/features/messages/ui/MessageReactions.tsx +++ b/desktop/src/features/messages/ui/MessageReactions.tsx @@ -3,6 +3,7 @@ import * as React from "react"; import { EmojiPicker } from "@/features/custom-emoji/ui/EmojiPicker"; import type { TimelineReaction } from "@/features/messages/types"; +import { reactionGlyphPresentation } from "@/features/messages/lib/reactionGlyphPresentation"; import { recordQuickReactionEmoji } from "@/features/messages/ui/useQuickReactionEmojis"; import { cn } from "@/shared/lib/cn"; import { emojiDisplayName } from "@/shared/lib/emojiName"; @@ -19,9 +20,15 @@ const REACTION_PILL_BASE_CLASSES = "inline-flex h-7 items-center rounded-full border text-xs font-medium leading-none transition-colors"; const REACTION_CUSTOM_GLYPH_CLASSES = "h-3.5 w-3.5"; const REACTION_NATIVE_GLYPH_CLASSES = "h-3 w-3 text-xs"; +const REACTION_TEXT_GLYPH_CLASSES = + "max-w-32 shrink-0 justify-start truncate text-left text-xs"; +const REACTION_POPOVER_NATIVE_GLYPH_CLASSES = "text-4xl"; +const REACTION_POPOVER_TEXT_GLYPH_CLASSES = + "w-full min-w-0 justify-start truncate text-left text-sm leading-snug"; const REACTION_COUNT_CLASSES = "text-muted-foreground"; const REACTION_NATIVE_COUNT_CLASSES = "text-muted-foreground translate-y-[0.5px]"; +const REACTION_TEXT_COUNT_CLASSES = "text-muted-foreground shrink-0"; const REACTION_PILL_HOVER_CLASSES = "hover:bg-primary/10 hover:text-foreground focus-visible:bg-primary/10 focus-visible:outline-hidden focus-visible:ring-2 focus-visible:ring-ring"; const BADGE_BURST_STABLE_FRAMES = 2; @@ -67,9 +74,11 @@ function isSameBadgeBurstRect( function EmojiGlyph({ reaction, className, + text, }: { reaction: TimelineReaction; className?: string; + text?: string; }) { const displayName = emojiDisplayName(reaction.emoji); if (reaction.emojiUrl) { @@ -94,7 +103,7 @@ function EmojiGlyph({ )} title={displayName} > - {reaction.emoji} + {text ?? reaction.emoji} ); } @@ -114,13 +123,25 @@ function formatReactionUsers(reaction: TimelineReaction): string { function ReactionPopoverContent({ reaction }: { reaction: TimelineReaction }) { const displayName = emojiDisplayName(reaction.emoji); const userText = formatReactionUsers(reaction); + const presentation = reaction.emojiUrl + ? null + : reactionGlyphPresentation(reaction.emoji); + const glyphClasses = reaction.emojiUrl + ? "h-12 w-12" + : presentation?.kind === "native" + ? REACTION_POPOVER_NATIVE_GLYPH_CLASSES + : REACTION_POPOVER_TEXT_GLYPH_CLASSES; return (
-
+
@@ -430,6 +451,29 @@ function ReactionPill({ }; const displayName = emojiDisplayName(reaction.emoji); + const presentation = reaction.emojiUrl + ? null + : reactionGlyphPresentation(reaction.emoji); + const glyphClasses = reaction.emojiUrl + ? REACTION_CUSTOM_GLYPH_CLASSES + : presentation?.kind === "native" + ? REACTION_NATIVE_GLYPH_CLASSES + : REACTION_TEXT_GLYPH_CLASSES; + const countClasses = reaction.emojiUrl + ? REACTION_COUNT_CLASSES + : presentation?.kind === "native" + ? REACTION_NATIVE_COUNT_CLASSES + : REACTION_TEXT_COUNT_CLASSES; + const pillContents = ( + <> + + + + ); if (reaction.users.length === 0) { return ( @@ -443,22 +487,7 @@ function ReactionPill({ ref={setPillRef} type="button" > - - + {pillContents} ); } @@ -484,22 +513,7 @@ function ReactionPill({ ref={setPillRef} type="button" > - - + {pillContents} diff --git a/desktop/src/shared/lib/emojiOnly.ts b/desktop/src/shared/lib/emojiOnly.ts index 5ab473f17b..a7a4f9f354 100644 --- a/desktop/src/shared/lib/emojiOnly.ts +++ b/desktop/src/shared/lib/emojiOnly.ts @@ -26,14 +26,14 @@ function buildNativeEmojiSet(): Set { return set; } -function isNativeEmojiCluster(cluster: string): boolean { +export function isNativeEmojiCluster(cluster: string): boolean { nativeEmojiSet ??= buildNativeEmojiSet(); return ( nativeEmojiSet.has(cluster) || /\p{Extended_Pictographic}/u.test(cluster) ); } -function readGrapheme(text: string, start: number): string { +export function readGrapheme(text: string, start: number): string { const firstCodePoint = text.codePointAt(start); if (firstCodePoint === undefined) { return ""; @@ -136,3 +136,19 @@ export function isEmojiOnlyMessage( return sawEmoji; } +/** True only when the entire value is one native emoji grapheme cluster. */ +export function isSingleNativeEmoji(value: string): boolean { + if (!value) return false; + const cluster = readGrapheme(value, 0); + if (cluster !== value) return false; + + nativeEmojiSet ??= buildNativeEmojiSet(); + if (nativeEmojiSet.has(cluster)) return true; + + // Keep future pictographs working without accepting arbitrary text that a + // malformed ZWJ sequence caused readGrapheme() to consume (for example, + // `πŸ‘©β€a`). Every ZWJ component must itself be pictographic. + return /^\p{Extended_Pictographic}(?:\ufe0f|[\u{1f3fb}-\u{1f3ff}])?(?:\u200d\p{Extended_Pictographic}(?:\ufe0f|[\u{1f3fb}-\u{1f3ff}])?)*$/u.test( + cluster, + ); +} diff --git a/desktop/tests/e2e/reaction-names.spec.ts b/desktop/tests/e2e/reaction-names.spec.ts index 028bb1645d..9670c0690e 100644 --- a/desktop/tests/e2e/reaction-names.spec.ts +++ b/desktop/tests/e2e/reaction-names.spec.ts @@ -15,6 +15,9 @@ const MAX_REACTION_AVATAR_URL = 'data:image/svg+xml,%3Csvg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16"%3E%3Crect width="16" height="16" rx="4" fill="%23e5484d"/%3E%3C/svg%3E'; const SHORT_REACTION_AVATAR_URL = 'data:image/svg+xml,%3Csvg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16"%3E%3Crect width="16" height="16" rx="4" fill="%2300a36c"/%3E%3C/svg%3E'; +const UNRESOLVED_SHORTCODE = ":missing_reaction:"; +const LONG_LITERAL_REACTION = + "this-is-a-deliberately-long-literal-reaction-that-must-truncate-without-moving-or-overlapping-the-count"; const SCREENSHOT_DIR = process.env.REACTION_POPOVER_SCREENSHOT_DIR ?? "test-results/reaction-popover-screenshots"; @@ -55,6 +58,97 @@ async function capturePopover( }); } +async function emitReaction( + page: import("@playwright/test").Page, + content: string, + pubkey: string, +): Promise { + await page.evaluate( + ({ content, pubkey, targetId }) => { + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content, + extraTags: [["e", targetId]], + kind: 7, + pubkey, + }); + }, + { content, pubkey, targetId: REACTION_TARGET_EVENT_ID }, + ); +} + +async function expectFallbackPill( + page: import("@playwright/test").Page, + reaction: string, + visibleText: string, +): Promise { + const pill = reactionTargetRow(page).getByRole("button", { + name: `Toggle ${reaction} reaction`, + }); + await expect(pill).toBeVisible(); + await expect(pill).toHaveAttribute("title", reaction); + await expect(pill.locator("img")).toHaveCount(0); + + const glyph = pill.locator("span[title]"); + await expect(glyph).toHaveText(visibleText); + const [pillRect, glyphRect, countRect] = await Promise.all([ + pill.boundingBox(), + glyph.boundingBox(), + pill.locator(".buzz-animated-count").boundingBox(), + ]); + expect(pillRect && glyphRect && countRect).toBeTruthy(); + if (!pillRect || !glyphRect || !countRect) return; + expect(glyphRect.x + glyphRect.width).toBeLessThanOrEqual(countRect.x); + expect( + glyphRect.x >= pillRect.x && + countRect.x + countRect.width <= pillRect.x + pillRect.width, + ).toBeTruthy(); +} + +async function expectFallbackPopover( + page: import("@playwright/test").Page, + reaction: string, + visibleText: string, + screenshotName: string, +): Promise { + const pill = reactionTargetRow(page).getByRole("button", { + name: `Toggle ${reaction} reaction`, + }); + await pill.hover(); + + const popover = page.locator("[data-radix-popper-content-wrapper]").filter({ + has: page + .getByTestId("reaction-popover-name") + .filter({ hasText: reaction }), + }); + await expect(popover).toBeVisible(); + await expect(popover.getByTestId("reaction-popover-name")).toHaveText( + reaction, + ); + + const container = popover.getByTestId("reaction-popover-glyph-container"); + const glyph = container.locator("span[title]"); + await expect(glyph).toHaveText(visibleText); + await expect(glyph).toHaveAttribute("title", reaction); + const [containerRect, glyphRect] = await Promise.all([ + container.boundingBox(), + glyph.boundingBox(), + ]); + expect(containerRect && glyphRect).toBeTruthy(); + if (containerRect && glyphRect) { + expect(glyphRect.x).toBeGreaterThanOrEqual(containerRect.x); + expect(glyphRect.x + glyphRect.width).toBeLessThanOrEqual( + containerRect.x + containerRect.width, + ); + expect(glyphRect.y).toBeGreaterThanOrEqual(containerRect.y); + expect(glyphRect.y + glyphRect.height).toBeLessThanOrEqual( + containerRect.y + containerRect.height, + ); + } + await expect(container).toHaveCSS("overflow", "hidden"); + await capturePopover(page, popover, screenshotName); +} + test.beforeEach(async ({ page }) => { await installMockBridge(page, { searchProfiles: [ @@ -168,3 +262,68 @@ test("maximum-length reaction name wraps inside a fixed-width popover", async ({ await waitForImage(avatar); await capturePopover(page, popover, "max-length-after.png"); }); + +test("literal fallback reactions do not overlap their counts", async ({ + page, +}) => { + await page.goto("/"); + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + await page.waitForFunction( + () => + window.__BUZZ_E2E_HAS_MOCK_LIVE_SUBSCRIPTION__?.({ + channelName: "general", + kind: 7, + }) === true, + ); + + for (const reaction of [ + UNRESOLVED_SHORTCODE, + "ship it", + LONG_LITERAL_REACTION, + ]) { + await emitReaction(page, reaction, BOB_PUBKEY); + await emitReaction(page, reaction, "c".repeat(64)); + } + + await expectFallbackPill(page, UNRESOLVED_SHORTCODE, "missing_reaction"); + await expectFallbackPill(page, "ship it", "ship it"); + const longPill = reactionTargetRow(page).getByRole("button", { + name: `Toggle ${LONG_LITERAL_REACTION} reaction`, + }); + const longGlyph = longPill.locator("span[title]"); + await expectFallbackPill(page, LONG_LITERAL_REACTION, LONG_LITERAL_REACTION); + await expect(longGlyph).toHaveCSS("max-width", "128px"); + await expect(longGlyph).toHaveCSS("text-align", "left"); + await expect + .poll(() => + longGlyph.evaluate( + (element) => element.scrollWidth > element.clientWidth, + ), + ) + .toBe(true); + + await expectFallbackPopover( + page, + UNRESOLVED_SHORTCODE, + "missing_reaction", + "unresolved-shortcode-after.png", + ); + await expectFallbackPopover( + page, + "ship it", + "ship it", + "literal-text-after.png", + ); + await expectFallbackPopover( + page, + LONG_LITERAL_REACTION, + LONG_LITERAL_REACTION, + "long-literal-after.png", + ); + + await reactionTargetRow(page).screenshot({ + animations: "disabled", + path: "test-results/reaction-text-fallback.png", + }); +});