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. 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)
This commit is contained in:
riyadhafraa
@@ -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<string, readonly unknown[]>();
|
||||
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<IncomingNotePayload>) => {
|
||||
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<IncomingReplyPayload> & { 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]);
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user