fix: let j and k go on from the last opened message after Escape

components/mail/__tests__/mail-app-conversation-pane.test.tsx

@@ -134,6 +134,7 @@ afterEach(() => {
   vi.unstubAllGlobals();
 });
 
+const selectedId = () => useEmailStore.getState().selectedEmail?.id ?? null;
 const press = (key: string) => act(() => { fireEvent.keyDown(window, { key }); });
 
 describe('The reading pane in the default layout', () => {
@@ -172,3 +173,46 @@ describe('The reading pane in the default layout', () => {
     expect(useEmailStore.getState().expandedThreadIds.size).toBe(0);
   });
 });
+
+describe('j and k after Escape', () => {
+  beforeEach(() => {
+    useSettingsStore.setState({ interfaceLayout: 'default', mailLayout: 'split' } as never);
+    useEmailStore.setState({ listRows: null } as never);
+  });
+
+  it('j opens the message after the one that was open, not the first', async () => {
+    render(<MailApp />);
+    await screen.findByTestId('reader', {}, { timeout: 5000 });
+    await press('j');
+    await waitFor(() => expect(selectedId()).toBe('m2'));
+    await press('Escape');
+    await waitFor(() => expect(selectedId()).toBeNull());
+    await press('j');
+    await waitFor(() => expect(selectedId()).toBe('m3'));
+  });
+
+  it('k opens the message before it', async () => {
+    useEmailStore.setState({ selectedEmail: inbox()[2] } as never);
+    render(<MailApp />);
+    await screen.findByTestId('reader', {}, { timeout: 5000 });
+    await press('Escape');
+    await waitFor(() => expect(selectedId()).toBeNull());
+    await press('k');
+    await waitFor(() => expect(selectedId()).toBe('m2'));
+  });
+
+  it('steps by conversation, in the order of the list', async () => {
+    useEmailStore.setState({
+      listRows: [{ key: 'm1', members: ['m1', 'm3'] }, { key: 'm2', members: ['m2'] }],
+    } as never);
+    render(<MailApp />);
+    await screen.findByTestId('reader', {}, { timeout: 5000 });
+    await press('j');
+    await waitFor(() => expect(selectedId()).toBe('m2'));
+    await press('j');
+    await act(async () => { await new Promise((resolve) => setTimeout(resolve, 50)); });
+    expect(selectedId()).toBe('m2');
+    await press('k');
+    await waitFor(() => expect(selectedId()).toBe('m1'));
+  });
+});

components/mail/mail-app.tsx

@@ -80,6 +80,7 @@ import { useMailStatusAnnouncements } from "@/hooks/use-mail-status-announcement
 import { cn, formatDateTime, getMailboxPath } from "@/lib/utils";
 import { localizeMailboxName } from "@/lib/mailbox-label";
 import { KEYWORD_PREFIX, KEYWORD_PREFIX_LEGACY, emailKeyFor, groupEmailsByThread, threadKeyFor } from "@/lib/thread-utils";
+import { stepListEmail } from "@/lib/list-step";
 import { resolveThreadRoute } from "@/lib/thread-routing";
 import {
   ErrorBoundary,
@@ -978,23 +979,33 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
     );
   };
 
+  // The open message's list key, kept after the reader closes.
+  const lastOpenedKeyRef = useRef<string | null>(null);
+  useEffect(() => {
+    if (selectedEmail) lastOpenedKeyRef.current = emailKeyFor(selectedEmail);
+  }, [selectedEmail]);
   // Keyboard shortcuts handlers
+  // j / k go on from the open message, or from the last one opened once the
+  // reader is closed (Escape), not from the top. With the conversation in the
+  // reading pane they step by conversation, as the list's rows do.
+  const stepFrom = (direction: 1 | -1) => stepListEmail(
+    activeEmails,
+    selectedEmail ? emailKeyFor(selectedEmail) : lastOpenedKeyRef.current,
+    direction,
+    paneConversationView ? useEmailStore.getState().listRows : null,
+  );
   const keyboardHandlers = useMemo(() => ({
     onNextEmail: () => {
-      if (activeEmails.length === 0) return;
-      const currentIndex = selectedEmail ? activeEmails.findIndex(e => emailKeyFor(e) === emailKeyFor(selectedEmail)) : -1;
-      const nextIndex = currentIndex < activeEmails.length - 1 ? currentIndex + 1 : currentIndex;
-      if (nextIndex >= 0 && nextIndex < activeEmails.length) {
-        void handleEmailSelect(activeEmails[nextIndex]);
+      const next = stepFrom(1);
+      if (next) {
+        void handleEmailSelect(next);
         if (mvShell) setCursorOnly(true);
       }
     },
     onPreviousEmail: () => {
-      if (activeEmails.length === 0) return;
-      const currentIndex = selectedEmail ? activeEmails.findIndex(e => emailKeyFor(e) === emailKeyFor(selectedEmail)) : activeEmails.length;
-      const prevIndex = currentIndex > 0 ? currentIndex - 1 : 0;
-      if (prevIndex >= 0 && prevIndex < activeEmails.length) {
-        void handleEmailSelect(activeEmails[prevIndex]);
+      const previous = stepFrom(-1);
+      if (previous) {
+        void handleEmailSelect(previous);
         if (mvShell) setCursorOnly(true);
       }
     },

lib/__tests__/list-step.test.ts

@@ -0,0 +1,34 @@
+import { describe, it, expect } from 'vitest';
+import type { Email } from '@/lib/jmap/types';
+import { stepListEmail } from '../list-step';
+
+const mail = (id: string) => ({ id, threadId: `t-${id}` }) as Email;
+const emails = [mail('a'), mail('b'), mail('c'), mail('d')];
+const ids = (e: Email | null) => e?.id ?? null;
+
+describe('stepListEmail', () => {
+  it('goes on from the given message', () => {
+    expect(ids(stepListEmail(emails, 'c', 1))).toBe('d');
+    expect(ids(stepListEmail(emails, 'c', -1))).toBe('b');
+  });
+  it('stays put at either end', () => {
+    expect(ids(stepListEmail(emails, 'd', 1))).toBe('d');
+    expect(ids(stepListEmail(emails, 'a', -1))).toBe('a');
+  });
+  it('starts at the top for j and the bottom for k without a place', () => {
+    expect(ids(stepListEmail(emails, null, 1))).toBe('a');
+    expect(ids(stepListEmail(emails, null, -1))).toBe('d');
+    expect(ids(stepListEmail(emails, 'gone', 1))).toBe('a');
+  });
+  it('counts a conversation once and opens it at its row', () => {
+    // b's row stands for b and a (its older message); then d, then c.
+    const rows = [{ key: 'b', members: ['b', 'a'] }, { key: 'd', members: ['d'] }, { key: 'c', members: ['c'] }];
+    expect(ids(stepListEmail(emails, 'a', 1, rows))).toBe('d');
+    expect(ids(stepListEmail(emails, 'd', 1, rows))).toBe('c');
+    expect(ids(stepListEmail(emails, 'd', -1, rows))).toBe('b');
+    expect(ids(stepListEmail(emails, null, -1, rows))).toBe('c');
+  });
+  it('is empty for an empty list', () => {
+    expect(stepListEmail([], 'a', 1)).toBeNull();
+  });
+});

lib/list-step.ts

@@ -0,0 +1,39 @@
+import type { Email } from '@/lib/jmap/types';
+import { emailKeyFor } from '@/lib/thread-utils';
+
+/** One row of the message list: its own key and the keys it stands for. */
+export interface ListStepRow {
+  key: string;
+  members: string[];
+}
+
+/**
+ * The message j / k open: the one after (`direction` 1) or before (-1) the
+ * message `fromKey` names, staying put at either end. `fromKey` is the open
+ * message, or the last one opened when the reader was closed, so j after
+ * Escape goes on from there. Without a place in the list, j starts at the
+ * top and k at the bottom.
+ *
+ * With `rows` (the list's rows in screen order) a conversation counts once,
+ * and opens at its newest message, as a click on its row does: the reading
+ * pane shows the rest of it.
+ */
+export function stepListEmail(
+  emails: readonly Email[],
+  fromKey: string | null | undefined,
+  direction: 1 | -1,
+  rows?: readonly ListStepRow[] | null,
+): Email | null {
+  const byKey = new Map(emails.map((e) => [emailKeyFor(e), e] as const));
+  const order: readonly ListStepRow[] = rows && rows.length > 0
+    ? rows.filter((row) => byKey.has(row.key))
+    : emails.map((e) => ({ key: emailKeyFor(e), members: [] }));
+  if (order.length === 0) return null;
+  const at = fromKey
+    ? order.findIndex((row) => row.key === fromKey || row.members.includes(fromKey))
+    : -1;
+  const target = at < 0
+    ? (direction === 1 ? 0 : order.length - 1)
+    : Math.min(order.length - 1, Math.max(0, at + direction));
+  return byKey.get(order[target].key) ?? null;
+}