From d03d4a0b780c1eee35d22c341f42d050c3a7217e Mon Sep 17 00:00:00 2001
From: Linus Rath <139418639+rathlinus@users.noreply.github.com>
Date: Wed, 4 Nov 2026 12:33:28 +0100
Subject: [PATCH] fix: show whole rows of attachments and keep uploading files
 in their place

The attachment order test now expects uploading files where they were
added: moved to the front, a big upload jumped out of sight once done.
---
 .../composer-attachment-list-scroll.test.tsx        | 10 ++++++++++
 components/email/email-composer.tsx                 | 13 +++++++------
 lib/compose/__tests__/attachment-order.test.ts      |  6 +++---
 lib/compose/attachment-order.ts                     |  9 +++++----
 4 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/components/email/__tests__/composer-attachment-list-scroll.test.tsx b/components/email/__tests__/composer-attachment-list-scroll.test.tsx
index 04bc59023..c4fe570c2 100644
--- a/components/email/__tests__/composer-attachment-list-scroll.test.tsx
+++ b/components/email/__tests__/composer-attachment-list-scroll.test.tsx
@@ -259,6 +259,16 @@ describe('attachment list', () => {
     expect(row.className).toContain('max-h-[');
   });
 
+  // compose-p2-11: 5.25rem showed two rows of 2rem chips and half a third.
+  it('is as high as whole rows of chips, so none is cut in half', () => {
+    render(<EmailComposer initialData={DRAFT} />);
+
+    const heights = [...chipRow().className.matchAll(/max-h-\[([\d.]+)rem\]/g)].map((m) => Number(m[1]));
+    expect(heights.length).toBeGreaterThan(0);
+    // n rows of 2rem chips with 0.5rem gaps: 2.5n - 0.5.
+    for (const height of heights) expect(Number.isInteger((height + 0.5) / 2.5)).toBe(true);
+  });
+
   it('says how many files and how much they weigh', () => {
     render(<EmailComposer initialData={DRAFT} />);
 
diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx
index 25849713f..d0d922b13 100644
--- a/components/email/email-composer.tsx
+++ b/components/email/email-composer.tsx
@@ -5679,12 +5679,13 @@ export function EmailComposer({
               </p>
             )}
             <div
-              // Every file shows, wrapping. Past about two rows the list
-              // scrolls by itself: it sits outside the body's scroller and
-              // can't shrink, and a few dozen files pushed Send off the
-              // bottom. Files still uploading or failed come first, so their
-              // progress and errors are never out of sight.
-              className="flex flex-wrap gap-2 max-h-[5.25rem] overflow-y-auto overscroll-contain"
+              // Every file shows, wrapping. Past four rows (two in a short
+              // window) the list scrolls by itself: it sits outside the
+              // body's scroller and can't shrink, and a few dozen files pushed
+              // Send off the bottom. The height is whole rows of 2rem chips
+              // with 0.5rem gaps, so no chip is cut in half (compose-p2-11);
+              // the line above says how many there are. Failed files first.
+              className="flex flex-wrap gap-2 max-h-[4.5rem] [@media(min-height:700px)]:max-h-[9.5rem] overflow-y-auto overscroll-contain [scrollbar-gutter:stable]"
               data-testid="composer-attachments"
             >
               {attachmentsForDisplay(attachments).map(({ att, index }) => {
diff --git a/lib/compose/__tests__/attachment-order.test.ts b/lib/compose/__tests__/attachment-order.test.ts
index d5337caea..de1376bed 100644
--- a/lib/compose/__tests__/attachment-order.test.ts
+++ b/lib/compose/__tests__/attachment-order.test.ts
@@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest';
 import { attachmentsForDisplay } from '../attachment-order';
 
 describe('the order of the composer attachment chips', () => {
-  it('puts failed, then uploading files first, and keeps the rest in the order added', () => {
+  it('puts failed files first, and keeps the rest, uploading or not, in the order added', () => {
     const files = [
       { name: 'a.pdf' },
       { name: 'b.pdf', uploading: true },
@@ -11,8 +11,8 @@ describe('the order of the composer attachment chips', () => {
       { name: 'broken.png', error: 'failed' },
     ];
     const shown = attachmentsForDisplay(files);
-    expect(shown.map(({ att }) => att.name)).toEqual(['broken.png', 'b.pdf', 'big.zip', 'a.pdf', 'c.pdf']);
+    expect(shown.map(({ att }) => att.name)).toEqual(['broken.png', 'a.pdf', 'b.pdf', 'c.pdf', 'big.zip']);
     // Each keeps its place in the composer's list, for Remove.
-    expect(shown.map(({ index }) => index)).toEqual([4, 1, 3, 0, 2]);
+    expect(shown.map(({ index }) => index)).toEqual([4, 0, 1, 2, 3]);
   });
 });
diff --git a/lib/compose/attachment-order.ts b/lib/compose/attachment-order.ts
index 158d340d6..594011a40 100644
--- a/lib/compose/attachment-order.ts
+++ b/lib/compose/attachment-order.ts
@@ -1,13 +1,14 @@
 /**
  * The composer's attachment chips in the order they're shown: failed first,
- * then still uploading, then the rest, each group in the order added. The
- * list scrolls past about two rows, so the files that need an eye on them
- * are the ones always in view. `index` is the place in the composer's list.
+ * then the rest in the order added. A file still uploading keeps its place:
+ * moved to the front, it jumped to the end once done and went out of sight
+ * past the rows on screen (compose-p2-11). `index` is the place in the
+ * composer's list.
  */
 export function attachmentsForDisplay<T extends { error?: unknown; uploading?: boolean }>(
   attachments: readonly T[],
 ): Array<{ att: T; index: number }> {
-  const rank = (att: T) => (att.error ? 0 : att.uploading ? 1 : 2);
+  const rank = (att: T) => (att.error ? 0 : 1);
   return attachments
     .map((att, index) => ({ att, index }))
     .sort((a, b) => rank(a.att) - rank(b.att) || a.index - b.index);