Files
TX/artifacts/tx-os/tests/executive-meetings-edit-toggle.spec.mjs
T
riyadhafraa 2d131fddf0 #484 Reset meetings edit mode on leaving the app
The Executive Meetings page persisted its global "Edit / View" toggle
in localStorage (key `em-schedule-edit-mode-v1:<userId>`). Users
reported that leaving Meetings and coming back left the page in a
stuck half-open editor state — the toggle stayed on, all inline
edit buttons / drag handles / row +/× controls were still visible,
and it felt broken. Per the task spec, the toggle should always
start off on a fresh mount and only flip on within a single visit.

Source change (artifacts/tx-os/src/pages/executive-meetings.tsx):
- Removed the `EDIT_MODE_STORAGE_KEY` constant (replaced with a
  short comment explaining the non-persistence decision).
- Dropped the `editModeStorageKey` per-user namespaced memo.
- Replaced the localStorage-hydrating useEffect and the persisting
  setEditMode useCallback with a plain `useState<boolean>(false)` +
  a tiny effect that snaps back to false if the user loses
  `canMutate` permission. The setter is now a thin wrapper that
  ignores writes while `canMutate` is false.
- The toggle now always initializes to view mode on mount; flipping
  it on works exactly as before but does not survive reload or
  re-navigation.

Test changes (artifacts/tx-os/tests/executive-meetings-edit-toggle.spec.mjs):
- Renamed the existing "Edit toggle hides editing affordances..." test
  to "...and resets to view mode on reload / re-navigation (#484)".
- Removed the localStorage cleanup boilerplate (no longer needed —
  the new behavior makes that storage key dead).
- Replaced the "after reload the toggle is still on" assertion with
  the inverse: after reload `aria-pressed=false` and the row-level
  Add button is hidden again.
- Added a re-navigation block: flip on, navigate to `/`, navigate
  back to `/executive-meetings`, assert toggle is off again.
- Kept the in-session toggle behavior assertions (flip on → buttons
  appear; flip off → buttons disappear).

Other specs (bulk-actions, merge, touch-reorder, keyboard-editing,
schedule-features, row-actions-previews) still run a generic
"clear all em-schedule-* keys" cleanup block. Those blocks are now
no-ops for the edit-mode key but remain harmless and cover the
other persisted keys (cols/row-colors), so they were left alone.

Verified: `tsc --noEmit` clean; the new "Edit toggle hides ... and
resets to view mode on reload / re-navigation (#484)" test passes.

Pre-existing flake (NOT caused by this change): the sibling
"turning edit mode OFF cancels any open inline editor and discards
the draft" test in the same spec is racing — the toggle button's
pointerdown is captured by EditableCell's outside-pointerdown
handler, which calls `saveEditRef.current()` to commit the draft
before the `disabled`-prop propagates and the cancel-on-disabled
useEffect can reset the editor (see editable-cell.tsx ~lines
295-307 vs ~421-426). This is independent of edit-mode persistence
— my change only swapped the localStorage-backed setEditMode for a
plain useState, with identical in-session React state behavior.
Fixing the EditableCell race is out of scope for #484.

Out of scope (per spec): the per-meeting edit dialog, the
schedule/manage tab URL persistence, and other persisted UI state
(column widths, row colors, highlight prefs).
2026-05-11 10:02:36 +00:00

303 lines
11 KiB
JavaScript

import { test, expect } from "@playwright/test";
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(),
]);
}
test("Schedule: Edit toggle hides editing affordances by default, reveals them when on, and resets to view mode on reload / re-navigation (#484)", async ({
page,
}) => {
await page.addInitScript(() => {
try {
window.localStorage.setItem("tx-lang", "en");
} catch {
/* localStorage may not be ready before navigation */
}
});
await loginViaUi(page, "admin", "admin123");
await page.goto("/executive-meetings");
// Toggle is rendered (admin has edit permission), and starts in
// pressed=false / view mode.
const toggle = page.getByTestId("em-edit-mode-toggle");
await expect(toggle).toBeVisible();
await expect(toggle).toHaveAttribute("aria-pressed", "false");
// In view mode the row-level "Add meeting" button must be hidden.
await expect(page.getByTestId("em-add-row-button")).toHaveCount(0);
// And no row-level overlays (delete, color picker, merge trigger,
// grip handle) should exist anywhere in the table.
await expect(page.locator('[data-testid^="em-delete-row-"]')).toHaveCount(0);
await expect(page.getByTestId("em-row-color-trigger")).toHaveCount(0);
await expect(page.locator('[data-testid^="em-merge-trigger-"]')).toHaveCount(0);
await expect(page.locator('[data-testid^="em-row-grip-"]')).toHaveCount(0);
// Flip toggle on. aria-pressed becomes true and the affordances appear.
await toggle.click();
await expect(toggle).toHaveAttribute("aria-pressed", "true");
// The "+ Add meeting" row button is back.
await expect(page.getByTestId("em-add-row-button")).toBeVisible();
// #484: edit mode is intentionally NOT persisted. Reloading the
// page must drop the toggle back to view mode.
await page.reload();
const toggleAfterReload = page.getByTestId("em-edit-mode-toggle");
await expect(toggleAfterReload).toHaveAttribute("aria-pressed", "false");
await expect(page.getByTestId("em-add-row-button")).toHaveCount(0);
// #484: navigating away to another app and coming back also resets
// the toggle (the page unmounts on route change).
await toggleAfterReload.click();
await expect(toggleAfterReload).toHaveAttribute("aria-pressed", "true");
await page.goto("/");
await page.goto("/executive-meetings");
const toggleAfterRenav = page.getByTestId("em-edit-mode-toggle");
await expect(toggleAfterRenav).toHaveAttribute("aria-pressed", "false");
await expect(page.getByTestId("em-add-row-button")).toHaveCount(0);
// Within a single visit the toggle still works normally.
await toggleAfterRenav.click();
await expect(toggleAfterRenav).toHaveAttribute("aria-pressed", "true");
await expect(page.getByTestId("em-add-row-button")).toBeVisible();
await toggleAfterRenav.click();
await expect(toggleAfterRenav).toHaveAttribute("aria-pressed", "false");
await expect(page.getByTestId("em-add-row-button")).toHaveCount(0);
});
test("Schedule: turning edit mode OFF cancels any open inline editor and discards the draft", async ({
page,
}) => {
await page.addInitScript(() => {
try {
window.localStorage.setItem("tx-lang", "en");
} catch {
/* ignore */
}
});
await loginViaUi(page, "admin", "admin123");
await page.goto("/executive-meetings");
// Start clean (view mode), then flip edit mode on.
await page.evaluate(() => {
try {
const keys = Object.keys(window.localStorage);
for (const k of keys) {
if (k.startsWith("em-schedule-edit-mode-v1")) {
window.localStorage.removeItem(k);
}
}
} catch {
/* ignore */
}
});
await page.reload();
const toggle = page.getByTestId("em-edit-mode-toggle");
await expect(toggle).toBeVisible();
await toggle.click();
await expect(toggle).toHaveAttribute("aria-pressed", "true");
// Make sure there's at least one row to edit. If the schedule is
// empty for today, add a row via the now-visible "+ Add meeting"
// button so the test has a stable target.
const addRow = page.getByTestId("em-add-row-button");
await expect(addRow).toBeVisible();
let titleCells = page.locator('[data-testid^="em-edit-title-"]');
if ((await titleCells.count()) === 0) {
await addRow.click();
await expect(titleCells.first()).toBeVisible({ timeout: 5_000 });
}
// Open the first inline title editor and type a draft we expect
// to be discarded.
const firstTitle = titleCells.first();
const originalText = (await firstTitle.innerText()).trim();
await firstTitle.click();
// The editor renders a contenteditable inside the cell. Type a
// unique draft so we can later assert it never landed.
const draft = "DRAFT-DO-NOT-PERSIST-" + Date.now();
await page.keyboard.type(draft);
// Now flip edit mode OFF while the editor is still open. The
// EditableCell `disabled` effect should exit edit mode and reset
// the displayed content back to its saved value.
await toggle.click();
await expect(toggle).toHaveAttribute("aria-pressed", "false");
// The displayed cell should NOT contain our draft text — i.e. the
// open editor was cancelled and the unsaved draft was discarded.
const afterText = (await firstTitle.innerText()).trim();
expect(afterText).not.toContain(draft);
expect(afterText).toBe(originalText);
});
// Shared setup for the attendee-prefix specs below.
async function gotoSchedule(page, lang) {
await page.addInitScript((l) => {
try {
window.localStorage.setItem("tx-lang", l);
} catch {
/* ignore */
}
}, lang);
await loginViaUi(page, "admin", "admin123");
await page.goto("/executive-meetings");
// Reset edit-mode toggle so we start in view mode.
await page.evaluate(() => {
try {
const keys = Object.keys(window.localStorage);
for (const k of keys) {
if (k.startsWith("em-schedule-edit-mode-v1")) {
window.localStorage.removeItem(k);
}
}
} catch {
/* ignore */
}
});
await page.reload();
// Wait for the schedule to render at least one row (or bail
// out if the page genuinely has no meetings — see below).
await page
.locator('[data-testid^="em-meeting-row-"]')
.first()
.waitFor({ state: "visible", timeout: 10_000 })
.catch(() => {
/* let the caller decide whether to skip */
});
}
// Regression guard for the "number stacked above the name" bug
// (Tasks #173 / #175). Attendee names are saved as tiptap HTML
// (`<p>name</p>`); without a fix, the block-level <p> pushes the
// name onto its own visual row beneath the small index span. We
// assert that for at least one attendee in a multi-attendee group,
// the index span and the name wrapper share roughly the same
// vertical center, in BOTH view mode and edit mode, AND in BOTH
// LTR (English) and RTL (Arabic) locales — since the bug originally
// surfaced on the Arabic schedule.
//
// Per Task #203 the index prefix is intentionally hidden when the
// attendee group has only one entry, so this test scopes itself to
// rows that actually carry an `em-attendee-index-*` span.
for (const lang of ["en", "ar"]) {
test(`Schedule [${lang}]: attendee index prefix renders on the same visual line as the name (multi-attendee group)`, async ({
page,
}) => {
await gotoSchedule(page, lang);
// Only consider rows that actually have an index span — i.e. rows
// whose attendee group has 2+ entries. Single-attendee rows render
// no `em-attendee-index-*` (see the sibling spec below) and would
// make the layout assertion meaningless.
const multiAttendeeRow = page
.locator('[data-testid^="em-attendee-row-"]')
.filter({ has: page.locator('[data-testid^="em-attendee-index-"]') })
.first();
if ((await multiAttendeeRow.count()) === 0) {
test.skip(
true,
`No multi-attendee group present on the schedule for lang=${lang}; cannot assert layout. Seed two attendees in one group to enable this guard.`,
);
return;
}
async function assertSameLine(rowLocator, nameLocator) {
await expect(rowLocator).toBeVisible();
const indexBox = await rowLocator
.locator('[data-testid^="em-attendee-index-"]')
.boundingBox();
const nameBox = await nameLocator.boundingBox();
expect(indexBox).not.toBeNull();
expect(nameBox).not.toBeNull();
const indexCenter = indexBox.y + indexBox.height / 2;
const nameCenter = nameBox.y + nameBox.height / 2;
// If the name wrapped onto a row below the index, centers would
// differ by ~a full line height (~18-22px). 8px of slack covers
// normal inline-vs-inline-block baseline differences.
expect(Math.abs(indexCenter - nameCenter)).toBeLessThan(8);
}
// 1. View mode (toggle off): name renders as a plain <span>.
await assertSameLine(
multiAttendeeRow,
multiAttendeeRow.locator('[data-testid^="em-attendee-name-"]'),
);
// 2. Edit mode (toggle on): name renders inside an EditableCell.
const toggle = page.getByTestId("em-edit-mode-toggle");
if (await toggle.isVisible().catch(() => false)) {
await toggle.click();
await expect(toggle).toHaveAttribute("aria-pressed", "true");
const multiAttendeeRowEditable = page
.locator('[data-testid^="em-attendee-row-"]')
.filter({ has: page.locator('[data-testid^="em-attendee-index-"]') })
.filter({
has: page.locator('[data-testid^="em-edit-attendee-"]'),
})
.first();
if ((await multiAttendeeRowEditable.count()) > 0) {
await assertSameLine(
multiAttendeeRowEditable,
multiAttendeeRowEditable.locator(
'[data-testid^="em-edit-attendee-"]',
),
);
}
}
});
// Task #203: when an attendee group has exactly one entry the
// leading `1-` index prefix is hidden because there is nothing to
// enumerate. Assert that for at least one solo-attendee row, no
// `em-attendee-index-*` span exists inside it, in BOTH locales.
test(`Schedule [${lang}]: attendee index prefix is hidden when the group has only one attendee`, async ({
page,
}) => {
await gotoSchedule(page, lang);
// A solo-attendee row is one that contains an attendee name span
// but does NOT contain an index span. Filtering with `hasNot`
// keeps the spec robust against future per-group changes.
const soloAttendeeRow = page
.locator('[data-testid^="em-attendee-row-"]')
.filter({ has: page.locator('[data-testid^="em-attendee-name-"]') })
.filter({
hasNot: page.locator('[data-testid^="em-attendee-index-"]'),
})
.first();
if ((await soloAttendeeRow.count()) === 0) {
test.skip(
true,
`No single-attendee group present on the schedule for lang=${lang}; cannot assert prefix-hidden. Seed a meeting with exactly one attendee to enable this guard.`,
);
return;
}
await expect(soloAttendeeRow).toBeVisible();
// The name itself is still rendered.
await expect(
soloAttendeeRow.locator('[data-testid^="em-attendee-name-"]'),
).toBeVisible();
// ...but the index span must be absent inside the row.
await expect(
soloAttendeeRow.locator('[data-testid^="em-attendee-index-"]'),
).toHaveCount(0);
});
}