fix: keep the composer body reachable in short reading panes #1114

2026-11-02 08:15:55 +00:00 · 2026-09-27 21:54:02 +02:00
parent dec4d644d0
commit 0b3e47e3d2
2 changed files with 318 additions and 5 deletions
@@ -0,0 +1,276 @@
+import { render, screen, act } from '@testing-library/react';
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import React from 'react';
+import { EmailComposer } from '../email-composer';
+
+// ─── Heavy component mocks (mirrors empty-subject-warning.test.tsx) ────────────
+
+vi.mock('@/components/email/rich-text-editor', () => ({
+  RichTextEditor: () => React.createElement('div', { 'data-testid': 'rich-text-editor' }),
+}));
+
+vi.mock('@/components/plugins/plugin-slot', () => ({ PluginSlot: () => null }));
+vi.mock('@/components/identity/sub-address-helper', () => ({ SubAddressHelper: () => null }));
+vi.mock('@/components/templates/template-picker', () => ({ TemplatePicker: () => null }));
+vi.mock('@/components/templates/template-form', () => ({ TemplateForm: () => null }));
+vi.mock('@/components/files/file-preview-modal', () => ({ FilePreviewModal: () => null }));
+vi.mock('@/hooks/use-focus-trap', () => ({
+  useFocusTrap: () => ({ current: null }),
+}));
+vi.mock('@/hooks/use-pro-multi-account-identities', () => ({
+  useProMultiAccountIdentities: () => ({ enabled: false, groups: [], allIdentities: [] }),
+  stripCrossAccountIdentityPrefix: (id: string) => ({ localAccountId: null, rawId: id }),
+}));
+
+// ─── Store mocks ──────────────────────────────────────────────────────────────
+
+vi.mock('@/stores/auth-store', () => {
+  const state = {
+    client: null,
+    identities: [],
+    primaryIdentity: null,
+    isAuthenticated: false,
+    isDemoMode: false,
+    activeAccountId: null,
+    connectionLost: false,
+    getClientForAccount: () => undefined,
+    getAllConnectedClients: () => new Map(),
+    syncIdentities: () => {},
+    refreshIdentities: async () => {},
+  };
+  const hook = (sel?: (s: typeof state) => unknown) =>
+    typeof sel === 'function' ? sel(state) : state;
+  hook.getState = () => state;
+  hook.setState = (p: Partial<typeof state>) => Object.assign(state, p);
+  return { useAuthStore: hook };
+});
+
+vi.mock('@/stores/identity-store', () => {
+  const state = {
+    identities: [{ id: 'id-me', email: 'me@example.com', name: 'Me' }],
+    defaultIdentityId: 'id-me',
+  };
+  const hook = (sel?: (s: typeof state) => unknown) =>
+    typeof sel === 'function' ? sel(state) : state;
+  hook.getState = () => state;
+  hook.setState = (p: Partial<typeof state>) => Object.assign(state, p);
+  return { useIdentityStore: hook };
+});
+
+vi.mock('@/stores/account-store', () => {
+  const state = { accounts: [], getAccountById: () => undefined };
+  const hook = (sel?: (s: typeof state) => unknown) =>
+    typeof sel === 'function' ? sel(state) : state;
+  hook.getState = () => state;
+  hook.setState = (p: Partial<typeof state>) => Object.assign(state, p);
+  return { useAccountStore: hook };
+});
+
+vi.mock('@/stores/email-store', () => {
+  const state = {
+    draftSaveEnabled: false,
+    sendRawEmail: async () => ({ sent: true }),
+  };
+  const hook = (sel?: (s: typeof state) => unknown) =>
+    typeof sel === 'function' ? sel(state) : state;
+  hook.getState = () => state;
+  hook.setState = (p: Partial<typeof state>) => Object.assign(state, p);
+  return { useEmailStore: hook };
+});
+
+const updateSetting = vi.fn();
+
+vi.mock('@/stores/settings-store', () => {
+  const state = {
+    timeFormat: '24h',
+    plainTextMode: false,
+    subAddressDelimiter: '+',
+    autoSelectReplyIdentity: true,
+    attachmentReminderEnabled: false,
+    attachmentReminderKeywords: [],
+    emptySubjectWarningEnabled: true,
+    sendDelaySeconds: 0,
+    signaturePosition: 'above_quote',
+    signatureSeparatorEnabled: false,
+    requestReadReceiptDefault: false,
+    addTrustedSender: () => {},
+    trustedSendersAddressBook: null,
+    updateSetting: (...args: unknown[]) => updateSetting(...args),
+  };
+  const hook = (sel?: (s: typeof state) => unknown) =>
+    typeof sel === 'function' ? sel(state) : state;
+  hook.getState = () => state;
+  hook.setState = (p: Partial<typeof state>) => Object.assign(state, p);
+  return { useSettingsStore: hook };
+});
+
+vi.mock('@/stores/contact-store', () => {
+  const state = {
+    contacts: [],
+    getAutocomplete: async () => [],
+    addToTrustedSendersBook: async () => {},
+  };
+  const hook = (sel?: (s: typeof state) => unknown) =>
+    typeof sel === 'function' ? sel(state) : state;
+  hook.getState = () => state;
+  hook.setState = (p: Partial<typeof state>) => Object.assign(state, p);
+  return { useContactStore: hook };
+});
+
+vi.mock('@/stores/template-store', () => {
+  const state = { templates: [], addTemplate: async () => {} };
+  const hook = (sel?: (s: typeof state) => unknown) =>
+    typeof sel === 'function' ? sel(state) : state;
+  hook.getState = () => state;
+  hook.setState = (p: Partial<typeof state>) => Object.assign(state, p);
+  return { useTemplateStore: hook };
+});
+
+// ─── Misc dependency mocks ────────────────────────────────────────────────────
+
+vi.mock('@/stores/toast-store', () => ({
+  toast: { info: () => {}, error: () => {}, success: () => {} },
+}));
+
+vi.mock('@/lib/plugin-hooks', () => ({
+  emailHooks: {
+    onComposerOpen: { call: async () => [] },
+    onRecipientChange: { call: async () => [] },
+    getRecipientSuggestions: { call: async () => [] },
+    onRecipientChipsChange: { transform: async (chips: unknown) => chips },
+    onDraftChange: { emit: () => {} },
+    onBeforeEmailSend: { intercept: async () => true },
+    onComposeSend: { intercept: async () => true },
+    onTransformOutgoingEmail: { transform: async (email: unknown) => email },
+  },
+  contactHooks: {
+    search: { call: async () => [] },
+    onProvideRecipientSuggestions: { transform: async (initial: unknown) => initial },
+  },
+}));
+
+vi.mock('@/lib/email-sanitization', () => ({
+  sanitizeSignatureHtml: (v: string) => v,
+  sanitizeEmailHtml: (v: string) => v,
+  parseHtmlSafely: (html: string) => new DOMParser().parseFromString(html, 'text/html'),
+}));
+
+vi.mock('@/lib/email-threading', () => ({
+  computeReplyThreadingHeaders: () => ({ inReplyTo: [], references: [] }),
+}));
+vi.mock('@/lib/signature-utils', () => ({
+  appendPlainTextSignature: (body: string) => body,
+  getPlainTextSignature: () => '',
+}));
+vi.mock('@/lib/sub-addressing', () => ({ generateSubAddress: () => '' }));
+vi.mock('@/lib/debug', () => ({ debug: { log: () => {}, warn: () => {}, error: () => {} } }));
+vi.mock('@/components/email/quoted-html', () => ({
+  buildQuotedHtmlBlock: () => '',
+  serializeEditorContent: () => '',
+}));
+vi.mock('@/lib/template-utils', () => ({ substitutePlaceholders: (s: string) => s }));
+
+// ─── Layout stubs ───────────────────────────────────────────────────────────────
+
+// jsdom has no layout and no ResizeObserver. Record what each observer
+// watches so a test can fire the composer's own observer after setting the
+// heights it reads.
+const observers: { callback: ResizeObserverCallback; targets: Element[] }[] = [];
+
+class ResizeObserverStub {
+  private entry: { callback: ResizeObserverCallback; targets: Element[] };
+  constructor(callback: ResizeObserverCallback) {
+    this.entry = { callback, targets: [] };
+    observers.push(this.entry);
+  }
+  observe(target: Element) { this.entry.targets.push(target); }
+  unobserve() {}
+  disconnect() { this.entry.targets = []; }
+}
+
+const DRAFT = {
+  to: 'bob@example.com',
+  cc: '',
+  bcc: '',
+  subject: 'Hello',
+  body: '<p>Hello there</p>',
+  showCc: false,
+  showBcc: false,
+  selectedIdentityId: 'id-me',
+  subAddressTag: '',
+  mode: 'compose' as const,
+  draftId: null,
+};
+
+/** Body wrapper > editor container > editor; the wrapper's parent holds the fields too. */
+function layout() {
+  const body = screen.getByTestId('rich-text-editor').parentElement!.parentElement!;
+  const main = body.parentElement!;
+  const fields = main.firstElementChild as HTMLElement;
+  return { main, fields, body };
+}
+
+function resize(main: HTMLElement, fields: HTMLElement, mainHeight: number, fieldsHeight: number) {
+  Object.defineProperty(main, 'clientHeight', { configurable: true, value: mainHeight });
+  Object.defineProperty(fields, 'offsetHeight', { configurable: true, value: fieldsHeight });
+  act(() => {
+    for (const o of observers) {
+      if (o.targets.includes(main)) o.callback([], {} as ResizeObserver);
+    }
+  });
+}
+
+// ─── Tests ──────────────────────────────────────────────────────────────────────────
+
+describe('composer in a short pane (#1114)', () => {
+  beforeEach(() => {
+    observers.length = 0;
+    vi.stubGlobal('ResizeObserver', ResizeObserverStub);
+  });
+
+  afterEach(() => {
+    vi.unstubAllGlobals();
+  });
+
+  it('pins the fields above a body scroller while the pane has room', () => {
+    render(<EmailComposer initialData={DRAFT} />);
+    const { main, fields, body } = layout();
+
+    resize(main, fields, 500, 160);
+
+    expect(body.className).toContain('overflow-auto');
+    expect(main.className).not.toContain('overflow-y-auto');
+  });
+
+  it('scrolls fields and body together once the body would be squeezed', () => {
+    render(<EmailComposer initialData={DRAFT} />);
+    const { main, fields, body } = layout();
+
+    // 219px pane, 160px of fields: the body scroller would be 59px, less
+    // than the formatting toolbar plus one line of text.
+    resize(main, fields, 219, 160);
+
+    expect(main.className).toContain('overflow-y-auto');
+    expect(body.className).not.toContain('overflow-auto');
+  });
+
+  it('pins the fields again when the pane grows', () => {
+    render(<EmailComposer initialData={DRAFT} />);
+    const { main, fields, body } = layout();
+
+    resize(main, fields, 219, 160);
+    resize(main, fields, 540, 160);
+
+    expect(body.className).toContain('overflow-auto');
+    expect(main.className).not.toContain('overflow-y-auto');
+  });
+
+  it('keeps the layout while the pane is hidden', () => {
+    render(<EmailComposer initialData={DRAFT} />);
+    const { main, fields, body } = layout();
+
+    resize(main, fields, 0, 0);
+
+    expect(body.className).toContain('overflow-auto');
+  });
+});
@@ -276,6 +276,11 @@ function getDefaultScheduleValue(): string {
  return formatLocalDateTimeInput(tomorrowAtEight);
 }

+// Height the body area (formatting toolbar included) must keep below the
+// pinned From/To/Cc/Bcc/Subject fields. Below it the fields scroll away with
+// the body instead (#1114).
+const PINNED_FIELDS_MIN_BODY_HEIGHT = 200;
+
 export function EmailComposer({
  onSend,
  onScheduledSendCreated,
@@ -1198,6 +1203,8 @@ export function EmailComposer({
  const subjectInputRef = useRef<HTMLInputElement>(null);
  const bodyRef = useRef<HTMLTextAreaElement>(null);
  const editorContainerRef = useRef<HTMLDivElement>(null);
+  const composerMainRef = useRef<HTMLDivElement>(null);
+  const composerFieldsRef = useRef<HTMLDivElement>(null);
  const toDropdownRef = useRef<HTMLDivElement>(null);
  const ccDropdownRef = useRef<HTMLDivElement>(null);
  const bccDropdownRef = useRef<HTMLDivElement>(null);
@@ -1215,6 +1222,29 @@ export function EmailComposer({
    }
  }, [plainTextMode]);

+  // The fields sit pinned above the body's own scroll container only while
+  // the pane leaves the body enough height under them. In a short pane
+  // (reading pane at the bottom on a laptop, a phone in landscape) the body
+  // scroller shrank to the height of its sticky formatting toolbar, which
+  // then covered the text completely (#1114). Without the room, fields and
+  // body scroll together and the toolbar pins to the top of that instead.
+  const [pinFields, setPinFields] = useState(true);
+  useEffect(() => {
+    const main = composerMainRef.current;
+    const fields = composerFieldsRef.current;
+    if (!main || !fields || typeof ResizeObserver === 'undefined') return;
+    const update = () => {
+      // Hidden or not laid out yet: keep the current layout.
+      if (main.clientHeight === 0) return;
+      setPinFields(main.clientHeight - fields.offsetHeight >= PINNED_FIELDS_MIN_BODY_HEIGHT);
+    };
+    const observer = new ResizeObserver(update);
+    observer.observe(main);
+    observer.observe(fields);
+    update();
+    return () => observer.disconnect();
+  }, []);
+
  // Switch this one message between rich text and plain text (#1022). The
  // body is converted in place and the embedded signature survives in the
  // target format (compose-format.ts). Formatting is dropped going to plain
@@ -2755,10 +2785,16 @@ export function EmailComposer({
        )}
      </div>

-      {/* Fields section - outside the scroll container so From/To/Cc/Bcc/
-          Subject stay reachable while scrolling long bodies, matching the
-          pinned formatting toolbar (see rich-text-editor.tsx). */}
-      <div className="shrink-0 space-y-0 border-b">
+      {/* Fields + body. While pinFields holds, the fields sit outside the
+          body's scroll container so From/To/Cc/Bcc/Subject stay reachable
+          while scrolling long bodies, matching the pinned formatting toolbar
+          (see rich-text-editor.tsx). Otherwise this wrapper is the scroll
+          container and the fields scroll away with the body. */}
+      <div
+        ref={composerMainRef}
+        className={cn("flex-1 min-h-0 flex flex-col", !pinFields && "overflow-y-auto")}
+      >
+      <div ref={composerFieldsRef} className="shrink-0 space-y-0 border-b">
          {/* From field */}
          <div className="flex items-center gap-2 px-4 py-2.5 border-b border-border/50">
            <span className="text-sm text-muted-foreground w-12 md:w-16 shrink-0">{t('from')}:</span>
@@ -3044,7 +3080,7 @@ export function EmailComposer({
          </div>
      </div>

-      <div className="flex-1 min-h-0 overflow-auto">
+      <div className={cn("flex-1", pinFields && "min-h-0 overflow-auto")}>
        {/* Body */}
        {plainTextMode ? (
          <textarea
@@ -3094,6 +3130,7 @@ export function EmailComposer({
            dangerouslySetInnerHTML={{ __html: `${signatureSeparatorEnabled ? '<div>-- </div>' : ''}${composerSignatureHtml}` }}
          />
        ) : null}
+      </div>
      </div>

        {/* Attachments */}