#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).
This commit is contained in:
@@ -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<boolean>(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
|
||||
|
||||
@@ -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:<userId>`), 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 ({
|
||||
|
||||
Reference in New Issue
Block a user