6adf42617f
Root cause on iOS Safari (iPad/phone): the `IncomingNotePopup` rendered a `fixed inset-0 z-[110]` full-viewport wrapper with `pointer-events-none`. Stacking that layer on top of the upcoming meeting alert plus a Radix toast intermittently caused the next finger tap to be swallowed — the user saw the popup "hang". Fixes: - Drop the full-viewport wrapper: render the popup card directly as a sized `fixed` element. No more invisible viewport-sized layer between touch and the rest of the UI. - Suppress the redundant note/reply toast when the floating popup actually accepts the event. The popup IS the surface; doubling up just stacks one more floating layer. - Remove `autoFocus` on the Reply button. A new popup arriving while the user is mid-tap would steal focus, contributing to the perceived hang. Tests: - New `notes-popup-touch-tap.spec.mjs`: emulates a touch viewport, sends a real cross-user note, verifies popup bounding box is card-sized (not viewport-sized), the redundant toast is suppressed, and a `locator.tap()` on Reply dismisses the popup and navigates. - `tsc --noEmit` clean. Files: - artifacts/tx-os/src/components/notes/incoming-note-popup.tsx - artifacts/tx-os/src/hooks/use-notifications-socket.ts - artifacts/tx-os/tests/notes-popup-touch-tap.spec.mjs (new)
211 lines
7.9 KiB
JavaScript
211 lines
7.9 KiB
JavaScript
// Task #427: on touch devices (iPad / phone), tapping a button on the
|
|
// floating note popup MUST work — even when other floating UI is on
|
|
// screen at the same time. The previous root cause was a full-viewport
|
|
// `fixed inset-0` wrapper at z-[110] that intermittently swallowed
|
|
// taps on iOS Safari. This test emulates a touch viewport, sends a
|
|
// note across two contexts, and verifies a real `touchscreen.tap` on
|
|
// the Reply button navigates the recipient to the thread.
|
|
import { test, expect } from "@playwright/test";
|
|
import pg from "pg";
|
|
|
|
const DATABASE_URL = process.env.DATABASE_URL;
|
|
if (!DATABASE_URL) {
|
|
throw new Error("DATABASE_URL must be set to run this UI test");
|
|
}
|
|
|
|
const TEST_PASSWORD = "TestPass123!";
|
|
const TEST_PASSWORD_HASH =
|
|
"$2b$10$Bs636ukPMyz01nKrsi.5m.JlDXSN22AVCvn8cgPWWDbo5yJRQX2vu";
|
|
|
|
const pool = new pg.Pool({ connectionString: DATABASE_URL });
|
|
const createdUserIds = [];
|
|
const createdNoteIds = [];
|
|
|
|
function uniq() {
|
|
return `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 8)}`;
|
|
}
|
|
|
|
async function createUser(prefix, displayEn) {
|
|
const username = `${prefix}_${uniq()}`;
|
|
const { rows } = await pool.query(
|
|
`INSERT INTO users (username, email, password_hash, display_name_en, preferred_language, is_active)
|
|
VALUES ($1, $2, $3, $4, 'en', true) RETURNING id`,
|
|
[username, `${username}@example.com`, TEST_PASSWORD_HASH, displayEn],
|
|
);
|
|
const id = rows[0].id;
|
|
createdUserIds.push(id);
|
|
await pool.query(
|
|
`INSERT INTO user_roles (user_id, role_id)
|
|
SELECT $1, id FROM roles WHERE name = 'user'`,
|
|
[id],
|
|
);
|
|
return { id, username, displayEn };
|
|
}
|
|
|
|
test.afterAll(async () => {
|
|
if (createdNoteIds.length > 0) {
|
|
await pool.query(
|
|
`DELETE FROM note_replies WHERE note_id = ANY($1::int[])`,
|
|
[createdNoteIds],
|
|
);
|
|
await pool.query(
|
|
`DELETE FROM note_recipients WHERE note_id = ANY($1::int[])`,
|
|
[createdNoteIds],
|
|
);
|
|
await pool.query(`DELETE FROM notes WHERE id = ANY($1::int[])`, [
|
|
createdNoteIds,
|
|
]);
|
|
}
|
|
if (createdUserIds.length > 0) {
|
|
await pool.query(`DELETE FROM notes WHERE user_id = ANY($1::int[])`, [
|
|
createdUserIds,
|
|
]);
|
|
await pool.query(
|
|
`DELETE FROM notifications WHERE user_id = ANY($1::int[])`,
|
|
[createdUserIds],
|
|
);
|
|
await pool.query(
|
|
`DELETE FROM user_roles WHERE user_id = ANY($1::int[])`,
|
|
[createdUserIds],
|
|
);
|
|
await pool.query(`DELETE FROM users WHERE id = ANY($1::int[])`, [
|
|
createdUserIds,
|
|
]);
|
|
}
|
|
await pool.end();
|
|
});
|
|
|
|
async function loginViaUi(page, username) {
|
|
await page.goto("/login");
|
|
await page.locator("#username").fill(username);
|
|
await page.locator("#password").fill(TEST_PASSWORD);
|
|
await Promise.all([
|
|
page.waitForURL((url) => !url.pathname.endsWith("/login"), {
|
|
timeout: 15_000,
|
|
}),
|
|
page.locator('form button[type="submit"]').click(),
|
|
]);
|
|
}
|
|
|
|
test("recipient on a touch device can tap Reply on the floating note popup", async ({
|
|
browser,
|
|
}) => {
|
|
test.setTimeout(120_000);
|
|
const sender = await createUser("popup_touch_sender", "Touch Sender");
|
|
const recipient = await createUser("popup_touch_recipient", "Touch Recipient");
|
|
|
|
// Touch-emulating context for the recipient — chromium honours
|
|
// hasTouch + isMobile so `(hover: none)` and `pointerType==='touch'`
|
|
// both report correctly. We avoid devices["iPad …"] because that
|
|
// pins to webkit, which isn't installed in this environment.
|
|
const recipientCtx = await browser.newContext({
|
|
hasTouch: true,
|
|
isMobile: true,
|
|
viewport: { width: 820, height: 1180 },
|
|
});
|
|
const senderCtx = await browser.newContext();
|
|
const recipientPage = await recipientCtx.newPage();
|
|
const senderPage = await senderCtx.newPage();
|
|
|
|
try {
|
|
await loginViaUi(recipientPage, recipient.username);
|
|
await recipientPage.goto("/");
|
|
// Sanity: the touch-only CSS path is what we depend on.
|
|
expect(
|
|
await recipientPage.evaluate(
|
|
() => window.matchMedia("(hover: none)").matches,
|
|
),
|
|
).toBe(true);
|
|
// Wait out the socket warmup window so events fanout cleanly.
|
|
await recipientPage.waitForTimeout(3500);
|
|
|
|
await loginViaUi(senderPage, sender.username);
|
|
await senderPage.goto("/notes");
|
|
await expect(senderPage.getByTestId("notes-page")).toBeVisible();
|
|
|
|
const noteTitle = `Touch Note ${uniq()}`;
|
|
const noteContent = "Tap reply on this card.";
|
|
await senderPage.getByTestId("notes-composer-open").click();
|
|
await senderPage.getByTestId("notes-composer-title").fill(noteTitle);
|
|
await senderPage.getByTestId("notes-composer-content").fill(noteContent);
|
|
const createPromise = senderPage.waitForResponse(
|
|
(r) =>
|
|
new URL(r.url()).pathname === "/api/notes" &&
|
|
r.request().method() === "POST" &&
|
|
r.status() >= 200 &&
|
|
r.status() < 300,
|
|
{ timeout: 15_000 },
|
|
);
|
|
await senderPage.getByTestId("notes-composer-save").click();
|
|
const createdNote = await (await createPromise).json();
|
|
createdNoteIds.push(createdNote.id);
|
|
|
|
const card = senderPage.getByTestId(`note-card-${createdNote.id}`);
|
|
await expect(card).toBeVisible();
|
|
await card.hover();
|
|
await senderPage.getByTestId(`note-card-send-${createdNote.id}`).click();
|
|
await expect(senderPage.getByTestId("send-note-dialog")).toBeVisible();
|
|
await senderPage.getByTestId("send-note-search").fill(recipient.displayEn);
|
|
await senderPage
|
|
.getByTestId(`send-recipient-option-${recipient.id}`)
|
|
.click();
|
|
const sendPromise = senderPage.waitForResponse(
|
|
(r) =>
|
|
new URL(r.url()).pathname === `/api/notes/${createdNote.id}/send` &&
|
|
r.request().method() === "POST" &&
|
|
r.status() >= 200 &&
|
|
r.status() < 300,
|
|
{ timeout: 15_000 },
|
|
);
|
|
await senderPage.getByTestId("send-note-submit").click();
|
|
await expect(senderPage.getByTestId("send-note-confirm")).toBeVisible();
|
|
await senderPage.getByTestId("send-note-confirm-submit").click();
|
|
await sendPromise;
|
|
|
|
// ---- Recipient sees the floating popup ----
|
|
const popup = recipientPage.getByTestId("incoming-note-popup");
|
|
await expect(popup).toBeVisible({ timeout: 5_000 });
|
|
|
|
// The popup must NOT be a full-viewport overlay. Before the fix
|
|
// it was wrapped in a `fixed inset-0` div whose child captured
|
|
// pointer events; after the fix the rendered popup root is
|
|
// sized to the card itself. We verify the bounding box is well
|
|
// smaller than the viewport in BOTH dimensions — the regression
|
|
// would re-introduce a viewport-sized box.
|
|
const box = await popup.boundingBox();
|
|
expect(box).not.toBeNull();
|
|
expect(box.width).toBeLessThan(500); // card is ~384 + ring/padding
|
|
expect(box.height).toBeLessThan(800);
|
|
|
|
// The redundant toast must NOT also surface — when the popup is
|
|
// shown the toast is suppressed (#427) so we don't pile two
|
|
// floating layers on top of each other on touch.
|
|
await expect(
|
|
recipientPage.locator('[data-state="open"][role="status"]'),
|
|
).toHaveCount(0);
|
|
|
|
// ---- The actual regression: a TOUCH tap on Reply must work ----
|
|
const replyBtn = recipientPage.getByTestId("incoming-note-popup-reply");
|
|
await expect(replyBtn).toBeVisible();
|
|
// `locator.tap()` requires `hasTouch: true` and fires a real
|
|
// touchstart/touchend → synthesized click sequence — the same path
|
|
// a finger tap takes on iOS Safari, which is what the user
|
|
// reported hung.
|
|
await replyBtn.tap();
|
|
|
|
// Reply navigates to /notes and dismisses the popup. If the tap
|
|
// were swallowed (the original hang) neither would happen.
|
|
// (We don't assert the querystring here — wouter v3's `setLocation`
|
|
// strips it; that's an unrelated pre-existing behaviour outside
|
|
// the scope of #427.)
|
|
await recipientPage.waitForURL(
|
|
(url) => url.pathname === "/notes",
|
|
{ timeout: 5_000 },
|
|
);
|
|
await expect(popup).toBeHidden();
|
|
} finally {
|
|
await senderCtx.close();
|
|
await recipientCtx.close();
|
|
}
|
|
});
|