mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
## Problem Two related gaps in global back/forward navigation. Fixes #3775. 1. The keyboard shortcuts almost never fire in real use — users fall back to clicking the toolbar chevrons and assume the shortcuts don't exist. 2. On macOS, mouse back/forward buttons (X1/X2) and horizontal swipe gestures do nothing, although they navigate in every browser and in Slack. **Duplicate check:** searched open PRs and issues — none found beyond #3775 (filed alongside this fix). #3078 / #3377 are next/previous-*channel* navigation, a different feature. ## Root causes **Keyboard:** `useBackForwardControls`'s keydown handler bailed whenever the event target was editable — but `useComposerAutofocus` deliberately focuses the message composer (a ProseMirror contenteditable) on mount and on every channel switch. In steady state focus almost always lives in the composer, so the chords were silently swallowed. Invisible to CI because `navigation.spec.ts` only ever clicked the `global-back` / `global-forward` buttons, never pressed the keys. **Mouse/swipe:** on macOS, WKWebView never delivers X1/X2 button events or swipe gestures to the page (Safari handles them natively in the app layer, not in page JS), and Buzz had no native handler. ## Fix ### Keyboard chords (web layer) Match the existing platform chord regardless of the event target and drop the editable-target guard: - `⌘[` / `⌘]` have no text-editing semantics in macOS text fields, and the TipTap/StarterKit editor config binds no `Mod-[` / `Mod-]` shortcuts (checked `useRichTextEditor.ts` — list indentation is Tab/Shift-Tab). - `preventDefault()` keeps the chord out of the editor — asserted in the e2e test. This matches browsers and Slack, where back/forward chords work while a text field is focused. Chord matching is extracted into a pure helper, `app/navigation/backForwardChords.ts`, so it can be unit tested; behavior (bindings, modifier exclusivity, `code`-based matching for non-US layouts) is unchanged. ### macOS mouse buttons and swipe gestures (native layer) An NSEvent local monitor in `mouse_nav.rs` catches what the webview can't see and emits a `mouse-nav` Tauri event to the main window (`emit_to`, so navigation stays scoped if multi-window ever lands) that the frontend acts on. Two AppKit event shapes map to navigation: - `otherMouseUp` with button 3/4 — mice whose X1/X2 buttons arrive as plain button events. These are swallowed after emitting so nothing downstream double-handles them. - `swipe` with a horizontal delta — AppKit's page-swipe gesture (`swipeWithEvent:`): `deltaX > 0` back, `deltaX < 0` forward. Sent by mouse drivers that synthesize a page-swipe gesture for the back/forward buttons instead of button-3/4 events (the hardware this was verified on). Stock Apple trackpad and Magic Mouse swipes arrive as phased scroll-wheel events instead, which this PR does not handle — that path (`ScrollWheel` + `trackSwipeEventWithOptions:`, which also needs scroll-edge detection) is deferred to a follow-up. Swipes are passed through (swallowing mid-gesture events could confuse AppKit gesture tracking). The swipe path was verified end to end on hardware whose back/forward buttons emit only swipe gestures, never button-3/4 events — an instrumented event monitor confirmed the events arrive as `NSEventType::Swipe` with `deltaX ±1`, and navigation worked after mapping them. ## Tests - **13 unit tests** for the web-side chord matcher (`backForwardChords.test.mjs`): supported chords, modifier exclusivity, `code` fallback, and preservation of line-editing shortcuts. - **6 Rust unit tests** for the native mapping helpers (`mouse_nav.rs`): button 3/4 directions, other buttons ignored, swipe delta sign → direction, zero-delta (gesture-begin) ignored. - **e2e regression case** in `navigation.spec.ts`: presses the platform chord *while the composer is focused* — the missing coverage. Verified it fails against the pre-fix implementation and passes with the fix. - Full desktop unit suite: 3832/3832 pass. Full Rust suite (`cargo test`, buzz-desktop): 1888 passed / 0 failed. `pnpm typecheck`, `biome check`, `pnpm check`, `cargo fmt --check`, `cargo clippy`: clean (no new warnings). - Full Playwright e2e: 958 passed; 6 failures are relay-infrastructure tests (live relay seeding / relay state seam) that fail identically without this change — `navigation.spec.ts` is fully green. ## Manual test 1. Open a channel, then another (composer autofocuses on each switch). 2. `⌘[` — returns to the previous channel; `⌘]` — forward again. Typing `[` / `]` in the composer inserts normally. 3. Mouse back/forward buttons navigate the same way, from anywhere in the window (verified on macOS on hardware using both event shapes). ## Update — 2026-07-31 Removed the redundant DOM mouse-button handler after verifying it was unnecessary. The native macOS path remains unchanged and was revalidated manually. --------- Signed-off-by: npub1yvnq5equak5errqpku8stskushny9wsvt0fc2ywcpwt79yslwaqswe7tse <23260a641ceda9918c01b70f05c2dc85e642ba0c5bd38511d80b97e2921f7741@buzz.block.builderlab.xyz> Signed-off-by: Matheus Iser <matheusiser@squareup.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Co-authored-by: npub1yvnq5equak5errqpku8stskushny9wsvt0fc2ywcpwt79yslwaqswe7tse <23260a641ceda9918c01b70f05c2dc85e642ba0c5bd38511d80b97e2921f7741@buzz.block.builderlab.xyz> Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz> Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
428 lines
15 KiB
TypeScript
428 lines
15 KiB
TypeScript
import { expect, test } from "@playwright/test";
|
|
|
|
import { installMockBridge } from "../helpers/bridge";
|
|
import { openSettings } from "../helpers/settings";
|
|
|
|
const ENGINEERING_CHANNEL_ID = "1c7e1c02-87bb-5e88-b2da-5a7a9432d0c9";
|
|
const WATERCOLOR_CHANNEL_ID = "a27e1ee9-76a6-5bdf-a5d5-1d85610dad11";
|
|
const FORUM_POST_ID = "mock-forum-release-thread";
|
|
const FORUM_REPLY_ID = "mock-forum-release-reply";
|
|
|
|
test.beforeEach(async ({ page }) => {
|
|
await installMockBridge(page);
|
|
});
|
|
|
|
async function navigateToWorkflows(page: import("@playwright/test").Page) {
|
|
await page.goto("/");
|
|
await page.getByTestId("open-workflows-view").click();
|
|
await expect(page).toHaveURL(/#\/workflows$/);
|
|
await expect(page.getByTestId("workflows-view")).toBeVisible();
|
|
}
|
|
|
|
async function createWorkflow(
|
|
page: import("@playwright/test").Page,
|
|
name: string,
|
|
) {
|
|
await page.getByRole("button", { name: "Create Workflow" }).click();
|
|
const dialog = page.getByRole("dialog");
|
|
await expect(dialog).toBeVisible();
|
|
await dialog.getByLabel("Workflow name").fill(name);
|
|
await dialog.getByRole("button", { name: "Add step" }).click();
|
|
await dialog.getByRole("button", { name: "Create" }).click();
|
|
await expect(dialog).not.toBeVisible();
|
|
}
|
|
|
|
test("global back and forward move across channel routes", async ({ page }) => {
|
|
await page.goto("/");
|
|
|
|
await page.getByTestId("channel-general").click();
|
|
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
|
|
|
await page.getByTestId("channel-random").click();
|
|
await expect(page.getByTestId("chat-title")).toHaveText("random");
|
|
|
|
await page.getByTestId("global-back").click();
|
|
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
|
|
|
await page.getByTestId("global-forward").click();
|
|
await expect(page.getByTestId("chat-title")).toHaveText("random");
|
|
});
|
|
|
|
test("back/forward keyboard chords work while the composer has focus", async ({
|
|
page,
|
|
}) => {
|
|
const backChord = process.platform === "darwin" ? "Meta+[" : "Alt+ArrowLeft";
|
|
const forwardChord =
|
|
process.platform === "darwin" ? "Meta+]" : "Alt+ArrowRight";
|
|
|
|
await page.goto("/");
|
|
|
|
await page.getByTestId("channel-general").click();
|
|
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
|
|
|
await page.getByTestId("channel-random").click();
|
|
await expect(page.getByTestId("chat-title")).toHaveText("random");
|
|
|
|
// The composer autofocuses on channel switch; make the regression
|
|
// condition explicit by clicking into it. The chords must still fire
|
|
// from inside the contenteditable (#3775).
|
|
await page.getByTestId("message-input").click();
|
|
await expect(page.getByTestId("message-input")).toBeFocused();
|
|
|
|
await page.keyboard.press(backChord);
|
|
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
|
|
|
await page.keyboard.press(forwardChord);
|
|
await expect(page.getByTestId("chat-title")).toHaveText("random");
|
|
|
|
// preventDefault kept the chord out of the editor — no stray characters.
|
|
await expect(page.getByTestId("message-input")).toHaveText("");
|
|
});
|
|
|
|
// FIXME: the forum post "Back to posts" header renders under the fixed top
|
|
// chrome drag region, which intercepts the click. Pre-existing breakage —
|
|
// this spec file was never registered in playwright.config.ts until now.
|
|
// The header-chrome rework (PR #941) covers this overlap class.
|
|
test.fixme("direct forum thread links close back to the forum route", async ({
|
|
page,
|
|
}) => {
|
|
await page.goto(
|
|
`/#/channels/${WATERCOLOR_CHANNEL_ID}/posts/${FORUM_POST_ID}`,
|
|
);
|
|
|
|
await expect(page.getByTestId("chat-title")).toHaveText("watercooler");
|
|
await expect(
|
|
page.getByRole("button", { name: "Back to posts" }),
|
|
).toBeVisible();
|
|
|
|
await page.getByRole("button", { name: "Back to posts" }).click();
|
|
|
|
await expect(page).toHaveURL(
|
|
/#\/channels\/a27e1ee9-76a6-5bdf-a5d5-1d85610dad11$/,
|
|
);
|
|
await expect(
|
|
page.getByText("Release checklist: async feedback thread."),
|
|
).toBeVisible();
|
|
});
|
|
|
|
test("direct workflow detail links close back to workflows", async ({
|
|
page,
|
|
}) => {
|
|
const workflowName = `workflow_nav_${Date.now()}`;
|
|
|
|
await navigateToWorkflows(page);
|
|
await createWorkflow(page, workflowName);
|
|
|
|
const workflowCard = page
|
|
.locator('[data-testid^="workflow-card-"]')
|
|
.filter({ hasText: workflowName })
|
|
.first();
|
|
const workflowTestId = await workflowCard.getAttribute("data-testid");
|
|
const workflowId = workflowTestId?.replace("workflow-card-", "");
|
|
|
|
expect(workflowId).toBeTruthy();
|
|
|
|
await page.goto(`/#/workflows/${workflowId}`);
|
|
|
|
await expect(page.getByTestId("workflow-detail-panel")).toBeVisible();
|
|
await page.getByRole("button", { name: "Close detail panel" }).click();
|
|
|
|
await expect(page).toHaveURL(/#\/workflows$/);
|
|
await expect(page.getByTestId("workflows-view")).toBeVisible();
|
|
});
|
|
|
|
test("forum reply deep links survive reload", async ({ page }) => {
|
|
await page.goto(
|
|
`/#/channels/${WATERCOLOR_CHANNEL_ID}/posts/${FORUM_POST_ID}?replyId=${FORUM_REPLY_ID}`,
|
|
);
|
|
|
|
await expect(page.getByTestId("chat-title")).toHaveText("watercooler");
|
|
await expect(
|
|
page.getByText("Looks good to me. We should ship it."),
|
|
).toBeVisible();
|
|
|
|
await page.reload();
|
|
|
|
await expect(page.getByTestId("chat-title")).toHaveText("watercooler");
|
|
await expect(
|
|
page.getByText("Looks good to me. We should ship it."),
|
|
).toBeVisible();
|
|
});
|
|
|
|
test("back and forward restore open thread panels", async ({ page }) => {
|
|
await page.goto("/");
|
|
|
|
await page.getByTestId("channel-general").click();
|
|
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
|
|
|
const rootMessage = page
|
|
.getByTestId("message-timeline")
|
|
.getByTestId("message-row")
|
|
.first();
|
|
await rootMessage.hover();
|
|
await rootMessage.getByRole("button", { name: "Reply" }).click();
|
|
|
|
const threadPanel = page.getByTestId("message-thread-panel");
|
|
await expect(threadPanel).toBeVisible();
|
|
await expect(page).toHaveURL(/thread=/);
|
|
|
|
await page.getByTestId("channel-random").click();
|
|
await expect(page.getByTestId("chat-title")).toHaveText("random");
|
|
await expect(threadPanel).not.toBeVisible();
|
|
|
|
await page.getByTestId("global-back").click();
|
|
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
|
await expect(threadPanel).toBeVisible();
|
|
|
|
await page.getByTestId("global-forward").click();
|
|
await expect(page.getByTestId("chat-title")).toHaveText("random");
|
|
await expect(threadPanel).not.toBeVisible();
|
|
});
|
|
|
|
test("back undoes closing a thread panel", async ({ page }) => {
|
|
await page.goto("/");
|
|
|
|
await page.getByTestId("channel-general").click();
|
|
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
|
|
|
const rootMessage = page
|
|
.getByTestId("message-timeline")
|
|
.getByTestId("message-row")
|
|
.first();
|
|
await rootMessage.hover();
|
|
await rootMessage.getByRole("button", { name: "Reply" }).click();
|
|
|
|
const threadPanel = page.getByTestId("message-thread-panel");
|
|
await expect(threadPanel).toBeVisible();
|
|
|
|
await threadPanel.getByRole("button", { name: "Close panel" }).click();
|
|
await expect(threadPanel).not.toBeVisible();
|
|
|
|
await page.getByTestId("global-back").click();
|
|
await expect(threadPanel).toBeVisible();
|
|
});
|
|
|
|
test("open thread panels survive reload", async ({ page }) => {
|
|
await page.goto("/");
|
|
|
|
await page.getByTestId("channel-general").click();
|
|
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
|
|
|
const rootMessage = page
|
|
.getByTestId("message-timeline")
|
|
.getByTestId("message-row")
|
|
.first();
|
|
await rootMessage.hover();
|
|
await rootMessage.getByRole("button", { name: "Reply" }).click();
|
|
|
|
const threadPanel = page.getByTestId("message-thread-panel");
|
|
await expect(threadPanel).toBeVisible();
|
|
await expect(page).toHaveURL(/thread=/);
|
|
|
|
await page.reload();
|
|
|
|
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
|
await expect(threadPanel).toBeVisible();
|
|
});
|
|
|
|
test("home inbox selection survives reload and back restores it", async ({
|
|
page,
|
|
}) => {
|
|
await page.goto("/");
|
|
|
|
const inboxList = page.getByTestId("home-inbox-list");
|
|
await expect(inboxList).toBeVisible();
|
|
const items = inboxList.locator('[data-testid^="home-inbox-item-"]');
|
|
await expect(items.first()).toBeVisible();
|
|
|
|
// The wide-viewport default selection stays local-only — the URL records
|
|
// explicit selections, so background loads never touch the history stack.
|
|
await expect(page.getByTestId("home-inbox-detail")).toBeVisible();
|
|
expect(page.url()).not.toContain("item=");
|
|
const defaultUrl = page.url();
|
|
|
|
const selectedItem = items.first();
|
|
const selectedTestId = await selectedItem.getAttribute("data-testid");
|
|
const selectedItemId = selectedTestId?.replace("home-inbox-item-", "");
|
|
expect(selectedItemId).toBeTruthy();
|
|
await selectedItem.click();
|
|
await expect
|
|
.poll(() => page.url())
|
|
.toContain(`item=${encodeURIComponent(selectedItemId ?? "")}`);
|
|
|
|
await page.reload();
|
|
|
|
await expect(inboxList).toBeVisible();
|
|
await expect(page.getByTestId("home-inbox-detail")).toBeVisible();
|
|
expect(page.url()).toContain(
|
|
`item=${encodeURIComponent(selectedItemId ?? "")}`,
|
|
);
|
|
|
|
await page.getByTestId("global-back").click();
|
|
await expect.poll(() => page.url()).toBe(defaultUrl);
|
|
});
|
|
|
|
test("settings is a route: section survives reload, closing returns to the previous panel state", async ({
|
|
page,
|
|
}) => {
|
|
await page.goto("/");
|
|
|
|
// Open a channel with a thread panel so there's panel state to come back to.
|
|
await page.getByTestId("channel-general").click();
|
|
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
|
const rootMessage = page
|
|
.getByTestId("message-timeline")
|
|
.getByTestId("message-row")
|
|
.first();
|
|
await rootMessage.hover();
|
|
await rootMessage.getByRole("button", { name: "Reply" }).click();
|
|
const threadPanel = page.getByTestId("message-thread-panel");
|
|
await expect(threadPanel).toBeVisible();
|
|
const channelUrl = page.url();
|
|
|
|
await openSettings(page);
|
|
await expect(page).toHaveURL(/#\/settings/);
|
|
|
|
// Section switches rewrite the settings entry (replace, not push).
|
|
await page.getByTestId("settings-nav-notifications").click();
|
|
await expect(page).toHaveURL(/section=notifications/);
|
|
|
|
await page.reload();
|
|
await expect(page.getByTestId("settings-view")).toBeVisible();
|
|
await expect(page).toHaveURL(/section=notifications/);
|
|
|
|
await page.getByTestId("settings-back-to-app").click();
|
|
await expect.poll(() => page.url()).toBe(channelUrl);
|
|
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
|
await expect(threadPanel).toBeVisible();
|
|
});
|
|
|
|
test("settings shortcut returns without opening search dialog", async ({
|
|
page,
|
|
}) => {
|
|
await page.goto("/");
|
|
await page.getByTestId("channel-general").click();
|
|
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
|
const channelUrl = page.url();
|
|
|
|
// Open search via the ⌘K shortcut so the focus-request counter is non-zero,
|
|
// then close it. The Settings subtree remounts the sidebar + search on close,
|
|
// which must not replay the stale counter and resurrect search.
|
|
await page.evaluate(() => {
|
|
const isMac = /mac|iphone|ipad|ipod/i.test(navigator.platform);
|
|
window.dispatchEvent(
|
|
new KeyboardEvent("keydown", {
|
|
bubbles: true,
|
|
cancelable: true,
|
|
code: "KeyK",
|
|
ctrlKey: !isMac,
|
|
key: "k",
|
|
metaKey: isMac,
|
|
}),
|
|
);
|
|
});
|
|
await expect(page.getByTestId("search-dialog-input")).toBeFocused();
|
|
await page.keyboard.press("Escape");
|
|
await expect(page.getByTestId("search-results")).not.toBeVisible();
|
|
|
|
await page.keyboard.press(
|
|
process.platform === "darwin" ? "Meta+Comma" : "Control+Comma",
|
|
);
|
|
|
|
await expect(page).toHaveURL(/#\/settings/);
|
|
await page.getByTestId("settings-back-to-app").click();
|
|
|
|
await expect.poll(() => page.url()).toBe(channelUrl);
|
|
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
|
await expect(page.getByTestId("search-results")).not.toBeVisible();
|
|
});
|
|
|
|
test("message links to visible root messages open the thread panel", async ({
|
|
page,
|
|
}) => {
|
|
await page.goto("/");
|
|
await page.getByTestId("channel-general").click();
|
|
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
|
await expect(page.getByTestId("message-timeline")).toContainText(
|
|
"Welcome to #general",
|
|
);
|
|
|
|
const link =
|
|
"buzz://message?channel=9a1657ac-f7aa-5db0-b632-d8bbeb6dfb50&id=mock-general-welcome";
|
|
await page.getByTestId("message-input").fill(`Root link repro ${link}`);
|
|
await page.getByTestId("send-message").click();
|
|
|
|
const linkMessage = page
|
|
.getByTestId("message-row")
|
|
.filter({ hasText: "Root link repro" })
|
|
.last();
|
|
await expect(linkMessage).toBeVisible();
|
|
await linkMessage
|
|
.getByRole("button", { name: "Open message in general" })
|
|
.click();
|
|
|
|
const threadPanel = page.getByTestId("message-thread-panel");
|
|
await expect(threadPanel).toBeVisible();
|
|
await expect(page).toHaveURL(/thread=mock-general-welcome/);
|
|
await expect(threadPanel.getByTestId("message-thread-head")).toContainText(
|
|
"Welcome to #general",
|
|
);
|
|
});
|
|
|
|
test("message links reopen a closed thread when the same messageId is already in the URL", async ({
|
|
page,
|
|
}) => {
|
|
await page.goto(
|
|
"/#/channels/9a1657ac-f7aa-5db0-b632-d8bbeb6dfb50?messageId=mock-general-welcome",
|
|
);
|
|
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
|
|
|
const threadPanel = page.getByTestId("message-thread-panel");
|
|
await expect(threadPanel).toBeVisible();
|
|
await expect(threadPanel.getByTestId("message-thread-head")).toContainText(
|
|
"Welcome to #general",
|
|
);
|
|
|
|
await threadPanel.getByRole("button", { name: "Close panel" }).click();
|
|
await expect(threadPanel).not.toBeVisible();
|
|
|
|
const link =
|
|
"buzz://message?channel=9a1657ac-f7aa-5db0-b632-d8bbeb6dfb50&id=mock-general-welcome";
|
|
await page
|
|
.getByTestId("message-input")
|
|
.fill(`Reopen same root link repro ${link}`);
|
|
await page.getByTestId("send-message").click();
|
|
|
|
const linkMessage = page
|
|
.getByTestId("message-row")
|
|
.filter({ hasText: "Reopen same root link repro" })
|
|
.last();
|
|
await expect(linkMessage).toBeVisible();
|
|
await linkMessage
|
|
.getByRole("button", { name: "Open message in general" })
|
|
.click();
|
|
|
|
await expect(threadPanel).toBeVisible();
|
|
await expect(threadPanel.getByTestId("message-thread-head")).toContainText(
|
|
"Welcome to #general",
|
|
);
|
|
});
|
|
|
|
test("message deep links survive reload", async ({ page }) => {
|
|
await page.goto(
|
|
`/#/channels/${ENGINEERING_CHANNEL_ID}?messageId=mock-engineering-shipped`,
|
|
);
|
|
|
|
await expect(page.getByTestId("chat-title")).toHaveText("engineering");
|
|
await expect(page.getByTestId("message-timeline")).toContainText(
|
|
"Engineering shipped the desktop build.",
|
|
);
|
|
|
|
await page.reload();
|
|
|
|
await expect(page.getByTestId("chat-title")).toHaveText("engineering");
|
|
await expect(page.getByTestId("message-timeline")).toContainText(
|
|
"Engineering shipped the desktop build.",
|
|
);
|
|
});
|