diff --git a/artifacts/tx-os/public/opengraph.jpg b/artifacts/tx-os/public/opengraph.jpg index c61a8cde..eb304be9 100644 Binary files a/artifacts/tx-os/public/opengraph.jpg and b/artifacts/tx-os/public/opengraph.jpg differ diff --git a/artifacts/tx-os/src/components/notes/incoming-note-popup.tsx b/artifacts/tx-os/src/components/notes/incoming-note-popup.tsx index 04066c9c..f9b22f55 100644 --- a/artifacts/tx-os/src/components/notes/incoming-note-popup.tsx +++ b/artifacts/tx-os/src/components/notes/incoming-note-popup.tsx @@ -290,32 +290,32 @@ export function IncomingNotePopup() { : current.content; return ( - // Full-viewport invisible layer. `pointer-events-none` so clicks - // fall through to the app — the popup is "floating", not modal. + // The popup is "floating", not modal: render the card directly as + // a fixed-positioned element. We deliberately AVOID a full-viewport + // wrapper — on iOS Safari a high-z `fixed inset-0` layer (even with + // `pointer-events-none`) interacts badly with stacked floating + // panels and intermittently swallows the next tap on whatever sits + // underneath. Sizing the wrapper to just the card fixes that.
-
@@ -444,6 +443,5 @@ export function IncomingNotePopup() {
- ); } diff --git a/artifacts/tx-os/src/hooks/use-notifications-socket.ts b/artifacts/tx-os/src/hooks/use-notifications-socket.ts index 69e7ec21..daa6d3d0 100644 --- a/artifacts/tx-os/src/hooks/use-notifications-socket.ts +++ b/artifacts/tx-os/src/hooks/use-notifications-socket.ts @@ -168,10 +168,18 @@ export function useNotificationsSocket() { vibrate: user.vibrationEnabledNote, }); } - toast({ - title: i18n.t("notes.toast.received.title"), - description: i18n.t("notes.toast.received.desc"), - }); + // Only fall back to a toast when the floating popup did NOT + // accept this event (sender's own note, duplicate, malformed, + // etc.). When the popup IS shown, the toast is redundant — and + // on touch devices stacking a toast over an already-stacked + // popup + meeting alert produced extra layers that intermittently + // intercepted the next tap (#427). + if (!enqueued) { + toast({ + title: i18n.t("notes.toast.received.title"), + description: i18n.t("notes.toast.received.desc"), + }); + } }); socket.on( @@ -245,10 +253,14 @@ export function useNotificationsSocket() { vibrate: user.vibrationEnabledNote, }); } - toast({ - title: i18n.t("notes.toast.replied.title"), - description: i18n.t("notes.toast.replied.desc"), - }); + // See `note_received` above: skip the redundant toast when the + // floating popup is the surface (#427). + if (!enqueued) { + toast({ + title: i18n.t("notes.toast.replied.title"), + description: i18n.t("notes.toast.replied.desc"), + }); + } }, ); diff --git a/artifacts/tx-os/tests/notes-popup-touch-tap.spec.mjs b/artifacts/tx-os/tests/notes-popup-touch-tap.spec.mjs new file mode 100644 index 00000000..1b55a68e --- /dev/null +++ b/artifacts/tx-os/tests/notes-popup-touch-tap.spec.mjs @@ -0,0 +1,210 @@ +// 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(); + } +});