Task #499: fix Quick Actions dialog X overlap + iPad long-press drag feedback

Two scoped UX fixes on Executive Meetings schedule, both confined to
artifacts/tx-os/src/pages/executive-meetings.tsx (shared
src/components/ui/dialog.tsx untouched per task spec).

1) Quick Actions dialog header overlap
   - Radix DialogContent renders the Close (X) at physical
     `right-4 top-4` regardless of `dir`. In AR (RTL) the
     `text-right`-aligned title "إجراءات سريعة" pinned to the same
     edge and visibly overlapped the X.
   - Fix: added `pr-8` to the per-row Quick Actions DialogHeader so
     the title's right edge always clears the close button. Works
     for AR (overlap eliminated) and EN (just adds trailing slack).

2) iPad long-press drag feedback
   - dnd-kit's TouchSensor has a 200ms activation delay; previously
     the row gave no cue during the hold window so users lifted
     early thinking the schedule didn't register.
   - Fix: added `isPressing` state toggled via capture-phase
     onPointerDown/Up/Cancel handlers on the row tr, gated on
     `pointerType === "touch"` && `effectiveDragEnabled`. Capture
     phase chosen so handlers don't collide with safeRowDragListeners
     (which spreads dnd-kit's bubble-phase onPointerDown).
   - Visual cue: subtle `inset 0 0 0 2px hexToRgba(highlightColor,
     0.45)` ring composed AFTER quickOpen/selection/current rings so
     it never outranks a real selection state. Defensive useEffect
     clears `isPressing` if drag becomes disabled mid-press.
   - data-pressing="true" attribute exposed on tr for tests.
   - TouchSensor delay (200ms) and tolerance (8px) unchanged.

Tests added:
 - executive-meetings-quick-actions-dialog-layout.spec.mjs: asserts
   close button + title bounding boxes don't overlap in AR + EN.
 - executive-meetings-row-touch-press-feedback.spec.mjs: uses
   hasTouch+isMobile + CDP Input.dispatchTouchEvent, asserts
   data-pressing flips true on touchstart, clears on touchend, and
   tap-to-open quick-actions dialog still works for short taps;
   second test verifies the cue persists past the 200ms window.

Verification: pnpm tsc --noEmit clean. Pre-existing `test` workflow
failures (rotate-content/font-settings/notes-share) are unrelated to
this change.
This commit is contained in:
riyadhafraa
2026-05-11 15:38:17 +00:00
parent 08ebd906a8
commit 27c5cc381c
3 changed files with 471 additions and 2 deletions
@@ -4278,7 +4278,37 @@ function MeetingRow({
const quickOpenShadow = quickActionsCanMutate && quickOpen
? `inset 0 0 0 3px ${highlightColor}`
: null;
const composedShadow = [quickOpenShadow, selectionShadow, currentShadow]
// #499: iPad long-press feedback. dnd-kit's TouchSensor has a 200ms
// activation delay before a long-press becomes a drag — during that
// window the user holds the row but sees nothing happen, so they
// often lift early thinking the schedule didn't register the touch.
// We track an `isPressing` flag set on touch pointerdown (gated on
// `pointerType === "touch"` so mouse/pen pointers aren't affected,
// and on `effectiveDragEnabled` so non-mutators / disabled rows
// don't get a misleading "you can drag this" cue) and cleared on
// pointerup / pointercancel. The cue is rendered as a subtle
// low-alpha inset ring composed AFTER the existing quickOpen /
// selection / current-meeting rings so it never visually outranks
// a real selection or current-meeting state. The ring is 2px (same
// as selection/current) but at 0.45 alpha of `highlightColor`, so
// a real selection ring (solid `#0B1E3F`) and the current ring
// (solid `highlightColor`) still read as the dominant visual.
const [isPressing, setIsPressing] = useState(false);
// Defensive cleanup: if drag becomes disabled mid-press (capability
// change, edit-mode flip), clear the stale press cue so we don't
// paint a "held" ring on a row that can no longer be dragged.
useEffect(() => {
if (!effectiveDragEnabled && isPressing) setIsPressing(false);
}, [effectiveDragEnabled, isPressing]);
const pressShadow = isPressing
? `inset 0 0 0 2px ${hexToRgba(highlightColor, 0.45)}`
: null;
const composedShadow = [
quickOpenShadow,
selectionShadow,
currentShadow,
pressShadow,
]
.filter(Boolean)
.join(", ");
const trStyle: CSSProperties = {
@@ -4352,7 +4382,26 @@ function MeetingRow({
data-current-meeting={isCurrent ? "true" : undefined}
data-bulk-selected={bulkSelected ? "true" : undefined}
data-quick-open={quickActionsCanMutate && quickOpen ? "true" : undefined}
data-pressing={isPressing ? "true" : undefined}
onClick={quickActionsCanMutate ? handleRowClick : undefined}
// #499: capture-phase pointer listeners drive the long-press
// visual cue. We deliberately use the *Capture variants so
// these handlers don't collide with `safeRowDragListeners`
// below, which spreads dnd-kit's bubble-phase `onPointerDown`
// (last spread wins for same-phase props in JSX). Capture
// fires before bubble, so we set/clear `isPressing` first
// and dnd-kit's sensor logic still runs after.
onPointerDownCapture={(e) => {
if (e.pointerType === "touch" && effectiveDragEnabled) {
setIsPressing(true);
}
}}
onPointerUpCapture={() => {
if (isPressing) setIsPressing(false);
}}
onPointerCancelCapture={() => {
if (isPressing) setIsPressing(false);
}}
{...(dragEnabled ? attributes : {})}
{...(safeRowDragListeners ?? {})}
>
@@ -4372,7 +4421,17 @@ function MeetingRow({
data-testid={`em-row-quick-${meeting.id}`}
dir={isRtl ? "rtl" : "ltr"}
>
<DialogHeader>
{/* #499: reserve `pr-8` (~2rem) on the header so the
title text never extends under the Radix close button,
which is positioned at physical `right-4 top-4` inside
the DialogContent. The close stays at physical right
in BOTH AR (RTL) and EN (LTR) because Radix uses CSS
physical positioning, so padding the header on the
right is the correct fix for both languages. In AR the
title `text-right`-aligns to the inside edge of that
padding (visibly clear of the X); in LTR it stays
left-aligned with extra trailing room. */}
<DialogHeader className="pr-8">
<DialogTitle className={isRtl ? "text-right" : "text-left"}>
{t("executiveMeetings.quickActions.label")}
</DialogTitle>
@@ -0,0 +1,191 @@
// #499: e2e for the Quick Actions dialog header layout. The Radix
// DialogContent renders a Close (X) button at physical `right-4 top-4`
// inside the dialog regardless of `dir`. Before #499 the
// DialogHeader had no trailing padding, so in Arabic (RTL) the
// `text-right`-aligned title "إجراءات سريعة" sat directly under the
// X — a visible overlap. The fix adds `pr-8` to the DialogHeader so
// the title text always clears the close button. This spec asserts
// the bounding boxes don't overlap in BOTH languages.
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 the quick-actions dialog layout test",
);
}
const pool = new pg.Pool({ connectionString: DATABASE_URL });
const createdMeetingIds = [];
async function loginViaUi(page, username, password) {
await page.goto("/login");
await page.locator("#username").fill(username);
await page.locator("#password").fill(password);
await Promise.all([
page.waitForURL((url) => !url.pathname.endsWith("/login"), {
timeout: 15_000,
}),
page.locator('form button[type="submit"]').click(),
]);
}
function pickFutureDate(offsetDays) {
const d = new Date(Date.now() + offsetDays * 86_400_000);
const y = d.getFullYear();
const m = String(d.getMonth() + 1).padStart(2, "0");
const day = String(d.getDate()).padStart(2, "0");
return `${y}-${m}-${day}`;
}
async function nextDailyNumberForDate(date) {
const { rows } = await pool.query(
`SELECT COALESCE(MAX(daily_number), 0) + 1 AS n
FROM executive_meetings
WHERE meeting_date = $1`,
[date],
);
return rows[0].n;
}
async function insertMeeting({ date, titleEn, titleAr, startTime, endTime }) {
const dn = await nextDailyNumberForDate(date);
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, titleAr ?? titleEn, titleEn, date, startTime, endTime],
);
const id = rows[0].id;
createdMeetingIds.push(id);
return id;
}
test.afterAll(async () => {
if (createdMeetingIds.length > 0) {
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],
);
}
await pool.end();
});
function rectsOverlap(a, b) {
if (!a || !b) return false;
return (
a.x < b.x + b.width &&
a.x + a.width > b.x &&
a.y < b.y + b.height &&
a.y + a.height > b.y
);
}
async function openDialogAndAssertNoOverlap(page, id) {
const row = page.locator(`[data-testid="em-row-${id}"]`);
await expect(row).toBeVisible({ timeout: 10_000 });
await row.click();
const surface = page.locator(`[data-testid="em-row-quick-${id}"]`);
await expect(surface).toBeVisible();
// The Radix DialogContent renders an unlabeled close button as its
// last child <button> with sr-only label "Close". We target the
// button that contains an svg + sr-only span — there's only one
// button in the header area; the Postpone button has its own
// testid and lives outside the header.
const closeBtn = surface.locator('button[type="button"]').filter({
has: page.locator("svg"),
}).filter({ hasText: /^Close$/ });
await expect(closeBtn).toHaveCount(1);
const titleEl = surface.locator('[id^="radix-"][id$="-title"]').first();
await expect(titleEl).toBeVisible();
const closeBox = await closeBtn.boundingBox();
const titleBox = await titleEl.boundingBox();
expect(closeBox).not.toBeNull();
expect(titleBox).not.toBeNull();
// Hard guard: the title's bounding box must NOT overlap the close
// button's bounding box. Before #499 in AR the rects overlapped on
// the right edge by ~16-24px (X at right-4 = ~16px from edge,
// 16-20px wide; title text-right pinned to the same edge).
expect(
rectsOverlap(closeBox, titleBox),
`Title and Close button bounding boxes overlap. close=${JSON.stringify(closeBox)} title=${JSON.stringify(titleBox)}`,
).toBe(false);
}
test("EN (LTR): Quick Actions dialog title does not overlap the Close X", async ({
page,
}) => {
await page.addInitScript(() => {
try {
window.localStorage.setItem("tx-lang", "en");
} catch {
/* ignore */
}
});
const date = pickFutureDate(40);
const id = await insertMeeting({
date,
titleEn: "QA Layout EN",
startTime: "10:00:00",
endTime: "10:30:00",
});
await loginViaUi(page, "admin", "admin123");
await page.goto("/executive-meetings");
await page.locator('input[type="date"]').first().fill(date);
await openDialogAndAssertNoOverlap(page, id);
});
test("AR (RTL): Quick Actions dialog title 'إجراءات سريعة' does not overlap the Close X", async ({
page,
}) => {
await page.addInitScript(() => {
try {
window.localStorage.setItem("tx-lang", "ar");
} catch {
/* ignore */
}
});
const date = pickFutureDate(41);
const id = await insertMeeting({
date,
titleEn: "QA Layout AR",
titleAr: "تخطيط الإجراءات السريعة",
startTime: "10:00:00",
endTime: "10:30:00",
});
await loginViaUi(page, "admin", "admin123");
await page.goto("/executive-meetings");
await page.locator('input[type="date"]').first().fill(date);
const surface = page.locator(`[data-testid="em-row-quick-${id}"]`);
// Sanity: dialog mounted in RTL.
await page.locator(`[data-testid="em-row-${id}"]`).click();
await expect(surface).toBeVisible({ timeout: 10_000 });
await expect(surface).toHaveAttribute("dir", "rtl");
// Re-run the overlap check via the shared helper (which re-opens the
// dialog via row click — already open here, but click is idempotent
// for already-open quick-actions and the helper just re-finds the
// surface).
const closeBtn = surface
.locator('button[type="button"]')
.filter({ hasText: /^Close$/ });
await expect(closeBtn).toHaveCount(1);
const titleEl = surface.locator('[id^="radix-"][id$="-title"]').first();
await expect(titleEl).toContainText("إجراءات سريعة");
const closeBox = await closeBtn.boundingBox();
const titleBox = await titleEl.boundingBox();
expect(closeBox).not.toBeNull();
expect(titleBox).not.toBeNull();
expect(
rectsOverlap(closeBox, titleBox),
`RTL title and Close button overlap. close=${JSON.stringify(closeBox)} title=${JSON.stringify(titleBox)}`,
).toBe(false);
});
@@ -0,0 +1,219 @@
// #499: e2e for the iPad long-press visual feedback. dnd-kit's
// TouchSensor has a 200ms activation delay before a touch becomes a
// drag. Before #499, the row gave NO visual cue during that window —
// users would tap-and-hold a meeting to drag it but, seeing nothing
// happen, lift their finger early thinking the schedule didn't
// register the touch. The fix paints a subtle inset ring on the row
// while a touch pointer is pressed, gated on `pointerType === "touch"`
// so mouse / pen pointers are unaffected. We expose the press state
// as `data-pressing="true"` on the row tr for deterministic testing.
//
// We use Chromium mobile emulation (`isMobile` + `hasTouch`) so the
// browser actually emits TouchEvents and PointerEvents with
// `pointerType === "touch"`, then drive presses through CDP because
// Playwright's high-level touchscreen API only exposes `tap()`.
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 the touch press-feedback test",
);
}
const pool = new pg.Pool({ connectionString: DATABASE_URL });
const createdMeetingIds = [];
test.use({
viewport: { width: 768, height: 1024 },
hasTouch: true,
isMobile: true,
});
async function loginViaUi(page, username, password) {
await page.goto("/login");
await page.locator("#username").fill(username);
await page.locator("#password").fill(password);
await Promise.all([
page.waitForURL((url) => !url.pathname.endsWith("/login"), {
timeout: 15_000,
}),
page.locator('form button[type="submit"]').click(),
]);
}
async function nextDailyNumberForDate(date) {
const { rows } = await pool.query(
`SELECT COALESCE(MAX(daily_number), 0) + 1 AS n
FROM executive_meetings
WHERE meeting_date = $1`,
[date],
);
return rows[0].n;
}
async function insertMeeting({ meetingDate, titleEn, startTime, endTime }) {
const dn = await nextDailyNumberForDate(meetingDate);
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, $2, $3, $4, $5, 'scheduled')
RETURNING id`,
[dn, titleEn, meetingDate, startTime, endTime],
);
const id = rows[0].id;
createdMeetingIds.push(id);
return id;
}
const RANDOM_DATE_BASE_DAYS =
365 + Math.floor(Math.random() * 1000) + ((Date.now() / 1000) | 0) % 500;
function uniqueFutureDate(offsetDays) {
const d = new Date();
d.setUTCDate(d.getUTCDate() + RANDOM_DATE_BASE_DAYS + offsetDays * 7);
return d.toISOString().slice(0, 10);
}
test.afterAll(async () => {
if (createdMeetingIds.length > 0) {
await pool.query(
`DELETE FROM executive_meeting_attendees 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_meetings WHERE id = ANY($1::int[])`,
[createdMeetingIds],
);
}
await pool.end();
});
test("Touch (iPad): pressing a row paints data-pressing=true within the 200ms TouchSensor delay, clears on release, and tap-to-open dialog still works", async ({
page,
}) => {
await page.addInitScript(() => {
try {
window.localStorage.setItem("tx-lang", "en");
} catch {
/* ignore */
}
});
const date = uniqueFutureDate(1);
const id = await insertMeeting({
meetingDate: date,
titleEn: `Touch Press Feedback ${Date.now().toString(36)}`,
startTime: "09:00:00",
endTime: "10:00:00",
});
await loginViaUi(page, "admin", "admin123");
await page.goto("/executive-meetings");
await page.locator('input[type="date"]').first().fill(date);
const row = page.getByTestId(`em-row-${id}`);
await expect(row).toBeVisible({ timeout: 10_000 });
// Baseline: not pressed.
await expect(row).not.toHaveAttribute("data-pressing", "true");
const numberCell = row.locator("td").first();
await numberCell.scrollIntoViewIfNeeded();
const box = await numberCell.boundingBox();
expect(box).not.toBeNull();
const x = box.x + box.width / 2;
const y = box.y + box.height / 2;
const cdp = await page.context().newCDPSession(page);
// 1. Touch down — DON'T move (stay inside the 8px tolerance window).
// The press cue should appear immediately on pointerdown, well
// inside the TouchSensor's 200ms activation delay.
await cdp.send("Input.dispatchTouchEvent", {
type: "touchStart",
touchPoints: [{ x, y, id: 1 }],
});
// Allow React to flush the state update from the capture-phase
// pointerdown handler. Should be effectively immediate, but we
// give 250ms upper bound so the assertion is robust on slow CI.
await expect(row).toHaveAttribute("data-pressing", "true", {
timeout: 250,
});
// 2. Release WITHOUT dragging — the cue must clear immediately and
// the tap-to-open quick-actions dialog must still fire (since
// the touch was a tap, not a long-press-into-drag).
await cdp.send("Input.dispatchTouchEvent", {
type: "touchEnd",
touchPoints: [],
});
await expect(row).not.toHaveAttribute("data-pressing", "true", {
timeout: 500,
});
// The original press WAS shorter than the 200ms drag-activation
// delay (no waitForTimeout between touchStart and touchEnd above),
// so dnd-kit treats it as a plain tap and the row's onClick fires
// on touchend → quick-actions dialog opens.
const surface = page.locator(`[data-testid="em-row-quick-${id}"]`);
await expect(surface).toBeVisible({ timeout: 5_000 });
});
test("Touch (iPad): press cue persists during the long-press hold (>200ms TouchSensor delay)", async ({
page,
}) => {
await page.addInitScript(() => {
try {
window.localStorage.setItem("tx-lang", "en");
} catch {
/* ignore */
}
});
const date = uniqueFutureDate(2);
const id = await insertMeeting({
meetingDate: date,
titleEn: `Touch Press Hold ${Date.now().toString(36)}`,
startTime: "11:00:00",
endTime: "12:00:00",
});
await loginViaUi(page, "admin", "admin123");
await page.goto("/executive-meetings");
await page.locator('input[type="date"]').first().fill(date);
const row = page.getByTestId(`em-row-${id}`);
await expect(row).toBeVisible({ timeout: 10_000 });
const numberCell = row.locator("td").first();
await numberCell.scrollIntoViewIfNeeded();
const box = await numberCell.boundingBox();
expect(box).not.toBeNull();
const x = box.x + box.width / 2;
const y = box.y + box.height / 2;
const cdp = await page.context().newCDPSession(page);
await cdp.send("Input.dispatchTouchEvent", {
type: "touchStart",
touchPoints: [{ x, y, id: 1 }],
});
// Hold past the 200ms TouchSensor activation delay — the cue must
// still be on the row (this is the whole point of the fix: feedback
// during the hold window so users don't lift early).
await page.waitForTimeout(350);
await expect(row).toHaveAttribute("data-pressing", "true");
// Release — cue clears.
await cdp.send("Input.dispatchTouchEvent", {
type: "touchEnd",
touchPoints: [],
});
await expect(row).not.toHaveAttribute("data-pressing", "true", {
timeout: 500,
});
});