From 2d131fddf0c06013c3eff48493753a52842631b6 Mon Sep 17 00:00:00 2001 From: riyadhafraa <49757212-riyadhafraa@users.noreply.replit.com> Date: Mon, 11 May 2026 10:02:36 +0000 Subject: [PATCH] #484 Reset meetings edit mode on leaving the app MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Executive Meetings page persisted its global "Edit / View" toggle in localStorage (key `em-schedule-edit-mode-v1:`). 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(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). --- .../tx-os/src/pages/executive-meetings.tsx | 48 ++++++------------- .../executive-meetings-edit-toggle.spec.mjs | 48 ++++++++----------- 2 files changed, 35 insertions(+), 61 deletions(-) diff --git a/artifacts/tx-os/src/pages/executive-meetings.tsx b/artifacts/tx-os/src/pages/executive-meetings.tsx index d916f884..5a00ed3f 100644 --- a/artifacts/tx-os/src/pages/executive-meetings.tsx +++ b/artifacts/tx-os/src/pages/executive-meetings.tsx @@ -349,7 +349,9 @@ const ROW_COLOR_OPTIONS: { key: string; bg: string; border: string; label: strin const COLS_STORAGE_KEY = "em-schedule-cols-v1"; const ROW_COLORS_STORAGE_KEY = "em-schedule-row-colors-v1"; const HIGHLIGHT_STORAGE_KEY = "em-current-meeting-highlight-v1"; -const EDIT_MODE_STORAGE_KEY = "em-schedule-edit-mode-v1"; +// #484: edit-mode is intentionally NOT persisted. It always starts off +// when the Meetings page mounts so users never come back to a stuck +// half-open editor state after navigating away. type HighlightPrefs = { enabled: boolean; color: string }; const DEFAULT_HIGHLIGHT_PREFS: HighlightPrefs = { @@ -1075,46 +1077,24 @@ function ScheduleSection({ // (no edit affordances). When a user with edit permission flips it on, // every per-cell editor and per-row action (merge, color swatch, drag // handle, add/delete row, inline edit, column drag, column resize) is - // re-enabled. We persist the user's last choice in localStorage so the - // toggle survives reloads + date changes, but we *also* always start in - // view mode for users without permission so they never see a stale "on" - // state from a previous session where they had access. Hydrating the - // localStorage value happens inside an effect to keep SSR-safe and to - // avoid reading from a non-mutator's namespace. + // re-enabled. // - // The storage key is namespaced by the current user's id so a shared - // browser profile can't leak one editor's last toggle state into the - // next account that signs in. When userId is null (no /me yet) we - // fall back to view mode without touching storage. - const editModeStorageKey = useMemo( - () => (userId != null ? `${EDIT_MODE_STORAGE_KEY}:${userId}` : null), - [userId], - ); + // #484: the toggle is NOT persisted. Every fresh page mount — whether + // from a reload or from navigating back to Meetings from another app — + // starts in view mode so the user never returns to a stuck half-open + // editor state. The in-memory state still flips normally during a + // single visit. We also force back to false the moment a user loses + // mutate permission so they never see a stale "on". const [editMode, setEditModeState] = useState(false); useEffect(() => { - if (!canMutate || !editModeStorageKey) { - setEditModeState(false); - return; - } - try { - const raw = window.localStorage.getItem(editModeStorageKey); - setEditModeState(raw === "true"); - } catch { - // localStorage may be unavailable (private mode); fall through to - // the default false. - } - }, [canMutate, editModeStorageKey]); + if (!canMutate) setEditModeState(false); + }, [canMutate]); const setEditMode = useCallback( (next: boolean) => { - if (!canMutate || !editModeStorageKey) return; + if (!canMutate) return; setEditModeState(next); - try { - window.localStorage.setItem(editModeStorageKey, next ? "true" : "false"); - } catch { - // Persistence is best-effort; the in-memory state still flips. - } }, - [canMutate, editModeStorageKey], + [canMutate], ); // Anything the user can mutate must AND with editMode. We pass this // derived flag down to every row + cell so they don't each have to diff --git a/artifacts/tx-os/tests/executive-meetings-edit-toggle.spec.mjs b/artifacts/tx-os/tests/executive-meetings-edit-toggle.spec.mjs index b5cf7bab..80c827c4 100644 --- a/artifacts/tx-os/tests/executive-meetings-edit-toggle.spec.mjs +++ b/artifacts/tx-os/tests/executive-meetings-edit-toggle.spec.mjs @@ -12,7 +12,7 @@ async function loginViaUi(page, username, password) { ]); } -test("Schedule: Edit toggle hides editing affordances by default and reveals them when on", async ({ +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(() => { @@ -24,27 +24,7 @@ test("Schedule: Edit toggle hides editing affordances by default and reveals the }); await loginViaUi(page, "admin", "admin123"); - // Make sure we always start the test in view mode regardless of any - // previously-persisted toggle state from another run. We clear AFTER - // login so the cleanup isn't undone by the initScript on every nav. await page.goto("/executive-meetings"); - // The toggle's storage key is namespaced per-user - // (`em-schedule-edit-mode-v1:`), so we wipe every entry that - // starts with the base key to make sure we begin in view mode no - // matter who admin's userId resolves to in this run. - 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(); // Toggle is rendered (admin has edit permission), and starts in // pressed=false / view mode. @@ -68,16 +48,30 @@ test("Schedule: Edit toggle hides editing affordances by default and reveals the // The "+ Add meeting" row button is back. await expect(page.getByTestId("em-add-row-button")).toBeVisible(); - // Persist + survive a reload: the toggle should still be on. + // #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", "true"); - await expect(page.getByTestId("em-add-row-button")).toBeVisible(); - - // Toggle back off — affordances disappear again. - await toggleAfterReload.click(); 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 ({