mirror of
https://github.com/block/buzz.git
synced 2026-10-27 12:40:30 +00:00
## Summary - Let Markdown tables use the available message width instead of their maximum-content width, inheriting the renderer's existing `wrap-anywhere` handling for long tokens and links. - Top-align header cells as well as body cells. Give cells a `min-w-24` readability floor so short labels do not collapse into one-letter columns; retain the existing table-local scrollbar when many columns genuinely cannot fit. - Preserve semantic table markup, links, inline code, and surrounding message layout. Add three browser regressions through the real message renderer (channel, narrow thread, and many-column overflow). ### Related issue Fixes #5313. No matching open table-readability PR found. Owner-authorized task channel: `819b36d5-7371-4ed0-bf9b-d461d723779e` Source: buzz://message?channel=819b36d5-7371-4ed0-bf9b-d461d723779e&id=3bf2d1bd5df28fcb481e71159dc1be2b3888918b05f67e270354fbfea65a8c90 ### Testing Original production candidate: `4427f2e6f9ef85a01a267fe9eabd1cf13cacfb78`; base: `7a9a5233d9d755e715be0c585cf7850e935d28cf`. - `pnpm -C desktop check` — passed. - `pnpm -C desktop test` — 6,110 passed, none skipped. - `pnpm -C desktop typecheck` — passed. - `BUZZ_PROTECTED_BUILD_OUTPUT=<isolated-production-directory> pnpm -C desktop build` — passed. - `pnpm -C desktop build:e2e --outDir <isolated-candidate-directory>` — passed. - `just file-size-check` and `git diff --check` — passed. - Chromium / mock Tauri bridge: all 3 new Playwright tests passed against the fixed candidate build. Both wrapping tests fail on the unchanged base build, while the overflow-fallback test passes. Each browser run used a unique output directory and a non-reused local server pinned to its build directory. - At 883px channel message width: table scroll width **1,395 → 883px**. At 292px thread message width: **1,395 → 292px**. All cells top-aligned; long tokens/URLs wrap, link destinations and code text remain intact, short labels stay on one line, and document width remains 1,280px. - Focused fresh-frame review checked actual before/after screenshots and the complete diff. It caught one-letter label wrapping in an early width-only candidate; the final cell-width floor and regression assertion address that finding. Limitations: native Tauri/WebKit and relay-backed integration were not exercised for this CSS-only delta. Repository-wide `just ci` was attempted but exceeded the local command budget during unrelated Rust compilation; it is **not** reported green. The full affected TypeScript package gates above passed on the exact candidate. Production/E2E builds retain existing chunk-size and mixed static/dynamic import warnings. Before/after screenshots are posted below using the repository's screenshot publication script. No merge, production installation, or runtime restart is requested. ### CI-driven test-only follow-up Current head: `925d964b6cf31ba704d74baf73769110774b2789`. Smoke shard 3 exposed an older test in `messaging.spec.ts:457` that still required three columns of ordinary prose to overflow horizontally ([failure log](https://github.com/block/buzz/actions/runs/33771325404/job/100702242706)). This expectation contradicts the intended wrapping change. Updated its name and assertion to require containment; the separate many-column local-scroll test is unchanged. No production code or harness settings changed in this follow-up. - Affected browser validation: **4/4 passed** (updated existing prose/narrow table case plus the three new channel/thread/overflow cases). - Reused the immutable `4427f2e6` E2E build on a fresh non-reused server; production source is unchanged at the current head. The package/build checks above remain evidence for that unchanged production tree, not a claim of rerunning the full suite at the new SHA. - Targeted Biome check, file-size gate, and `git diff --check` passed. - Current-head CI: https://github.com/block/buzz/actions/runs/33773490446 - Prior unrelated Bestie baseline failure and reproduction: https://github.com/block/buzz/pull/7279#issuecomment-5528077181 - Security authorization and review must target the **current** head, not the old screenshot/build SHA. A Block organization member must comment exactly `@buzz-security-review 925d964b6cf31ba704d74baf73769110774b2789`. --------- Signed-off-by: Logan Johnson <loganj@squareup.com>
155 lines
6.2 KiB
TypeScript
155 lines
6.2 KiB
TypeScript
import { expect, test } from "@playwright/test";
|
|
import { waitForAnimations } from "../helpers/animations";
|
|
import { installMockBridge } from "../helpers/bridge";
|
|
|
|
const token = "0123456789abcdef".repeat(8);
|
|
const url = `https://example.com/reports/${token}`;
|
|
const prose =
|
|
"Review the rollout notes and confirm that each owner can read the complete status without scrolling sideways. Keep the next action beside its owner, even when this description spans several lines.";
|
|
const content = `Table readability fixture
|
|
|
|
| Owner | Status and next action with enough detail to span multiple lines in a narrow pane |
|
|
| --- | --- |
|
|
| Alice | ${prose} |
|
|
| Bob | [Read the complete rollout notes and review checklist](${url}) and then confirm the next step. |
|
|
| Token | ${token} |
|
|
| Link | <${url}> |
|
|
| Code | \`git diff --check\` and **review** the result. |
|
|
|
|
Surrounding paragraph stays in the message layout.`;
|
|
|
|
for (const surface of ["channel", "thread"] as const) {
|
|
test(`markdown tables wrap and stay contained in the ${surface}`, async ({
|
|
page,
|
|
}, testInfo) => {
|
|
await page.setViewportSize({ width: 1280, height: 1440 });
|
|
await installMockBridge(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",
|
|
}),
|
|
);
|
|
const root = await page.evaluate((body) => {
|
|
const root = window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({
|
|
channelName: "general",
|
|
content: body,
|
|
});
|
|
if (!root) throw new Error("Mock message was not emitted");
|
|
return root.id;
|
|
}, content);
|
|
|
|
const timelineMessage = page
|
|
.getByTestId("message-timeline")
|
|
.locator(`[data-message-id="${root}"]`);
|
|
await expect(timelineMessage).toBeVisible();
|
|
if (surface === "thread") {
|
|
await timelineMessage.hover();
|
|
await page.getByTestId(`reply-message-${root}`).click();
|
|
await expect(page.getByTestId("message-thread-panel")).toBeVisible();
|
|
}
|
|
const scope = page.getByTestId(
|
|
surface === "thread" ? "message-thread-panel" : "message-timeline",
|
|
);
|
|
const markdown = scope
|
|
.locator(".message-markdown")
|
|
.filter({ hasText: "Table readability fixture" });
|
|
const block = markdown.locator("[data-table-block]");
|
|
await expect(block).toBeVisible();
|
|
await page.mouse.move(0, 0);
|
|
await waitForAnimations(page);
|
|
await markdown.screenshot({ path: testInfo.outputPath(`${surface}.png`) });
|
|
const metrics = await block.evaluate((element) => {
|
|
const table = element.querySelector("table");
|
|
if (!table) throw new Error("Semantic table missing");
|
|
const label = document.createRange();
|
|
label.selectNodeContents(table.rows[0].cells[0]);
|
|
return {
|
|
labelLines: label.getClientRects().length,
|
|
width: element.clientWidth,
|
|
scrollWidth: element.scrollWidth,
|
|
tableWidth: table.getBoundingClientRect().width,
|
|
alignments: Array.from(
|
|
table.querySelectorAll("th, td"),
|
|
(cell) => getComputedStyle(cell).verticalAlign,
|
|
),
|
|
rowHeight: table.rows[1].getBoundingClientRect().height,
|
|
lineHeight: Number.parseFloat(getComputedStyle(table).lineHeight),
|
|
pageWidth: document.documentElement.clientWidth,
|
|
pageScrollWidth: document.documentElement.scrollWidth,
|
|
};
|
|
});
|
|
await testInfo.attach("layout", {
|
|
body: JSON.stringify(metrics, null, 2),
|
|
contentType: "application/json",
|
|
});
|
|
expect(metrics.width).toBeGreaterThan(250);
|
|
if (surface === "thread") expect(metrics.width).toBeLessThan(500);
|
|
expect(metrics.scrollWidth).toBeLessThanOrEqual(metrics.width + 1);
|
|
expect(metrics.tableWidth).toBeLessThanOrEqual(metrics.width + 1);
|
|
expect(metrics.alignments.every((value) => value === "top")).toBe(true);
|
|
expect(metrics.labelLines).toBe(1);
|
|
expect(metrics.rowHeight).toBeGreaterThan(metrics.lineHeight * 2);
|
|
expect(metrics.pageScrollWidth).toBe(metrics.pageWidth);
|
|
await expect(block.locator("tbody tr")).toHaveCount(5);
|
|
await expect(block.getByRole("link")).toHaveCount(2);
|
|
for (const link of await block.getByRole("link").all()) {
|
|
await expect(link).toHaveAttribute("href", url);
|
|
}
|
|
await expect(block.locator("code")).toHaveText("git diff --check");
|
|
await expect(
|
|
block.locator("td").filter({ hasText: token }).first(),
|
|
).toHaveText(token);
|
|
await expect(markdown.locator("p").last()).toHaveText(
|
|
"Surrounding paragraph stays in the message layout.",
|
|
);
|
|
});
|
|
}
|
|
|
|
test("unavoidably wide tables scroll locally without losing cells", async ({
|
|
page,
|
|
}) => {
|
|
await installMockBridge(page);
|
|
await page.goto("/");
|
|
await page.getByTestId("channel-general").click();
|
|
await page.waitForFunction(() =>
|
|
window.__BUZZ_E2E_HAS_MOCK_LIVE_SUBSCRIPTION__?.({
|
|
channelName: "general",
|
|
}),
|
|
);
|
|
const columns = Array.from({ length: 40 }, (_, i) => `C${i}`);
|
|
const wide = [columns, columns.map(() => "---"), columns]
|
|
.map((row) => `| ${row.join(" | ")} |`)
|
|
.join("\n");
|
|
await page.evaluate((body) => {
|
|
window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({
|
|
channelName: "general",
|
|
content: `Wide table fixture\n\n${body}`,
|
|
});
|
|
}, wide);
|
|
const block = page
|
|
.getByTestId("message-timeline")
|
|
.locator(".message-markdown")
|
|
.filter({ hasText: "Wide table fixture" })
|
|
.locator("[data-table-block]");
|
|
await expect(block.locator("td")).toHaveCount(40);
|
|
const metrics = await block.evaluate((element) => {
|
|
element.scrollLeft = element.scrollWidth;
|
|
return {
|
|
width: element.clientWidth,
|
|
scrollWidth: element.scrollWidth,
|
|
scrollLeft: element.scrollLeft,
|
|
overflow: getComputedStyle(element).overflowX,
|
|
pageWidth: document.documentElement.clientWidth,
|
|
pageScrollWidth: document.documentElement.scrollWidth,
|
|
};
|
|
});
|
|
expect(metrics.scrollWidth).toBeGreaterThan(metrics.width);
|
|
expect(metrics.scrollLeft).toBeGreaterThan(0);
|
|
expect(metrics.overflow).toBe("auto");
|
|
expect(metrics.pageScrollWidth).toBe(metrics.pageWidth);
|
|
await expect(block.locator("td").last()).toHaveText("C39");
|
|
});
|