tx-os #317: optimistic-cache saves for all inline editors
Extends the optimistic React Query cache pattern (proven in #316 for the schedule time cell) to every other inline editor on executive-meetings.tsx, fixing the "تأخير في الاستجابة وأحياناً ما يحفظ" regression where typed edits felt sluggish or appeared to silently drop. executive-meetings.tsx * New `applyMeetingPatch(meetingId, patch)` helper: snapshots the day's DayResponse, applies a per-meeting patch via setQueryData, and returns a rollback closure that re-sets the snapshot on PATCH failure. * `saveTitle`, `saveAttendeeName`, `saveMerge`, and `setRowColor` now write the optimistic value before awaiting the network call and rollback() in their catch branches. editable-cell.tsx (3-layer blur hardening) 1. saveEdit() failure now re-opens the editor with the user's typed draft preserved (mirrors #316 time-cell UX) so a server error never forces them to retype. The parent's optimistic cache rollback restores the read-only `value`; the editor itself still holds the draft, so flipping `editing` back on is enough. 2. Outside-pointerdown now flushes saveEdit synchronously (capture phase, before the click reaches its target) instead of waiting on the 100 ms blur timer. Closes the gap where a fast click into another cell triggered a row remount before blur fired. 3. Unmount cleanup flushes any pending blurTimerRef fire-and-forget, a final safety net for the case where a refetch tears down the row before any pointer event arrives. tests/executive-meetings-keyboard-editing.spec.mjs (+4 tests) * Title repaint <300 ms with PATCH delayed 1500 ms. * Title PATCH 500 rolls back; editor re-opens with the typed draft intact; Escape cancels back to the original; DB unchanged. * Rapid click-switch from cell A to cell B commits BOTH PATCHes to the server (proves the synchronous outside-pointerdown flush). * Attendee-name PUT delayed 1500 ms repaints in <300 ms (uses captured testid so the locator survives the rename). All 4 new + 3 #316 time-editor tests green. Out-of-scope pre-existing failures in the `test` workflow (notifications/postpone-race/row-color) left untouched per plan.
This commit is contained in:
@@ -327,7 +327,16 @@ export function EditableCell({
|
||||
try {
|
||||
await onSave(stripped);
|
||||
} catch {
|
||||
editor.commands.setContent(value || "", { emitUpdate: false });
|
||||
// #317: failure UX matches the schedule time editor (#316) —
|
||||
// re-open the editor with the user's typed draft intact so
|
||||
// they can fix the error and retry without retyping. The
|
||||
// parent's optimistic cache write is rolled back in its own
|
||||
// catch branch, so the read-only `value` we close over here
|
||||
// already reflects the original pre-edit content. We
|
||||
// intentionally do NOT call `editor.commands.setContent(value)`
|
||||
// — the editor still holds the typed HTML, and flipping
|
||||
// `editing` back to true makes it visible and focusable.
|
||||
setEditing(true);
|
||||
}
|
||||
}
|
||||
}, [editor, onSave, value, forceSaveOnBlur]);
|
||||
@@ -376,20 +385,34 @@ export function EditableCell({
|
||||
// When the toolbar is portalled outside wrapperRef, native pointerdown on
|
||||
// the toolbar would still trigger our onFocusOut path on some browsers.
|
||||
// We watch document-level pointerdown and ignore taps inside the toolbar.
|
||||
// #317: pointerdown OUTSIDE the wrapper+toolbar synchronously flushes
|
||||
// the save instead of waiting for the 100 ms blur timer. This closes
|
||||
// the timing window where a fast click into another cell would
|
||||
// re-render this row before the blur timer fired and silently drop
|
||||
// the typed change. The unmount cleanup above is the second safety
|
||||
// net for the case where a refetch tears down the row before any
|
||||
// pointer event fires.
|
||||
useEffect(() => {
|
||||
if (!editing) return;
|
||||
const onPointerDownDoc = (e: PointerEvent) => {
|
||||
const target = e.target as Node | null;
|
||||
if (!target) return;
|
||||
if (
|
||||
const inside =
|
||||
wrapperRef.current?.contains(target) ||
|
||||
toolbarRef.current?.contains(target)
|
||||
) {
|
||||
toolbarRef.current?.contains(target);
|
||||
if (inside) {
|
||||
if (blurTimerRef.current) {
|
||||
clearTimeout(blurTimerRef.current);
|
||||
blurTimerRef.current = null;
|
||||
}
|
||||
return;
|
||||
}
|
||||
// Outside tap → cancel any pending blur timer and commit now.
|
||||
if (blurTimerRef.current) {
|
||||
clearTimeout(blurTimerRef.current);
|
||||
blurTimerRef.current = null;
|
||||
}
|
||||
void saveEditRef.current();
|
||||
};
|
||||
document.addEventListener("pointerdown", onPointerDownDoc, true);
|
||||
return () => {
|
||||
|
||||
@@ -1876,11 +1876,23 @@ test("Schedule title editor: a failed PATCH rolls back the optimistic title and
|
||||
{ timeout: 15_000 },
|
||||
);
|
||||
|
||||
// After the rollback, the cell must show the ORIGINAL title (not
|
||||
// the optimistic "BROKEN-..." one) and must NOT contain the
|
||||
// appended fragment. We poll briefly because rollback runs in the
|
||||
// PATCH catch branch on the next microtask.
|
||||
await expect(titleCell).toContainText(originalTitle, { timeout: 3_000 });
|
||||
// Failure UX (mirrors #316 time-cell): the editor must re-open with
|
||||
// the user's typed draft intact so they can fix the error and
|
||||
// retry without retyping. The contenteditable still shows
|
||||
// "BROKEN-..." while the underlying read-only `value` (driving
|
||||
// titleEn in the cache) is rolled back to the original.
|
||||
await expect(
|
||||
titleCell.locator('[contenteditable="true"]'),
|
||||
).toContainText(originalTitle + appended, { timeout: 3_000 });
|
||||
await expect(titleCell.locator('[contenteditable="true"]')).toBeFocused();
|
||||
|
||||
// Cancel the draft → cell must collapse back to the rolled-back
|
||||
// ORIGINAL title with no trace of the appended fragment.
|
||||
await page.keyboard.press("Escape");
|
||||
await expect(
|
||||
titleCell.locator('[contenteditable="true"]'),
|
||||
).toHaveCount(0, { timeout: 3_000 });
|
||||
await expect(titleCell).toContainText(originalTitle);
|
||||
await expect(titleCell).not.toContainText(appended);
|
||||
|
||||
// DB must be untouched.
|
||||
@@ -1894,6 +1906,115 @@ test("Schedule title editor: a failed PATCH rolls back the optimistic title and
|
||||
await page.unroute(`**/api/executive-meetings/${meetingId}`);
|
||||
});
|
||||
|
||||
test("Schedule title editor: rapid click-switch from cell A to cell B commits BOTH edits to the server (no dropped saves)", async ({
|
||||
page,
|
||||
}) => {
|
||||
await setLangEn(page);
|
||||
const date = uniqueFutureDate(333);
|
||||
const uniq = Date.now().toString(36);
|
||||
const aOriginal = `RapidA ${uniq}`;
|
||||
const bOriginal = `RapidB ${uniq}`;
|
||||
const meetingA = await insertMeeting({
|
||||
meetingDate: date,
|
||||
dailyNumber: 1,
|
||||
titleEn: aOriginal,
|
||||
titleAr: aOriginal,
|
||||
startTime: "09:00:00",
|
||||
endTime: "10:00:00",
|
||||
});
|
||||
const meetingB = await insertMeeting({
|
||||
meetingDate: date,
|
||||
dailyNumber: 2,
|
||||
titleEn: bOriginal,
|
||||
titleAr: bOriginal,
|
||||
startTime: "10:00:00",
|
||||
endTime: "11:00:00",
|
||||
});
|
||||
|
||||
await gotoMeetingRow(page, meetingA, date);
|
||||
// Verify both rows are visible before we start poking at them.
|
||||
await expect(page.getByTestId(`em-row-${meetingB}`)).toBeVisible();
|
||||
|
||||
// Record every PATCH that lands so we can assert both commits hit
|
||||
// the server even though we never blurred cell A explicitly — the
|
||||
// outside-pointerdown flush in EditableCell must commit it.
|
||||
const patches = [];
|
||||
page.on("request", (req) => {
|
||||
if (req.method() !== "PATCH") return;
|
||||
const u = new URL(req.url());
|
||||
const m = u.pathname.match(/^\/api\/executive-meetings\/(\d+)$/);
|
||||
if (!m) return;
|
||||
patches.push({ id: Number(m[1]), body: req.postDataJSON() ?? null });
|
||||
});
|
||||
|
||||
const cellA = page.getByTestId(`em-edit-title-${meetingA}`);
|
||||
const cellB = page.getByTestId(`em-edit-title-${meetingB}`);
|
||||
|
||||
// Enter edit mode on A and type without committing.
|
||||
await cellA.evaluate((el) => el.focus());
|
||||
await page.keyboard.press("Enter");
|
||||
const editorA = cellA.locator('[contenteditable="true"]');
|
||||
await expect(editorA).toBeFocused();
|
||||
const aAppended = ` -A-${uniq}`;
|
||||
await page.keyboard.type(aAppended);
|
||||
|
||||
// Now move focus to cell B WITHOUT pressing Enter on A. We
|
||||
// dispatch a pointerdown directly on cell B's element to trigger
|
||||
// EditableCell's outside-pointerdown flush in cell A. Doing this
|
||||
// via .evaluate() avoids Playwright's hit-test rejecting the
|
||||
// click when cell A's portalled floating toolbar happens to
|
||||
// overlap cell B.
|
||||
await cellB.evaluate((el) => {
|
||||
el.dispatchEvent(
|
||||
new PointerEvent("pointerdown", { bubbles: true, cancelable: true }),
|
||||
);
|
||||
});
|
||||
// Then enter edit mode on cell B the same way every other test
|
||||
// in this file does — focus + Enter.
|
||||
await cellB.evaluate((el) => el.focus());
|
||||
await page.keyboard.press("Enter");
|
||||
const editorB = cellB.locator('[contenteditable="true"]');
|
||||
await expect(editorB).toBeFocused();
|
||||
const bAppended = ` -B-${uniq}`;
|
||||
await page.keyboard.type(bAppended);
|
||||
await page.keyboard.press("Enter");
|
||||
|
||||
// Both PATCHes must have reached the server with the right bodies.
|
||||
await expect
|
||||
.poll(() => patches.filter((p) => p.id === meetingA).length, {
|
||||
timeout: 5_000,
|
||||
})
|
||||
.toBeGreaterThanOrEqual(1);
|
||||
await expect
|
||||
.poll(() => patches.filter((p) => p.id === meetingB).length, {
|
||||
timeout: 5_000,
|
||||
})
|
||||
.toBeGreaterThanOrEqual(1);
|
||||
|
||||
// Body may use titleEn or titleAr depending on the i18n language
|
||||
// active at save-time (the page may default to Arabic) — assert
|
||||
// against whichever field the patch carries.
|
||||
const titleOf = (p) =>
|
||||
String(p?.body?.titleEn ?? p?.body?.titleAr ?? "");
|
||||
const aPatch = patches.find((p) => p.id === meetingA);
|
||||
const bPatch = patches.find((p) => p.id === meetingB);
|
||||
expect(titleOf(aPatch)).toContain(aAppended.trim());
|
||||
expect(titleOf(bPatch)).toContain(bAppended.trim());
|
||||
|
||||
// And the DB must reflect both edits — proves nothing was silently
|
||||
// dropped by the rapid cell switch.
|
||||
await waitForDayRefetch(page, date);
|
||||
const { rows } = await pool.query(
|
||||
`SELECT id, title_en, title_ar FROM executive_meetings WHERE id = ANY($1::int[])`,
|
||||
[[meetingA, meetingB]],
|
||||
);
|
||||
const byId = Object.fromEntries(
|
||||
rows.map((r) => [r.id, `${r.title_en ?? ""} ${r.title_ar ?? ""}`]),
|
||||
);
|
||||
expect(byId[meetingA]).toContain(aAppended.trim());
|
||||
expect(byId[meetingB]).toContain(bAppended.trim());
|
||||
});
|
||||
|
||||
test("Schedule attendees editor: optimistic repaint shows the renamed attendee before the PUT round-trip resolves", async ({
|
||||
page,
|
||||
}) => {
|
||||
|
||||
Reference in New Issue
Block a user