Skip to content

Fix clipped emoji in reaction pills #339

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
klopez4212 merged 2 commits into from
Sep 28, 2026
Merged

Fix clipped emoji in reaction pills #339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1465ef2
Fix clipped native emoji in reaction pills
codex Sep 28, 2026
a713106
Update reaction pill sizing expectation
codex Sep 28, 2026
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
8 changes: 4 additions & 4 deletions src/features/messages/Messages.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,11 @@
}
.reactionNativeEmoji {
display: inline-block;
min-width: 12px;
min-width: 18px;
max-width: 96px;
height: 12px;
font-size: var(--text-caption);
line-height: 1;
height: 18px;
font-size: var(--text-body-sm);

@wesbillman wesbillman Sep 28, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Update the existing reaction-polish size assertion alongside this change

Switching native emoji to --text-body-sm changes the default computed size from 12px to 14px, but tests/browser/reactions-polish.spec.mjs:30 still asserts 12px on the same native glyph. That retained journey is selected in both browser projects, so adding the new 14px assertion in reactions.spec.mjs leaves the suite inconsistent and stops the older journey before its preview/toggle/inline-add checks. The completed Chromium 2/3 job already confirms this exact failure (expected 12px, received 14px): https://github.com/block/buzz-app/actions/runs/36412891794/job/108897041312 . Update the retained assertion to the intended 14px contract, keep its downstream behavior checks, and revalidate both reaction specs in Chromium and WebKit rather than reverting the glyph fix or weakening the assertions.

line-height: 18px;
text-align: center;
overflow: hidden;
text-overflow: ellipsis;
Expand Down
2 changes: 1 addition & 1 deletion tests/browser/reactions-polish.spec.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ test("reaction pills wrap, preview, toggle, and add from the inline control", as
const native = row.locator('button[data-reaction="👍"]');
await expect(custom.locator("img")).toHaveCSS("object-fit", "contain");
await expect(custom.locator("img")).toHaveCSS("width", "14px");
await expect(native.locator("span").first()).toHaveCSS("font-size", "12px");
await expect(native.locator("span").first()).toHaveCSS("font-size", "14px");
await expect(custom).toHaveCSS("height", "28px");
const plus = row.getByRole("button", { name: "Add reaction" });
await expect(plus).toHaveCSS("width", "28px");
Expand Down
31 changes: 31 additions & 0 deletions tests/browser/reactions.spec.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,37 @@ import { createServer } from "./vite-server.mjs";
import react from "@vitejs/plugin-react";
import { fileURLToPath } from "node:url";

test("native reaction emoji fit inside compact pills", async ({ page }) => {

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reuse the worker-scoped source fixture

This case uses the standard stateless source configuration and no mutable server middleware, but declaring it with the base test starts and stops another Vite server for each browser execution instead of sharing the worker-scoped server already exposed by the imported sourceTest. Use sourceTest with the relative fixture URL so the added Chromium/WebKit coverage does not impose redundant server and optimizer startup work on every browser gate.

AGENTS.md reference: AGENTS.md:L112-L115

Useful? React with 👍 / 👎.

const server = await createServer({
root: fileURLToPath(new URL("../../", import.meta.url)),
configFile: false,
envFile: false,
plugins: [react()],
logLevel: "error",
server: { host: "127.0.0.1", port: 0 },
});
try {
await server.listen();
await page.goto(
`http://127.0.0.1:${server.httpServer.address().port}/tests/fixtures/emoji.html?reactions&wrap`,
);
for (const emoji of ["👍", "🙌"]) {
const pill = page.locator(`button[data-reaction="${emoji}"]`);
const glyph = pill.locator("span").first();
await expect(pill).toHaveCSS("height", "28px");
await expect(glyph).toHaveCSS("min-width", "18px");
await expect(glyph).toHaveCSS("height", "18px");
await expect(glyph).toHaveCSS("font-size", "14px");

@chatgpt-codex-connector chatgpt-codex-connector Bot Sep 28, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reconcile the existing font-size regression check

Every Chromium and WebKit functional run also executes reactions-polish.spec.mjs, whose line 30 still requires this glyph to have font-size: 12px; the changed CSS and this new assertion compute 14px, so the mandatory browser gate will fail regardless of whether the new case passes. Update or consolidate the existing assertion as part of this change.

AGENTS.md reference: AGENTS.md:L144-L149

Useful? React with 👍 / 👎.

await expect(glyph).toHaveCSS("line-height", "18px");
}
await page.locator('button[data-reaction="👍"]').screenshot({
path: test.info().outputPath("native-reaction-emoji.png"),
});
} finally {
await server.close();
}
});

test("reaction plus opens a visible emoji-only picker, restores focus and publishes custom emoji", async ({
page,
}) => {
Expand Down
Loading