fix: ask before the rail or the mountain view top bar leaves an event or task with unsaved changes

This commit is contained in:
Linus Rath
2026-10-28 12:50:59 +01:00
parent ab417728ec
commit 382b26f128
5 changed files with 179 additions and 12 deletions
@@ -1,4 +1,4 @@
import { render, screen, fireEvent, within, waitFor } from '@testing-library/react';
import { act, render, screen, fireEvent, within, waitFor } from '@testing-library/react';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import type { Calendar, CalendarEvent } from '@/lib/jmap/types';
import { useCalendarStore } from '@/stores/calendar-store';
@@ -31,6 +31,7 @@ vi.mock('@/hooks/use-config', () => ({ useConfig: () => config }));
import { CalendarApp } from '../calendar-app';
import { useRegisteredCommandGroups } from '@/lib/command-registry';
import { interceptLeave } from '@/lib/leave-guard';
// The first render loads the whole page and is slow under load.
vi.setConfig({ testTimeout: 30000 });
@@ -273,6 +274,38 @@ describe('CalendarApp unsaved editor changes', () => {
fireEvent.click(within(ask).getByRole('button', { name: 'discard.confirm' }));
await waitFor(() => expect(screen.queryByPlaceholderText('tasks.title_placeholder')).toBeNull());
});
it('has the rail ask before it leaves an editor with changes', async () => {
renderApp();
// An untouched editor lets the rail go at once.
fireEvent.keyDown(document.body, { key: 'n', code: 'KeyN' });
const navigated = vi.fn();
// A rail link wired as the real one is: interceptLeave first.
const link = document.createElement('a');
link.addEventListener('click', (e) => { if (!interceptLeave(e)) navigated(); });
document.body.appendChild(link);
try {
act(() => { link.click(); });
expect(navigated).toHaveBeenCalledOnce();
expect(screen.queryByRole('alertdialog')).toBeNull();
fireEvent.change(screen.getByPlaceholderText('form.title'), { target: { value: 'Lunch' } });
act(() => { link.click(); });
const ask = await screen.findByRole('alertdialog', { name: 'discard.title' });
expect(navigated).toHaveBeenCalledOnce();
fireEvent.click(within(ask).getByRole('button', { name: 'discard.keep' }));
await waitFor(() => expect(screen.queryByRole('alertdialog')).toBeNull());
expect(screen.getByPlaceholderText('form.title')).toHaveValue('Lunch');
expect(navigated).toHaveBeenCalledOnce();
act(() => { link.click(); });
const again = await screen.findByRole('alertdialog', { name: 'discard.title' });
fireEvent.click(within(again).getByRole('button', { name: 'discard.confirm' }));
await waitFor(() => expect(navigated).toHaveBeenCalledTimes(2));
} finally {
link.remove();
}
});
});
describe('CalendarApp browser Back with an editor open (R5)', () => {
+17 -1
View File
@@ -87,6 +87,7 @@ import { appPath, buildCalendarPath, parseCalendarPath, type CalendarDeepLink }
import { consumePendingDeepLinkEntry, subscribePendingDeepLink } from "@/lib/deep-link-handoff";
import { useDeepLinkUrl } from "@/hooks/use-deep-link-url";
import { useCloseOnBack } from "@/hooks/use-close-on-back";
import { useLeaveGuard } from "@/lib/leave-guard";
import { useCalendarEventSearch } from "@/hooks/use-calendar-event-search";
import { CalendarSearchField, CalendarSearchResults } from "@/components/calendar/calendar-search";
import { useProInterfaceActive } from "@/components/pro/pro-interface-redirect";
@@ -966,11 +967,26 @@ export function CalendarApp({ linkSegments: routeSegments }: CalendarAppProps =
setShowTaskModal(false);
setEditTask(null);
}, []);
const editorsInView = (showEventModal || showTaskModal) && (isEmbedded ? isFocusedProTab : !proInterfaceActive);
useCloseOnBack({
active: (showEventModal || showTaskModal) && (isEmbedded ? isFocusedProTab : !proInterfaceActive),
active: editorsInView,
shouldHold: () => eventEditorDirtyRef.current || taskEditorDirtyRef.current,
onBack: () => whenEditorsDiscarded(closeEditors),
});
// The rail, the phone tab bar and the Mountain View top bar ask too,
// before they leave the calendar (R5). Only ask there: leaving unmounts
// the editor, and clearing it first would let the Back guard above take
// its history step back over the navigation.
useLeaveGuard(
() => editorsInView && (eventEditorDirtyRef.current || taskEditorDirtyRef.current),
() => confirmAction({
title: t("discard.title"),
message: t("discard.message"),
confirmText: t("discard.confirm"),
cancelText: t("discard.keep"),
variant: "destructive",
}),
);
const handleEditFromDetail = useCallback(() => {
if (detailEvent) {
@@ -0,0 +1,95 @@
import { act, fireEvent, render, screen, waitFor } from '@testing-library/react';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import type React from 'react';
// R5: an editor with unsaved changes asks before Mountain View's top bar
// leaves it, as the rail and the phone tab bar do.
const nav = vi.hoisted(() => ({ push: vi.fn(), followed: vi.fn() }));
const switcher = vi.hoisted(() => ({ onSelectAllInboxes: undefined as undefined | (() => void) }));
vi.mock('../account-switcher', () => ({
AccountSwitcher: (props: { onSelectAllInboxes?: () => void }) => {
switcher.onSelectAllInboxes = props.onSelectAllInboxes;
return null;
},
}));
vi.mock('@/hooks/use-config', () => ({ useConfig: () => ({ appName: 'Mail' }) }));
vi.mock('@/hooks/use-stalwart-admin', () => ({ useStalwartAdmin: () => false }));
vi.mock('@/i18n/navigation', () => ({
useRouter: () => ({ push: nav.push, back: vi.fn(), replace: vi.fn() }),
usePathname: () => '/calendar',
// Like next/link: the handler runs first, and a prevented click stays put.
Link: ({ href, onClick, children, ...rest }: React.AnchorHTMLAttributes<HTMLAnchorElement> & { href: string }) => (
<a
{...rest}
href={href}
onClick={(e) => {
onClick?.(e);
if (!e.defaultPrevented) nav.followed(href);
e.preventDefault();
}}
>
{children}
</a>
),
}));
const { MvTopBar } = await import('../mv-top-bar');
const { registerLeaveGuard } = await import('@/lib/leave-guard');
const { useSettingsStore } = await import('@/stores/settings-store');
const { useAccountStore } = await import('@/stores/account-store');
const { usePolicyStore } = await import('@/stores/policy-store');
const { DEFAULT_POLICY } = await import('@/lib/admin/types');
let unregister: (() => void) | null = null;
let confirm: ReturnType<typeof vi.fn<() => Promise<boolean>>>;
beforeEach(() => {
nav.push.mockClear();
nav.followed.mockClear();
usePolicyStore.setState({ policy: { ...DEFAULT_POLICY, features: { ...DEFAULT_POLICY.features, unifiedCrossAccountEnabled: true } } });
useSettingsStore.setState({ enableUnifiedMailbox: true, unifiedCrossAccount: true } as never);
useAccountStore.setState({ accounts: [{ id: 'a', isConnected: true }, { id: 'b', isConnected: true }] } as never);
confirm = vi.fn(async () => true);
unregister = registerLeaveGuard({ isDirty: () => true, confirm });
});
afterEach(() => { unregister?.(); unregister = null; });
describe('Mountain View top bar with unsaved changes', () => {
it('asks before the Settings button leaves, then goes on', async () => {
render(<MvTopBar showMenuButton={false} />);
fireEvent.click(screen.getByRole('link', { name: 'settings' }));
expect(confirm).toHaveBeenCalledOnce();
expect(nav.followed).not.toHaveBeenCalled();
await waitFor(() => expect(nav.followed).toHaveBeenCalledWith('/settings'));
});
it('stays when the user keeps editing', async () => {
confirm.mockResolvedValue(false);
render(<MvTopBar showMenuButton={false} />);
fireEvent.click(screen.getByRole('link', { name: 'Mail' }));
await waitFor(() => expect(confirm).toHaveBeenCalledOnce());
await act(async () => { await new Promise((r) => setTimeout(r, 10)); });
expect(nav.followed).not.toHaveBeenCalled();
expect(nav.push).not.toHaveBeenCalled();
});
it('asks before an app in the apps menu, and opens it after the menu has closed', async () => {
render(<MvTopBar showMenuButton={false} />);
fireEvent.click(screen.getByRole('button', { name: 'add_app' }));
fireEvent.click(screen.getByRole('menuitem', { name: 'mail' }));
expect(confirm).toHaveBeenCalledOnce();
expect(nav.followed).not.toHaveBeenCalled();
expect(screen.queryByRole('menu')).toBeNull();
await waitFor(() => expect(nav.push).toHaveBeenCalledWith('/'));
});
it('asks before "All inboxes" opens mail', async () => {
render(<MvTopBar showMenuButton={false} />);
act(() => switcher.onSelectAllInboxes!());
expect(confirm).toHaveBeenCalledOnce();
expect(nav.push).not.toHaveBeenCalled();
await waitFor(() => expect(nav.push).toHaveBeenCalledWith('/'));
});
});
+16 -7
View File
@@ -31,6 +31,7 @@ import { getPathPrefix, withBasePath } from "@/lib/browser-navigation";
import { useStalwartAdmin } from "@/hooks/use-stalwart-admin";
import { useUpdateStore, selectHasUpdate } from "@/stores/update-store";
import { cn } from "@/lib/utils";
import { interceptLeave, leaveWhenAllowed } from "@/lib/leave-guard";
import { openKeyboardShortcuts, parkAppCommand, requestAppCommand } from "@/lib/app-commands";
import { useSettingsStore } from "@/stores/settings-store";
import { useAccountStore } from "@/stores/account-store";
@@ -127,8 +128,10 @@ export function MvTopBar({
const selectAllInboxes = onSelectAllInboxes ?? (allInboxesReachable
? () => {
if (requestAppCommand("all-inboxes")) return;
parkAppCommand("all-inboxes");
router.push("/");
leaveWhenAllowed(() => {
parkAppCommand("all-inboxes");
router.push("/");
});
}
: undefined);
const { appName, appLogoLightUrl, appLogoDarkUrl } = useConfig();
@@ -213,7 +216,7 @@ export function MvTopBar({
<Menu className="w-5 h-5" />
</button>
)}
<Link href="/" className="flex items-center gap-2 pe-4 min-w-0" aria-label={appName}>
<Link href="/" onClick={(e) => { interceptLeave(e); }} className="flex items-center gap-2 pe-4 min-w-0" aria-label={appName}>
{logoUrl ? <img src={logoUrl} alt="" className="w-8 h-8 object-contain" /> : null}
<span className="text-[22px] leading-none text-foreground/80 truncate">{appName}</span>
</Link>
@@ -288,6 +291,7 @@ export function MvTopBar({
{isStalwartAdmin && (
<a
href={`${getPathPrefix()}/admin`}
onClick={(e) => { interceptLeave(e); }}
className="relative grid place-items-center w-12 h-12 rounded-full text-muted-foreground hover:bg-muted hover:text-foreground transition-colors"
title={t("admin")}
aria-label={hasUpdate ? `${t("admin")}, ${tShell(updateImportant ? "update_important" : "update_available")}` : t("admin")}
@@ -306,6 +310,7 @@ export function MvTopBar({
)}
<Link
href="/settings"
onClick={(e) => { interceptLeave(e); }}
className="grid place-items-center w-12 h-12 rounded-full text-muted-foreground hover:bg-muted hover:text-foreground transition-colors"
title={t("settings")}
aria-label={t("settings")}
@@ -347,8 +352,11 @@ export function MvTopBar({
key={app.id}
href={app.href}
role="menuitem"
onClick={() => {
if (activeAppId) onCloseInlineApp?.();
onClick={(e) => {
// The menu is gone by the time the user answers,
// so the way out is replayed, not re-clicked.
const leave = () => { if (activeAppId) onCloseInlineApp?.(); };
if (!interceptLeave(e, () => { leave(); router.push(app.href); })) leave();
setAppsOpen(false);
}}
className={cn(
@@ -370,8 +378,9 @@ export function MvTopBar({
role="menuitem"
title={page.pluginName}
data-plugin-page-link={`${page.pluginId}/${page.id}`}
onClick={() => {
if (activeAppId) onCloseInlineApp?.();
onClick={(e) => {
const leave = () => { if (activeAppId) onCloseInlineApp?.(); };
if (!interceptLeave(e, () => { leave(); router.push(pluginPageHref(page)); })) leave();
setAppsOpen(false);
}}
className={cn(
+17 -3
View File
@@ -47,8 +47,14 @@ export async function confirmLeave(): Promise<boolean> {
* when an editor has unsaved changes, stops the click, asks, and clicks the
* same element again once the user lets the changes go. Returns true when
* it stopped the click; the handler should then do nothing else.
*
* `replay` leaves in place of the second click, for an element that is gone
* by then (an item of a menu that closes).
*/
export function interceptLeave(e: { preventDefault(): void; currentTarget: EventTarget | null }): boolean {
export function interceptLeave(
e: { preventDefault(): void; currentTarget: EventTarget | null },
replay?: () => void,
): boolean {
if (replaying || !hasUnsavedChanges()) return false;
e.preventDefault();
const target = e.currentTarget instanceof HTMLElement ? e.currentTarget : null;
@@ -56,17 +62,25 @@ export function interceptLeave(e: { preventDefault(): void; currentTarget: Event
asking = true;
void confirmLeave().then(async (leave) => {
asking = false;
if (!leave || !target) return;
if (!leave || (!replay && !target)) return;
// The dialog that asked steps Back off its own history entry as it
// closes; a navigation started before that lands is undone by it.
await new Promise((resolve) => setTimeout(resolve, 0));
await historySettled();
replaying = true;
try { target.click(); } finally { replaying = false; }
try {
if (replay) replay();
else target?.click();
} finally { replaying = false; }
});
return true;
}
/** For a way out that isn't a click (a menu command): leaves now, or once the user lets the changes go. */
export function leaveWhenAllowed(leave: () => void): void {
if (!interceptLeave({ preventDefault() {}, currentTarget: null }, leave)) leave();
}
/** Registers the calling editor's guard while it is mounted. */
export function useLeaveGuard(isDirty: () => boolean, confirm: () => Promise<boolean>): void {
const latest = useRef({ isDirty, confirm });