2d131fddf0
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).
303 lines
11 KiB
JavaScript
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);
|
|
});
|
|
|
|
}
|