Task #427: fix floating note popup hanging on touch devices
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)
This commit is contained in:
riyadhafraa
Binary file not shown.
|
Before Width: | Height: | Size: 25 KiB After Width: | Height: | Size: 25 KiB |
@@ -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.
|
||||
<div
|
||||
className="fixed inset-0 z-[110] pointer-events-none"
|
||||
aria-hidden="false"
|
||||
ref={cardRef}
|
||||
key={replyPayload ? `reply-${replyPayload.replyId}` : `note-${current.noteId}`}
|
||||
role="dialog"
|
||||
aria-label={heading}
|
||||
data-testid="incoming-note-popup"
|
||||
data-popup-kind={replyPayload ? "reply" : "note"}
|
||||
dir={isRtl ? "rtl" : "ltr"}
|
||||
className="fixed top-0 left-0 z-[110]"
|
||||
style={{
|
||||
width: CARD_WIDTH,
|
||||
maxWidth: "calc(100vw - 16px)",
|
||||
transform: `translate3d(${position.x}px, ${position.y}px, 0)`,
|
||||
transition: isDragging
|
||||
? "none"
|
||||
: "transform 220ms cubic-bezier(0.34, 1.56, 0.64, 1), opacity 200ms ease-out",
|
||||
opacity: enterAnim ? 1 : 0,
|
||||
willChange: "transform",
|
||||
}}
|
||||
>
|
||||
<div
|
||||
ref={cardRef}
|
||||
key={replyPayload ? `reply-${replyPayload.replyId}` : `note-${current.noteId}`}
|
||||
role="dialog"
|
||||
aria-label={heading}
|
||||
data-testid="incoming-note-popup"
|
||||
data-popup-kind={replyPayload ? "reply" : "note"}
|
||||
dir={isRtl ? "rtl" : "ltr"}
|
||||
className="pointer-events-auto absolute top-0 left-0"
|
||||
style={{
|
||||
width: CARD_WIDTH,
|
||||
maxWidth: "calc(100vw - 16px)",
|
||||
transform: `translate3d(${position.x}px, ${position.y}px, 0)`,
|
||||
transition: isDragging
|
||||
? "none"
|
||||
: "transform 220ms cubic-bezier(0.34, 1.56, 0.64, 1), opacity 200ms ease-out",
|
||||
opacity: enterAnim ? 1 : 0,
|
||||
willChange: "transform",
|
||||
}}
|
||||
>
|
||||
<div
|
||||
className={`rounded-2xl bg-background ring-4 ring-amber-400/70 shadow-2xl shadow-amber-500/30 overflow-hidden transition-transform duration-200 ${
|
||||
enterAnim ? "scale-100" : "scale-95"
|
||||
@@ -433,7 +433,6 @@ export function IncomingNotePopup() {
|
||||
<Button
|
||||
type="button"
|
||||
size="sm"
|
||||
autoFocus
|
||||
onClick={handleReply}
|
||||
data-testid="incoming-note-popup-reply"
|
||||
>
|
||||
@@ -444,6 +443,5 @@ export function IncomingNotePopup() {
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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"),
|
||||
});
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
});
|
||||
Reference in New Issue
Block a user