From fc14ca4c019aa7c4d0d8767b8e29957b742251c7 Mon Sep 17 00:00:00 2001 From: riyadhafraa <49757212-riyadhafraa@users.noreply.replit.com> Date: Wed, 6 May 2026 11:54:13 +0000 Subject: [PATCH] Task #427: fix floating note popup hanging on touch devices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause on iOS Safari (iPad/phone): the `IncomingNotePopup` rendered a `fixed inset-0 z-[110]` full-viewport wrapper. 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". A simultaneous re-render storm from synchronous query invalidations on multi-event socket fanouts made it worse on slower hardware. 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 — focus-steal was contributing to the perceived hang. - Coalesce `react-query` `invalidateQueries` calls in the notifications socket: collect into a Set and flush once per animation frame so a burst of socket events triggers a single refetch pass instead of N synchronous re-renders. Cleaned up on socket disconnect. Tests (artifacts/tx-os/tests/notes-popup-touch-tap.spec.mjs): - "recipient on a touch device can tap Reply on the floating note popup" — verifies the popup is no longer viewport-sized, the duplicate toast is suppressed, and a real `locator.tap()` succeeds. - "popup + meeting alert simultaneously: tap on popup still works" — the user's exact scenario: seeds an imminent meeting so the alert appears, triggers a real cross-user note delivery so both float simultaneously, then taps Reply on the popup and confirms it dismisses while the alert remains. Both tests + 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) --- .../src/hooks/use-notifications-socket.ts | 117 ++++++------ .../tests/notes-popup-touch-tap.spec.mjs | 175 ++++++++++++++++++ 2 files changed, 240 insertions(+), 52 deletions(-) diff --git a/artifacts/tx-os/src/hooks/use-notifications-socket.ts b/artifacts/tx-os/src/hooks/use-notifications-socket.ts index daa6d3d0..e1fcd451 100644 --- a/artifacts/tx-os/src/hooks/use-notifications-socket.ts +++ b/artifacts/tx-os/src/hooks/use-notifications-socket.ts @@ -56,6 +56,38 @@ export function useNotificationsSocket() { // load don't all play sounds back-to-back. connectedAtRef.current = nowMs(); + // ---- Coalesced invalidations (#427) ---- + // When several socket events arrive in the same JS task (e.g. a + // `note_received` AND a `notification_created` AND a meeting + // alert all fire from one server fanout), invalidating each + // query key synchronously triggers a re-render storm that + // briefly stalls the UI on slower iPad/phone hardware — which + // contributed to the perceived "tap hang". We collect query + // keys in a Set keyed by JSON stringification (cheap dedupe) + // and flush them once per animation frame so React Query batches + // the refetches into one render pass. + const pendingKeys = new Map(); + let rafHandle: number | null = null; + const flushPending = () => { + rafHandle = null; + const keys = Array.from(pendingKeys.values()); + pendingKeys.clear(); + for (const key of keys) { + queryClient.invalidateQueries({ queryKey: key as unknown[] }); + } + }; + const invalidate = (key: readonly unknown[]) => { + pendingKeys.set(JSON.stringify(key), key); + if (rafHandle !== null) return; + if (typeof window !== "undefined" && typeof window.requestAnimationFrame === "function") { + rafHandle = window.requestAnimationFrame(flushPending); + } else { + // SSR / test fallback: macro-task defer is still enough to + // coalesce events arriving in the same tick. + rafHandle = window.setTimeout(flushPending, 0); + } + }; + const socket = io(window.location.origin, { path: `${BASE}/api/socket.io`, }); @@ -65,15 +97,11 @@ export function useNotificationsSocket() { }); socket.on("notification_created", (payload?: { type?: string }) => { - queryClient.invalidateQueries({ queryKey: getListNotificationsQueryKey() }); - queryClient.invalidateQueries({ queryKey: getGetHomeStatsQueryKey() }); + invalidate(getListNotificationsQueryKey()); + invalidate(getGetHomeStatsQueryKey()); if (payload?.type === "order") { - queryClient.invalidateQueries({ - queryKey: getListMyServiceOrdersQueryKey(), - }); - queryClient.invalidateQueries({ - queryKey: getListIncomingServiceOrdersQueryKey(), - }); + invalidate(getListMyServiceOrdersQueryKey(),); + invalidate(getListIncomingServiceOrdersQueryKey(),); } // Per-user sound + vibration. Plays whether the tab is in the @@ -100,34 +128,24 @@ export function useNotificationsSocket() { }); socket.on("order_updated", () => { - queryClient.invalidateQueries({ - queryKey: getListMyServiceOrdersQueryKey(), - }); - queryClient.invalidateQueries({ - queryKey: getListIncomingServiceOrdersQueryKey(), - }); + invalidate(getListMyServiceOrdersQueryKey(),); + invalidate(getListIncomingServiceOrdersQueryKey(),); }); socket.on("order_deleted", () => { - queryClient.invalidateQueries({ - queryKey: getListMyServiceOrdersQueryKey(), - }); - queryClient.invalidateQueries({ - queryKey: getListIncomingServiceOrdersQueryKey(), - }); + invalidate(getListMyServiceOrdersQueryKey(),); + invalidate(getListIncomingServiceOrdersQueryKey(),); }); socket.on("order_incoming_changed", () => { - queryClient.invalidateQueries({ - queryKey: getListIncomingServiceOrdersQueryKey(), - }); + invalidate(getListIncomingServiceOrdersQueryKey(),); }); // Realtime note events: invalidate notes queries AND surface a toast so // the recipient (or sender, on a reply) sees an immediate, dismissible // confirmation regardless of which page they're on. socket.on("note_received", (payload?: Partial) => { - queryClient.invalidateQueries({ queryKey: ["notes"] }); + invalidate(["notes"]); const inWarmup = nowMs() - connectedAtRef.current < SOCKET_WARMUP_MS; let enqueued = false; if ( @@ -185,7 +203,7 @@ export function useNotificationsSocket() { socket.on( "note_replied", (payload?: Partial & { recipientUserId?: number }) => { - queryClient.invalidateQueries({ queryKey: ["notes"] }); + invalidate(["notes"]); const inWarmup = nowMs() - connectedAtRef.current < SOCKET_WARMUP_MS; let enqueued = false; // The server emits `note_replied` only to the *original note @@ -265,33 +283,27 @@ export function useNotificationsSocket() { ); socket.on("note_status_changed", () => { - queryClient.invalidateQueries({ queryKey: ["notes"] }); + invalidate(["notes"]); }); socket.on("apps_changed", () => { - queryClient.invalidateQueries({ queryKey: getListAppsQueryKey() }); - queryClient.invalidateQueries({ queryKey: getGetMeQueryKey() }); + invalidate(getListAppsQueryKey()); + invalidate(getGetMeQueryKey()); }); socket.on( "executive_meetings_changed", (payload?: { date?: string }) => { if (typeof payload?.date === "string" && payload.date.length > 0) { - queryClient.invalidateQueries({ - queryKey: ["/api/executive-meetings", payload.date], - }); + invalidate(["/api/executive-meetings", payload.date],); } else { - queryClient.invalidateQueries({ - queryKey: ["/api/executive-meetings"], - }); + invalidate(["/api/executive-meetings"],); } }, ); socket.on("executive_meeting_notifications_changed", () => { - queryClient.invalidateQueries({ - queryKey: ["/api/executive-meetings/notifications"], - }); + invalidate(["/api/executive-meetings/notifications"],); }); // #277: Per-user push for the 5-min upcoming-meeting alert. The server @@ -303,34 +315,35 @@ export function useNotificationsSocket() { socket.on( "executive_meeting_alert_state_changed", () => { - queryClient.invalidateQueries({ - queryKey: ["/api/executive-meetings/alert-state"], - }); + invalidate(["/api/executive-meetings/alert-state"],); }, ); socket.on( "role_permissions_changed", (payload?: { roleId?: number }) => { - queryClient.invalidateQueries({ queryKey: getGetMeQueryKey() }); - queryClient.invalidateQueries({ queryKey: getListPermissionsQueryKey() }); - queryClient.invalidateQueries({ queryKey: getListRolesQueryKey() }); + invalidate(getGetMeQueryKey()); + invalidate(getListPermissionsQueryKey()); + invalidate(getListRolesQueryKey()); const roleId = payload?.roleId; if (typeof roleId === "number") { - queryClient.invalidateQueries({ - queryKey: getGetRolePermissionsQueryKey(roleId), - }); - queryClient.invalidateQueries({ - queryKey: getGetRolePermissionAuditQueryKey(roleId), - }); - queryClient.invalidateQueries({ - queryKey: getGetRoleUsageQueryKey(roleId), - }); + invalidate(getGetRolePermissionsQueryKey(roleId),); + invalidate(getGetRolePermissionAuditQueryKey(roleId),); + invalidate(getGetRoleUsageQueryKey(roleId),); } }, ); return () => { + if (rafHandle !== null) { + if (typeof window !== "undefined" && typeof window.cancelAnimationFrame === "function") { + window.cancelAnimationFrame(rafHandle); + } else { + window.clearTimeout(rafHandle); + } + rafHandle = null; + } + pendingKeys.clear(); socket.disconnect(); }; }, [user, queryClient]); diff --git a/artifacts/tx-os/tests/notes-popup-touch-tap.spec.mjs b/artifacts/tx-os/tests/notes-popup-touch-tap.spec.mjs index 1b55a68e..030ec4ff 100644 --- a/artifacts/tx-os/tests/notes-popup-touch-tap.spec.mjs +++ b/artifacts/tx-os/tests/notes-popup-touch-tap.spec.mjs @@ -20,11 +20,41 @@ const TEST_PASSWORD_HASH = const pool = new pg.Pool({ connectionString: DATABASE_URL }); const createdUserIds = []; const createdNoteIds = []; +const createdMeetingIds = []; function uniq() { return `${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 8)}`; } +function todayLocal() { + const d = new Date(); + return `${d.getFullYear()}-${String(d.getMonth() + 1).padStart(2, "0")}-${String(d.getDate()).padStart(2, "0")}`; +} +function timePlusMinutes(deltaMins) { + const d = new Date(Date.now() + deltaMins * 60_000); + return `${String(d.getHours()).padStart(2, "0")}:${String(d.getMinutes()).padStart(2, "0")}:00`; +} +async function insertImminentMeeting(titleEn, deltaMins = 3) { + const date = todayLocal(); + const startTime = timePlusMinutes(deltaMins); + const endTime = timePlusMinutes(deltaMins + 30); + const { rows: dnRows } = await pool.query( + `SELECT COALESCE(MAX(daily_number), 0) + 1 AS n FROM executive_meetings WHERE meeting_date = $1`, + [date], + ); + const dn = dnRows[0].n; + const { rows } = await pool.query( + `INSERT INTO executive_meetings + (daily_number, title_ar, title_en, meeting_date, start_time, end_time, status) + VALUES ($1, $2, $3, $4, $5, $6, 'scheduled') + RETURNING id`, + [dn, `${titleEn} AR`, titleEn, date, startTime, endTime], + ); + const id = rows[0].id; + createdMeetingIds.push(id); + return id; +} + async function createUser(prefix, displayEn) { const username = `${prefix}_${uniq()}`; const { rows } = await pool.query( @@ -43,6 +73,24 @@ async function createUser(prefix, displayEn) { } test.afterAll(async () => { + if (createdMeetingIds.length > 0) { + await pool.query( + `DELETE FROM executive_meeting_alert_state WHERE meeting_id = ANY($1::int[])`, + [createdMeetingIds], + ); + await pool.query( + `DELETE FROM executive_meeting_audit_logs WHERE entity_type = 'meeting' AND entity_id = ANY($1::int[])`, + [createdMeetingIds], + ); + await pool.query( + `DELETE FROM executive_meeting_attendees WHERE meeting_id = ANY($1::int[])`, + [createdMeetingIds], + ); + await pool.query( + `DELETE FROM executive_meetings WHERE id = ANY($1::int[])`, + [createdMeetingIds], + ); + } if (createdNoteIds.length > 0) { await pool.query( `DELETE FROM note_replies WHERE note_id = ANY($1::int[])`, @@ -208,3 +256,130 @@ test("recipient on a touch device can tap Reply on the floating note popup", asy await recipientCtx.close(); } }); + +// Stacked-layer reproduction (#427): the user reported the hang +// specifically when the floating note popup AND the 5-min upcoming +// meeting alert are on screen at the same time. This test seeds an +// imminent meeting, has admin (the "recipient") log in on a touch +// viewport so the meeting alert appears, then triggers a real +// cross-user note delivery so the floating popup stacks on top of +// the alert. We then tap the popup's Reply button — the same gesture +// the user reported as hung — and assert it works. +test("popup + meeting alert simultaneously: tap on popup still works", async ({ + browser, +}) => { + test.setTimeout(150_000); + const sender = await createUser("popup_stack_sender", "Stack Sender"); + + const meetingTitle = `STACK_TEST ${uniq()}`; + await insertImminentMeeting(meetingTitle, 3); + + // Admin user (seeded by migrations) sees the upcoming meeting alert + // on every page since the alert is global. We use admin as the + // touch recipient so we don't need to wire up permissions for a + // throwaway user. + 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 { + // Recipient logs in via the existing admin account. + await recipientPage.goto("/login"); + await recipientPage.locator("#username").fill("admin"); + await recipientPage.locator("#password").fill("admin123"); + await Promise.all([ + recipientPage.waitForURL( + (url) => !url.pathname.endsWith("/login"), + { timeout: 15_000 }, + ), + recipientPage.locator('form button[type="submit"]').click(), + ]); + await recipientPage.goto("/"); + + // Wait for the meeting alert to surface (poll runs every 30s but + // the initial fetch fires on mount). + const alert = recipientPage.getByTestId("upcoming-meeting-alert"); + await expect(alert).toBeVisible({ timeout: 20_000 }); + + // Look up admin's userId so the sender can target them. + const { rows: adminRows } = await pool.query( + `SELECT id FROM users WHERE username = 'admin' LIMIT 1`, + ); + const adminId = adminRows[0].id; + + // Sender composes + sends a note to admin. + await loginViaUi(senderPage, sender.username); + await senderPage.goto("/notes"); + await expect(senderPage.getByTestId("notes-page")).toBeVisible(); + + const noteTitle = `Stack Note ${uniq()}`; + await senderPage.getByTestId("notes-composer-open").click(); + await senderPage.getByTestId("notes-composer-title").fill(noteTitle); + await senderPage + .getByTestId("notes-composer-content") + .fill("Tap reply with the meeting alert visible too."); + 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("admin"); + await senderPage + .getByTestId(`send-recipient-option-${adminId}`) + .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; + + // ---- Both layers must be visible at the same time ---- + const popup = recipientPage.getByTestId("incoming-note-popup"); + await expect(popup).toBeVisible({ timeout: 10_000 }); + await expect(alert).toBeVisible(); + + // The actual reproduction: tap Reply on the popup while the + // meeting alert is also on screen. Pre-fix this would intermittently + // hang on iOS Safari because the popup's `fixed inset-0` wrapper + // sat above the alert and swallowed the tap. + const replyBtn = recipientPage.getByTestId("incoming-note-popup-reply"); + await expect(replyBtn).toBeVisible(); + await replyBtn.tap(); + + await recipientPage.waitForURL( + (url) => url.pathname === "/notes", + { timeout: 5_000 }, + ); + await expect(popup).toBeHidden(); + // The meeting alert should still be present after dismissing the + // popup — they are independent floating layers. + await expect(alert).toBeVisible(); + } finally { + await senderCtx.close(); + await recipientCtx.close(); + } +});