mirror of
https://github.com/block/buzz.git
synced 2026-10-27 04:30:52 +00:00
fix(desktop): keep drafts out of the Inbox All view (#3217)
## Summary Drafts were showing up in the Home Inbox **All** view, mixed in with messages and reminders (reported in `#buzz-bugs`). Drafts are private composer state, not inbox activity — they now appear only under the dedicated **Drafts** filter. ## Changes - **`inboxListRows.ts`** — drop the `draft` row variant from `buildInboxListRows`; the mixed view builds only `inbox` + `reminder` rows. - **`InboxListPane.tsx`** — remove the draft branch of the All-view render path; `PersonalItemRow` now renders reminders only. - **`useHomePersonalInbox.ts`** — stop enabling draft selection (and its root-status relay probing) for the mixed view; draft selection is scoped to the Drafts filter. - Drafts filter behavior is unchanged: the filter badge count, `DraftsPanel` list, and `DraftDetailPane` all still work. ## Testing - `pnpm test` (desktop unit suite): 3697 passed, 0 failed. - `pnpm exec biome check src/features/home tests`: clean. - Updated `inboxListRows.test.mjs` for the two-variant row model. - Updated the e2e test (`channels.spec.ts`) to assert All never lists drafts and that the draft is still reachable under the Drafts filter. - Added `drafts-all-fix-screenshots.spec.ts` capturing both states (screenshots below). ### All view — draft is gone, messages/reminders unaffected  ### Drafts filter — the draft is still listed and editable  Signed-off-by: Thomas Petersen <thomasp@squareup.com>
This commit is contained in:
@@ -103,6 +103,7 @@ export default defineConfig({
|
||||
"**/project-pr-review.spec.ts",
|
||||
"**/persona-model-combobox-screenshots.spec.ts",
|
||||
"**/drafts-screenshots.spec.ts",
|
||||
"**/drafts-all-fix-screenshots.spec.ts",
|
||||
"**/inbox-refactor-screenshots.spec.ts",
|
||||
"**/buzz-theme-screenshots.spec.ts",
|
||||
"**/channel-sort.spec.ts",
|
||||
|
||||
@@ -17,16 +17,6 @@ function inboxItem(
|
||||
};
|
||||
}
|
||||
|
||||
function draftItem(key, updatedAt, rootStatus = "available") {
|
||||
return {
|
||||
entry: {
|
||||
key,
|
||||
draft: { createdAt: updatedAt, updatedAt },
|
||||
},
|
||||
rootStatus,
|
||||
};
|
||||
}
|
||||
|
||||
function reminder(
|
||||
id,
|
||||
createdAt,
|
||||
@@ -46,20 +36,18 @@ function reminder(
|
||||
|
||||
test("Inbox All combines rows in latest-first order", () => {
|
||||
const rows = buildInboxListRows({
|
||||
drafts: [draftItem("draft", "2026-07-21T12:00:00.000Z")],
|
||||
items: [inboxItem("message", 1_753_099_300)],
|
||||
reminders: [reminder("reminder", 1_753_099_100)],
|
||||
});
|
||||
|
||||
assert.deepEqual(
|
||||
rows.map((row) => row.kind),
|
||||
["draft", "inbox", "reminder"],
|
||||
["inbox", "reminder"],
|
||||
);
|
||||
});
|
||||
|
||||
test("Inbox All excludes completed reminders and deleted-root drafts", () => {
|
||||
test("Inbox All excludes completed reminders", () => {
|
||||
const rows = buildInboxListRows({
|
||||
drafts: [draftItem("deleted", "2026-07-21T12:00:00.000Z", "deleted")],
|
||||
items: [],
|
||||
reminders: [reminder("done", 1_753_099_100, "done")],
|
||||
});
|
||||
@@ -69,12 +57,10 @@ test("Inbox All excludes completed reminders and deleted-root drafts", () => {
|
||||
|
||||
test("Inbox conversation keys stay stable when the representative changes", () => {
|
||||
const first = buildInboxListRows({
|
||||
drafts: [],
|
||||
items: [inboxItem("reply-1", 1, "thread-root")],
|
||||
reminders: [],
|
||||
});
|
||||
const second = buildInboxListRows({
|
||||
drafts: [],
|
||||
items: [inboxItem("reply-2", 2, "thread-root")],
|
||||
reminders: [],
|
||||
});
|
||||
@@ -87,7 +73,6 @@ test("due reminder enriches its existing conversation instead of duplicating it"
|
||||
const item = inboxItem("message", 100);
|
||||
item.groupItems = [{ id: "reminded-reply" }];
|
||||
const rows = buildInboxListRows({
|
||||
drafts: [],
|
||||
items: [item],
|
||||
reminders: [
|
||||
reminder("reminder", 50, "pending", {
|
||||
@@ -105,7 +90,6 @@ test("due reminder enriches its existing conversation instead of duplicating it"
|
||||
|
||||
test("due reminder without a represented conversation sorts at trigger time", () => {
|
||||
const rows = buildInboxListRows({
|
||||
drafts: [],
|
||||
items: [inboxItem("newer-than-creation", 150)],
|
||||
reminders: [
|
||||
reminder("reminder", 50, "pending", {
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
import type { InboxItem } from "@/features/home/lib/inbox";
|
||||
import type { DraftViewItem } from "@/features/messages/ui/DraftsPanel";
|
||||
import type { Reminder } from "@/features/reminders/lib/reminderTypes";
|
||||
|
||||
export type InboxListRow =
|
||||
@@ -15,31 +14,12 @@ export type InboxListRow =
|
||||
kind: "reminder";
|
||||
reminder: Reminder;
|
||||
sortAt: number;
|
||||
}
|
||||
| {
|
||||
key: string;
|
||||
kind: "draft";
|
||||
item: DraftViewItem;
|
||||
sortAt: number;
|
||||
};
|
||||
|
||||
function draftActivityAt(item: DraftViewItem): number {
|
||||
for (const value of [
|
||||
item.entry.draft.updatedAt,
|
||||
item.entry.draft.createdAt,
|
||||
]) {
|
||||
const timestamp = Date.parse(value);
|
||||
if (Number.isFinite(timestamp)) return timestamp / 1_000;
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
|
||||
export function buildInboxListRows({
|
||||
drafts,
|
||||
items,
|
||||
reminders,
|
||||
}: {
|
||||
drafts: readonly DraftViewItem[];
|
||||
items: readonly InboxItem[];
|
||||
reminders: readonly Reminder[];
|
||||
}): InboxListRow[] {
|
||||
@@ -98,15 +78,5 @@ export function buildInboxListRows({
|
||||
sortAt: reminder.notBefore ?? reminder.createdAt,
|
||||
}),
|
||||
),
|
||||
...drafts
|
||||
.filter((item) => item.rootStatus !== "deleted")
|
||||
.map(
|
||||
(item): InboxListRow => ({
|
||||
key: `draft:${item.entry.key}`,
|
||||
kind: "draft",
|
||||
item,
|
||||
sortAt: draftActivityAt(item),
|
||||
}),
|
||||
),
|
||||
].sort((left, right) => right.sortAt - left.sortAt);
|
||||
}
|
||||
|
||||
@@ -1,11 +1,4 @@
|
||||
import {
|
||||
Bell,
|
||||
Clock,
|
||||
Ellipsis,
|
||||
ExternalLink,
|
||||
FileText,
|
||||
MailOpen,
|
||||
} from "lucide-react";
|
||||
import { Bell, Clock, Ellipsis, ExternalLink, MailOpen } from "lucide-react";
|
||||
import * as React from "react";
|
||||
|
||||
import {
|
||||
@@ -18,7 +11,6 @@ import { buildInboxListRows } from "@/features/home/lib/inboxListRows";
|
||||
import { InboxFilterMenu } from "@/features/home/ui/InboxFilterMenu";
|
||||
import {
|
||||
DraftsPanel,
|
||||
getDraftPreview,
|
||||
type DraftViewItem,
|
||||
} from "@/features/messages/ui/DraftsPanel";
|
||||
import { UserProfilePopover } from "@/features/profile/ui/UserProfilePopover";
|
||||
@@ -130,7 +122,6 @@ function formatReminderStatus(notBefore: number | undefined) {
|
||||
|
||||
function PersonalItemRow({
|
||||
id,
|
||||
kind,
|
||||
location,
|
||||
onClick,
|
||||
preview,
|
||||
@@ -138,16 +129,12 @@ function PersonalItemRow({
|
||||
status,
|
||||
}: {
|
||||
id: string;
|
||||
kind: "drafts" | "reminders";
|
||||
location: InboxTypeLabel | null;
|
||||
onClick: () => void;
|
||||
preview: string;
|
||||
selected: boolean;
|
||||
status: string;
|
||||
}) {
|
||||
const isDraft = kind === "drafts";
|
||||
const Icon = isDraft ? FileText : Bell;
|
||||
|
||||
return (
|
||||
<button
|
||||
aria-current={selected ? "true" : undefined}
|
||||
@@ -155,16 +142,16 @@ function PersonalItemRow({
|
||||
"flex w-full items-center gap-3 border-b border-border/45 px-4 py-3 text-left transition-colors hover:bg-muted/40 focus-visible:bg-muted/40 focus-visible:outline-hidden",
|
||||
selected && "bg-muted/40",
|
||||
)}
|
||||
data-testid={`home-all-${kind}-${id}`}
|
||||
data-testid={`home-all-reminders-${id}`}
|
||||
onClick={onClick}
|
||||
type="button"
|
||||
>
|
||||
<span className="flex h-9 w-9 shrink-0 items-center justify-center rounded-full bg-muted text-muted-foreground">
|
||||
<Icon className="h-4 w-4" />
|
||||
<Bell className="h-4 w-4" />
|
||||
</span>
|
||||
<span className="min-w-0 flex-1">
|
||||
<span className="block text-sm font-semibold text-foreground">
|
||||
{isDraft ? "Draft" : "Reminder"}
|
||||
Reminder
|
||||
</span>
|
||||
{location ? (
|
||||
<InboxLabel
|
||||
@@ -246,7 +233,6 @@ export function InboxListPane({
|
||||
const inboxRows = React.useMemo(
|
||||
() =>
|
||||
buildInboxListRows({
|
||||
drafts: unreadOnly || !isMixedInboxView ? [] : draftItems,
|
||||
items,
|
||||
reminders: unreadOnly
|
||||
? []
|
||||
@@ -254,7 +240,7 @@ export function InboxListPane({
|
||||
isDue(reminder, Math.floor(Date.now() / 1_000)),
|
||||
),
|
||||
}),
|
||||
[draftItems, isMixedInboxView, items, reminders, unreadOnly],
|
||||
[items, reminders, unreadOnly],
|
||||
);
|
||||
const visibleInboxRows = React.useMemo(
|
||||
() =>
|
||||
@@ -632,57 +618,30 @@ export function InboxListPane({
|
||||
return renderItem(row.item, row.dueReminder);
|
||||
}
|
||||
|
||||
if (row.kind === "reminder") {
|
||||
const source = reminderSources.get(row.reminder.id);
|
||||
return (
|
||||
<PersonalItemRow
|
||||
id={row.reminder.id}
|
||||
kind="reminders"
|
||||
location={
|
||||
source?.channel
|
||||
? source.channel.channelType === "dm"
|
||||
? {
|
||||
text: `In DM with ${source.channelLabel}`,
|
||||
channelLabel: null,
|
||||
}
|
||||
: { text: "In", channelLabel: source.channelLabel }
|
||||
: null
|
||||
}
|
||||
onClick={() => {
|
||||
onSelectReminder(row.reminder.id);
|
||||
}}
|
||||
preview={
|
||||
row.reminder.content.target?.preview ||
|
||||
row.reminder.content.note ||
|
||||
"Reminder"
|
||||
}
|
||||
selected={selectedReminderId === row.reminder.id}
|
||||
status={formatReminderStatus(row.reminder.notBefore)}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
const { entry, source } = row.item;
|
||||
const source = reminderSources.get(row.reminder.id);
|
||||
return (
|
||||
<PersonalItemRow
|
||||
id={entry.key}
|
||||
kind="drafts"
|
||||
id={row.reminder.id}
|
||||
location={
|
||||
source.channel
|
||||
source?.channel
|
||||
? source.channel.channelType === "dm"
|
||||
? {
|
||||
text: `In DM with ${source.label}`,
|
||||
text: `In DM with ${source.channelLabel}`,
|
||||
channelLabel: null,
|
||||
}
|
||||
: { text: "In", channelLabel: source.label }
|
||||
: { text: "In", channelLabel: source.channelLabel }
|
||||
: null
|
||||
}
|
||||
onClick={() => {
|
||||
onSelectDraft(entry.key);
|
||||
onSelectReminder(row.reminder.id);
|
||||
}}
|
||||
preview={getDraftPreview(entry.draft)}
|
||||
selected={selectedDraftKey === entry.key}
|
||||
status="Draft saved"
|
||||
preview={
|
||||
row.reminder.content.target?.preview ||
|
||||
row.reminder.content.note ||
|
||||
"Reminder"
|
||||
}
|
||||
selected={selectedReminderId === row.reminder.id}
|
||||
status={formatReminderStatus(row.reminder.notBefore)}
|
||||
/>
|
||||
);
|
||||
}}
|
||||
|
||||
@@ -68,10 +68,12 @@ export function useHomePersonalInbox({
|
||||
viewportWidthPx,
|
||||
]);
|
||||
|
||||
// Drafts are only listed (and selectable) under the dedicated Drafts
|
||||
// filter — they never appear in the mixed All view.
|
||||
const drafts = useHomeDrafts({
|
||||
autoSelect: isDrafts,
|
||||
isNarrowHomeViewport,
|
||||
selectionEnabled: isDrafts || allowMixedSelection,
|
||||
selectionEnabled: isDrafts,
|
||||
viewportWidthPx,
|
||||
});
|
||||
|
||||
|
||||
@@ -2632,7 +2632,7 @@ test("Inbox All excludes generic channel traffic", async ({ page }) => {
|
||||
).toHaveCount(0);
|
||||
});
|
||||
|
||||
test("Inbox unread-only hides reminders and drafts from mixed All", async ({
|
||||
test("Inbox All never lists drafts and unread-only hides reminders", async ({
|
||||
page,
|
||||
}) => {
|
||||
const draftKey = `channel:${GENERAL_CHANNEL_ID}`;
|
||||
@@ -2737,7 +2737,8 @@ test("Inbox unread-only hides reminders and drafts from mixed All", async ({
|
||||
const draftRow = page.getByTestId(`home-all-drafts-${draftKey}`);
|
||||
await expect(messageRow).toBeVisible();
|
||||
await expect(reminderRow).toBeVisible();
|
||||
await expect(draftRow).toBeVisible();
|
||||
// Drafts belong to the dedicated Drafts filter — never the mixed All view.
|
||||
await expect(draftRow).toHaveCount(0);
|
||||
|
||||
await page.getByTestId("inbox-options-trigger").click();
|
||||
await page.getByRole("switch", { name: "Show unread only" }).click();
|
||||
@@ -2745,6 +2746,12 @@ test("Inbox unread-only hides reminders and drafts from mixed All", async ({
|
||||
await expect(messageRow).toBeVisible();
|
||||
await expect(reminderRow).toHaveCount(0);
|
||||
await expect(draftRow).toHaveCount(0);
|
||||
|
||||
// The draft is still reachable under the Drafts filter.
|
||||
await page.keyboard.press("Escape");
|
||||
await page.getByTestId("inbox-filter-trigger").click();
|
||||
await page.getByRole("menuitemradio", { name: "Drafts" }).click();
|
||||
await expect(page.getByTestId("home-inbox-drafts")).toBeVisible();
|
||||
});
|
||||
|
||||
test("Inbox merges a due reminder into its represented conversation", async ({
|
||||
|
||||
105
desktop/tests/e2e/drafts-all-fix-screenshots.spec.ts
Normal file
105
desktop/tests/e2e/drafts-all-fix-screenshots.spec.ts
Normal file
@@ -0,0 +1,105 @@
|
||||
import { expect, test } from "@playwright/test";
|
||||
|
||||
import { waitForAnimations } from "../helpers/animations";
|
||||
import { TEST_IDENTITIES, installMockBridge } from "../helpers/bridge";
|
||||
|
||||
const SHOTS = "test-results/drafts-all-fix";
|
||||
|
||||
const GENERAL_CHANNEL_ID = "9a1657ac-f7aa-5db0-b632-d8bbeb6dfb50";
|
||||
const MOCK_IDENTITY_PUBKEY = "deadbeef".repeat(8);
|
||||
|
||||
type MockFeedWindow = Window & {
|
||||
__BUZZ_E2E_PUSH_MOCK_FEED_ITEM__?: (item: Record<string, unknown>) => void;
|
||||
};
|
||||
|
||||
test.use({ viewport: { width: 1280, height: 720 } });
|
||||
|
||||
test("Inbox All hides drafts while the Drafts filter keeps them", async ({
|
||||
page,
|
||||
}) => {
|
||||
const draftKey = `channel:${GENERAL_CHANNEL_ID}`;
|
||||
await page.addInitScript(
|
||||
({ draftStoreKey, draftStorageKey, channelId }) => {
|
||||
const timestamp = new Date().toISOString();
|
||||
window.localStorage.setItem(
|
||||
draftStoreKey,
|
||||
JSON.stringify({
|
||||
[draftStorageKey]: {
|
||||
channelId,
|
||||
content: "Draft that must stay out of the All view",
|
||||
createdAt: timestamp,
|
||||
pendingImeta: [],
|
||||
selectionEnd: 41,
|
||||
selectionStart: 41,
|
||||
spoileredAttachmentUrls: [],
|
||||
status: "active",
|
||||
updatedAt: timestamp,
|
||||
},
|
||||
}),
|
||||
);
|
||||
},
|
||||
{
|
||||
channelId: GENERAL_CHANNEL_ID,
|
||||
draftStorageKey: draftKey,
|
||||
draftStoreKey: `buzz-drafts.v2:ws://localhost:3000:${MOCK_IDENTITY_PUBKEY}`,
|
||||
},
|
||||
);
|
||||
await installMockBridge(page);
|
||||
await page.goto("/");
|
||||
await page.waitForFunction(() => {
|
||||
const win = window as MockFeedWindow;
|
||||
return typeof win.__BUZZ_E2E_PUSH_MOCK_FEED_ITEM__ === "function";
|
||||
});
|
||||
|
||||
const messageId = "drafts-all-fix-message";
|
||||
await page.evaluate(
|
||||
({ channelId, currentPubkey, messageId: id, senderPubkey }) => {
|
||||
const pushFeedItem = (window as MockFeedWindow)
|
||||
.__BUZZ_E2E_PUSH_MOCK_FEED_ITEM__;
|
||||
if (!pushFeedItem) throw new Error("Mock feed helper is not installed.");
|
||||
pushFeedItem({
|
||||
category: "mention",
|
||||
channel_id: channelId,
|
||||
channel_name: "general",
|
||||
channel_type: "stream",
|
||||
content: "Regular message that belongs in All",
|
||||
created_at: Math.floor(Date.now() / 1_000),
|
||||
id,
|
||||
kind: 9,
|
||||
pubkey: senderPubkey,
|
||||
tags: [
|
||||
["h", channelId],
|
||||
["p", currentPubkey],
|
||||
],
|
||||
});
|
||||
},
|
||||
{
|
||||
channelId: GENERAL_CHANNEL_ID,
|
||||
currentPubkey: MOCK_IDENTITY_PUBKEY,
|
||||
messageId,
|
||||
senderPubkey: TEST_IDENTITIES.alice.pubkey,
|
||||
},
|
||||
);
|
||||
|
||||
// All view: the message is present, the draft row is not.
|
||||
await expect(page.getByTestId(`home-inbox-item-${messageId}`)).toBeVisible();
|
||||
await expect(page.getByTestId(`home-all-drafts-${draftKey}`)).toHaveCount(0);
|
||||
await waitForAnimations(page);
|
||||
await page
|
||||
.getByTestId("home-inbox")
|
||||
.screenshot({ path: `${SHOTS}/01-all-view-no-drafts.png` });
|
||||
|
||||
// Drafts filter: the draft is still listed.
|
||||
await page.getByTestId("inbox-filter-trigger").click();
|
||||
await page.getByRole("menuitemradio", { name: "Drafts" }).click();
|
||||
await page.keyboard.press("Escape");
|
||||
const panel = page.getByTestId("home-inbox-drafts");
|
||||
await expect(panel).toBeVisible();
|
||||
await expect(
|
||||
panel.getByText("Draft that must stay out of the All view"),
|
||||
).toBeVisible();
|
||||
await waitForAnimations(page);
|
||||
await page
|
||||
.getByTestId("home-inbox")
|
||||
.screenshot({ path: `${SHOTS}/02-drafts-filter-still-lists.png` });
|
||||
});
|
||||
Reference in New Issue
Block a user