Cap note thread dialog height (task #417)
User reported (in Arabic, with screenshot) that the note Replies dialog was
covering the entire page when a thread had multiple grouped conversations.
The owner/admin grouped-replies view (`GroupedReplies`) had a `max-h-72`
internal scroll, but the outer `DialogContent` had no overall height cap, so
on tall content the dialog grew to fill the viewport.
Changes
- artifacts/tx-os/src/pages/notes.tsx
- `DialogContent` for the thread dialog: add `max-h-[85vh] flex flex-col`.
- Refactored body into three vertical zones (per code-review feedback):
1) static meta + recipient chips (`shrink-0`, always visible),
2) replies-only scroll region (`flex-1 min-h-0 overflow-y-auto`,
`data-testid="note-thread-scroll"`),
3) static composer (`shrink-0`, pinned at the bottom).
- Drop now-redundant `max-h-48` on the recipient ReplyBubble list and
`max-h-72` on `GroupedReplies` — the dedicated scroll zone handles
overflow, avoiding nested double scrollbars.
Tests
- New artifacts/tx-os/tests/notes-thread-dialog.spec.mjs:
- Seeds an owner note with 2 recipients and 30 alternating replies via
SQL, deep-links into `/notes?thread=ID`, asserts the dialog's bounding
box height ≤ 0.9 × viewport height, asserts the inner scroll region
actually overflows (scrollHeight > clientHeight), and asserts the
composer remains visible.
- Verified the existing notes-folders.spec.mjs (2 tests) still pass.
- `tsc --noEmit` clean.
Out of scope (left as-is): visual redesign, reply send/archive logic,
Inbox/Sent thread list.
This commit is contained in:
@@ -1179,14 +1179,14 @@ function ThreadDialog({
|
||||
<Loading />
|
||||
) : (
|
||||
<>
|
||||
{/* Scrollable middle: header info + recipients + replies. The
|
||||
outer DialogContent caps height at 85vh and the composer
|
||||
below stays pinned, so long threads scroll inside this
|
||||
region instead of pushing the dialog past the viewport. */}
|
||||
<div
|
||||
className="flex-1 min-h-0 overflow-y-auto -mx-1 px-1 flex flex-col gap-3"
|
||||
data-testid="note-thread-scroll"
|
||||
>
|
||||
{/* Three vertical zones inside the 85vh-capped DialogContent:
|
||||
1) static meta + recipient chips (always visible),
|
||||
2) scrollable replies-only region,
|
||||
3) static composer (pinned below).
|
||||
This keeps the From line, recipient pills, and the reply
|
||||
box on screen while long conversations scroll inside the
|
||||
middle zone. */}
|
||||
<div className="shrink-0 flex flex-col gap-2" data-testid="note-thread-meta">
|
||||
<div className="text-[11px] text-muted-foreground flex items-center gap-1">
|
||||
<Mail size={12} />
|
||||
{t("notes.from", "From:")} {userDisplayName(thread.sender, lang)}
|
||||
@@ -1218,17 +1218,22 @@ function ThreadDialog({
|
||||
</div>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
|
||||
<div className="border-t pt-3">
|
||||
<div className="text-xs text-muted-foreground mb-1.5 flex items-center gap-1">
|
||||
<MessageCircle size={12} />
|
||||
{t("notes.replies", "Replies")}
|
||||
{thread.replies.length > 0 && (
|
||||
<span className="text-foreground/70">
|
||||
({thread.replies.length})
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
<div className="border-t pt-3 flex flex-col flex-1 min-h-0">
|
||||
<div className="text-xs text-muted-foreground mb-1.5 flex items-center gap-1 shrink-0">
|
||||
<MessageCircle size={12} />
|
||||
{t("notes.replies", "Replies")}
|
||||
{thread.replies.length > 0 && (
|
||||
<span className="text-foreground/70">
|
||||
({thread.replies.length})
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
<div
|
||||
className="flex-1 min-h-0 overflow-y-auto -mx-1 px-1"
|
||||
data-testid="note-thread-scroll"
|
||||
>
|
||||
{thread.replies.length === 0 ? (
|
||||
<div className="text-xs text-muted-foreground italic">
|
||||
{t("notes.noRepliesYet", "No replies yet")}
|
||||
|
||||
@@ -142,14 +142,28 @@ test("thread dialog does not exceed viewport height when conversation is long",
|
||||
|
||||
// The internal scroll container should actually overflow (proving content
|
||||
// is being scrolled, not truncated).
|
||||
const scrollInfo = await page
|
||||
.getByTestId("note-thread-scroll")
|
||||
.evaluate((el) => ({
|
||||
scrollHeight: el.scrollHeight,
|
||||
clientHeight: el.clientHeight,
|
||||
}));
|
||||
const scroller = page.getByTestId("note-thread-scroll");
|
||||
const scrollInfo = await scroller.evaluate((el) => ({
|
||||
scrollHeight: el.scrollHeight,
|
||||
clientHeight: el.clientHeight,
|
||||
}));
|
||||
expect(scrollInfo.scrollHeight).toBeGreaterThan(scrollInfo.clientHeight);
|
||||
|
||||
// Composer remains visible and usable.
|
||||
await expect(page.getByTestId("thread-reply-input")).toBeVisible();
|
||||
// Recipient chips and composer must stay pinned while replies scroll —
|
||||
// both should remain in the viewport even after we scroll the replies
|
||||
// region all the way down.
|
||||
const r1Chip = dialog.getByTestId(`thread-recipient-${r1.id}`);
|
||||
const r2Chip = dialog.getByTestId(`thread-recipient-${r2.id}`);
|
||||
const composer = page.getByTestId("thread-reply-input");
|
||||
await expect(r1Chip).toBeInViewport();
|
||||
await expect(r2Chip).toBeInViewport();
|
||||
await expect(composer).toBeInViewport();
|
||||
|
||||
await scroller.evaluate((el) => {
|
||||
el.scrollTop = el.scrollHeight;
|
||||
});
|
||||
|
||||
await expect(r1Chip).toBeInViewport();
|
||||
await expect(r2Chip).toBeInViewport();
|
||||
await expect(composer).toBeInViewport();
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user