fix: keep text typed after a clean Mountain View composer move and ask before dropping handed-over edits

This commit is contained in:
Linus Rath
2026-11-03 14:42:41 +01:00
parent 2aab469816
commit 0f799454e4
4 changed files with 98 additions and 14 deletions
@@ -246,6 +246,52 @@ describe('composer flush before an automatic sign-out', () => {
expect(createDraft.mock.calls[0][1]).toBe('Half-written');
});
it('asks before closing a composer opened with stashed edits', async () => {
const onClose = vi.fn();
const requestCloseRef: { current: ((afterClose?: () => void) => void) | null } = { current: null };
render(<EmailComposer initialData={{ ...DRAFT, unsaved: true }} onClose={onClose} requestCloseRef={requestCloseRef} />);
act(() => requestCloseRef.current?.());
expect(await screen.findByRole('alertdialog')).toBeTruthy();
expect(onClose).not.toHaveBeenCalled();
});
it('closes a composer opened with text the server already has without asking', () => {
const onClose = vi.fn();
const requestCloseRef: { current: ((afterClose?: () => void) => void) | null } = { current: null };
render(<EmailComposer initialData={{ ...DRAFT, draftId: 'draft-0', unsaved: false }} onClose={onClose} requestCloseRef={requestCloseRef} />);
act(() => requestCloseRef.current?.());
expect(screen.queryByRole('alertdialog')).toBeNull();
expect(onClose).toHaveBeenCalled();
});
it('autosaves a composer opened with stashed edits', async () => {
render(<EmailComposer initialData={{ ...DRAFT, unsaved: true }} onClose={vi.fn()} />);
await waitFor(() => expect(createDraft).toHaveBeenCalledTimes(1), { timeout: 4000 });
expect(createDraft.mock.calls[0][1]).toBe('Half-written');
});
it('hands nothing back for stashed edits it did not change: the host has them', () => {
const onSaveState = vi.fn();
const view = render(<EmailComposer initialData={{ ...DRAFT, unsaved: true }} onClose={vi.fn()} onSaveState={onSaveState} />);
view.unmount();
expect(onSaveState).not.toHaveBeenCalled();
});
it('hands back the draft id an autosave of stashed edits got', async () => {
const onSaveState = vi.fn();
const view = render(<EmailComposer initialData={{ ...DRAFT, unsaved: true }} onClose={vi.fn()} onSaveState={onSaveState} />);
await act(async () => { await flushComposers(5000); });
view.unmount();
expect(onSaveState).toHaveBeenCalledWith(expect.objectContaining({ draftId: 'draft-1', unsaved: false }));
});
it('waits for a send in flight and saves no draft of the sent message', async () => {
let finishSend!: () => void;
const onSend = vi.fn(() => new Promise<void>((resolve) => { finishSend = resolve; }));
+7 -1
View File
@@ -1779,6 +1779,11 @@ export function EmailComposer({
if (priority !== initialPriorityRef.current) isDirtyRef.current = true;
const initialLabelChoiceRef = useRef(classification.choice);
if (classification.choice !== initialLabelChoiceRef.current) isDirtyRef.current = true;
// Text the host handed in is no edit to hand back, but the server may not have
// it: it still needs the autosave and the question on close, so it counts as dirty.
const hasEditsRef = useRef(false);
hasEditsRef.current = isDirtyRef.current;
if (initialData?.unsaved) isDirtyRef.current = true;
const firstLabelRunRef = useRef(true);
useEffect(() => {
if (firstLabelRunRef.current) {
@@ -1839,7 +1844,8 @@ export function EmailComposer({
useEffect(() => {
return () => {
if (explicitCloseRef.current) return;
if (onSaveState && isDirtyRef.current) {
// Only what the host has no copy of: an edit, or a draft saved under a new id.
if (onSaveState && (hasEditsRef.current || draftIdRef.current !== (initialData?.draftId ?? null))) {
const s = stateRef.current;
onSaveState({
...s,
@@ -21,7 +21,8 @@ vi.mock('@/components/layout/sidebar', () => ({
Sidebar: ({ onCompose }: { onCompose: () => void }) => <button onClick={onCompose}>stub compose</button>,
}));
// The real composer's contract: its state starts from initialData, and it hands
// its edits to onSaveState as it unmounts.
// its edits to onSaveState as it unmounts, only if it has any, through the
// handler of its first render. Text it started with is no edit.
vi.mock('@/components/email/email-composer', async () => {
const { useEffect, useRef, useState } = await import('react');
return {
@@ -30,10 +31,11 @@ vi.mock('@/components/email/email-composer', async () => {
onSaveState: (data: { subject: string }) => void;
}) => {
const [subject, setSubject] = useState(initialData?.subject ?? '');
const initial = useRef(subject);
const latest = useRef(subject);
latest.current = subject;
useEffect(() => () => {
if (latest.current) onSaveState({ subject: latest.current });
if (latest.current !== initial.current) onSaveState({ subject: latest.current });
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
return <input aria-label="stub subject" value={subject} onChange={(e) => setSubject(e.target.value)} />;
@@ -119,6 +121,43 @@ describe('moving the Mountain View composer between layouts', () => {
rerender(<MailApp />);
await waitFor(() => expect(screen.getByLabelText<HTMLInputElement>('stub subject').value).toBe('Still here'));
});
const setWidth = (rerender: (ui: React.ReactElement) => void, width: 'desktop' | 'tablet') => {
device.current = { isMobile: false, isTablet: width === 'tablet', isDesktop: width === 'desktop' };
rerender(<MailApp />);
};
const subject = () => screen.getByLabelText<HTMLInputElement>('stub subject');
it('keeps text typed after the composer moved with nothing in it yet', async () => {
const { rerender } = render(<MailApp />);
fireEvent.click(screen.getByText('stub compose'));
await screen.findByLabelText('stub subject');
// Clean, so this move hands nothing over.
setWidth(rerender, 'tablet');
fireEvent.change(await screen.findByLabelText('stub subject'), { target: { value: 'Typed in the pane' } });
setWidth(rerender, 'desktop');
await waitFor(() => expect(subject().value).toBe('Typed in the pane'));
setWidth(rerender, 'tablet');
await waitFor(() => expect(subject().value).toBe('Typed in the pane'));
});
it('keeps text typed after moving back and forth untouched', async () => {
const { rerender } = render(<MailApp />);
fireEvent.click(screen.getByText('stub compose'));
fireEvent.change(await screen.findByLabelText('stub subject'), { target: { value: 'First' } });
setWidth(rerender, 'tablet');
await waitFor(() => expect(subject().value).toBe('First'));
setWidth(rerender, 'desktop');
await waitFor(() => expect(subject().value).toBe('First'));
fireEvent.change(subject(), { target: { value: 'First and more' } });
setWidth(rerender, 'tablet');
await waitFor(() => expect(subject().value).toBe('First and more'));
});
});
describe('a composer opened from the phone and tablet drawer', () => {
+4 -11
View File
@@ -268,8 +268,6 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
const composerSessionRef = useRef(composerSessionId);
composerSessionRef.current = composerSessionId;
const composerHostRef = useRef('pane');
const composerRemountHostRef = useRef('pane');
const composerRemountSessionRef = useRef(composerSessionId);
const showComposerRef = useRef(showComposer);
showComposerRef.current = showComposer;
// Fields a plugin filled in through onBeforeCompose for the composer about
@@ -4576,10 +4574,6 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
const floatingComposer = mvShell && showComposer && !isEmbedded && !mvInlineReply;
const composerHost = floatingComposer ? 'window' : mvInlineReply ? 'inline' : 'pane';
composerHostRef.current = composerHost;
if (!showComposer || composerRemountSessionRef.current !== composerSessionId) {
composerRemountSessionRef.current = composerSessionId;
composerRemountHostRef.current = composerHost;
}
const hasViewerContent = (showComposer && !floatingComposer) || Boolean(conversationThread)
|| Boolean(selectedEmail && !(mvShell && cursorOnly)) || deepLinkPane !== null;
// A linked message on a phone opens over the list, as one picked there does.
@@ -4945,11 +4939,10 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
});
return;
}
if (showComposerRef.current && composerSessionRef.current === composerSessionId) {
// Still open: it moved to another host, or is the stand-in mounted
// there before the edits arrived. The stand-in's state is out of date.
if (composerHostRef.current === composerRemountHostRef.current) return;
composerRemountHostRef.current = composerHostRef.current;
// Still open in another host: the one mounted there started before
// these edits arrived. The composer calls the handler of its first
// render, so composerHost is the host this instance lived in.
if (showComposerRef.current && composerSessionRef.current === composerSessionId && composerHostRef.current !== composerHost) {
setComposerRemount((n) => n + 1);
}
setPendingDraft(data);