From 53810971fcb6557116b1a6097453ee4de4e063b7 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Tue, 27 Oct 2026 00:01:42 -0700 Subject: [PATCH 01/21] feat(sidebar): expose section creation from menus and drag Co-authored-by: Carl Signed-off-by: Taylor Ho --- browser-host/sidebar-create-section.test.mjs | 35 +++++ browser-host/sidebar-preferences.mjs | 2 +- docs/channels.md | 8 ++ src/bundled/channels/Channels.module.css | 11 ++ .../channels/CreateSidebarSection.test.tsx | 60 ++++++++- src/bundled/channels/CreateSidebarSection.tsx | 60 ++++++--- src/bundled/channels/SidebarSection.tsx | 22 +++ src/bundled/channels/SoloWorkspaces.test.tsx | 8 +- src/bundled/channels/useSidebarPreferences.ts | 2 + .../ChannelInventory.test.tsx | 4 +- .../channel-navigation/ChannelInventory.tsx | 44 ++++++ .../channel-navigation/ChannelSidebarDnd.tsx | 35 ++++- src/features/relay/native-sidebar.test.ts | 32 +++++ src/features/relay/native-sidebar.ts | 2 +- src/features/relay/sidebar-edits.ts | 7 +- .../relay/sidebar-moves-store.test.ts | 30 ++++- .../relay/sidebar-preferences-store.test.ts | 6 +- .../relay/sidebar-preferences-store.ts | 18 ++- src/features/relay/sidebar-preferences.ts | 5 +- src/features/relay/solo-preferences.ts | 9 +- tests/browser/navigation-groups.spec.mjs | 10 +- .../browser/sidebar-section-creation.spec.mjs | 125 ++++++++++++++++++ 22 files changed, 487 insertions(+), 48 deletions(-) create mode 100644 tests/browser/sidebar-section-creation.spec.mjs diff --git a/browser-host/sidebar-create-section.test.mjs b/browser-host/sidebar-create-section.test.mjs index a52b438474..e6e970b4e7 100644 --- a/browser-host/sidebar-create-section.test.mjs +++ b/browser-host/sidebar-create-section.test.mjs @@ -257,3 +257,38 @@ it.each([ expect(decode(head)).toEqual(blob); }, ); + +it("creates an empty section without changing any assignments, and confirms the same ID on retry", async () => { + const create = { createSection: { id, name: " Launch " } }; + let head = event({ + version: 1, + sections: [{ id: "work", name: "Work", order: 0 }], + assignments: { alpha: "work" }, + }); + const publications = []; + const read = async () => [head]; + const publish = async (record) => { + publications.push(record); + head = record; + }; + const groups = await mutateSidebarAssignment(create, secret, read, publish); + expect(groups.sections).toEqual([ + { id: "work", name: "Work", order: 0 }, + { id, name: "Launch", order: 1 }, + ]); + expect(groups.assignments).toEqual({ alpha: "work" }); + expect(decode(head).meta.a).not.toHaveProperty("undefined"); + expect(await mutateSidebarAssignment(create, secret, read, publish)).toEqual( + groups, + ); + expect(publications).toHaveLength(1); +}); + +it.each([ + {}, + { sectionId: "work" }, + { createSection: { id, name: " " } }, + { channelId: null, createSection: { id, name: "Launch" } }, +])("rejects invalid empty-section envelopes: %j", (intent) => { + expect(() => assertSidebarAssignmentIntent(intent)).toThrow("Invalid"); +}); diff --git a/browser-host/sidebar-preferences.mjs b/browser-host/sidebar-preferences.mjs index 19434f5acd..2fff380175 100644 --- a/browser-host/sidebar-preferences.mjs +++ b/browser-host/sidebar-preferences.mjs @@ -77,7 +77,7 @@ function validAssignmentIntent(intent) { intent && typeof intent === "object" && !Array.isArray(intent) && - typeof intent.channelId === "string" && + (intent.channelId === undefined || typeof intent.channelId === "string") && (intent.sectionId === undefined || typeof intent.sectionId === "string") && (intent.createSection === undefined || (intent.createSection && diff --git a/docs/channels.md b/docs/channels.md index 4b0831b6f1..2ff96094ca 100644 --- a/docs/channels.md +++ b/docs/channels.md @@ -151,6 +151,14 @@ belong to this persistent sidebar. Rows in saved groups, **Starred** and **Chann can also be dragged with a pointer onto another saved group, **Starred** or **Channels**; the drop is the same Move intent, with the same gate, optimistic placement, rollback notice and Retry. +The **Channels** ellipsis menu also offers **Create section**, which creates an +empty section without moving a channel. Its dialog retains the name and section +ID on failure and waits for confirmation before closing. While dragging a channel, +a **Create section** drop target stays at the bottom of the visible sidebar. +Dropping there opens the existing create-and-move dialog; cancelling leaves the +channel in place, and no preference is written until submission. These entry +points supplement the per-channel **Move channel → Create new…** route and use +the existing gated preference queue and encrypted section record. Only a channel's select surface starts a drag, the surface its row menu wraps. **Starred** accepts drops only when it is already visible; starting a drag does not reveal an empty section. Use the row menu to star the first channel. diff --git a/src/bundled/channels/Channels.module.css b/src/bundled/channels/Channels.module.css index 41b72541c5..ac6f10c432 100644 --- a/src/bundled/channels/Channels.module.css +++ b/src/bundled/channels/Channels.module.css @@ -296,6 +296,15 @@ border-radius: var(--radius-row); background: var(--affordance-panel-hover); } +.createSectionDrop { + position: sticky; + bottom: 0; + z-index: 1; + margin-top: var(--space-2); + border: 1px dashed var(--border-standard); + border-radius: var(--radius-row); + background: var(--surface-panel); +} [data-channel-dragging], .channelSection[data-section-dragging] { opacity: 0.3; @@ -319,6 +328,8 @@ transform: translateY(50%); } .channelDragOverlay { + /* Keep the destination label visible beneath the pointer. */ + translate: var(--space-3) var(--space-6); display: flex; align-items: center; gap: var(--space-2); diff --git a/src/bundled/channels/CreateSidebarSection.test.tsx b/src/bundled/channels/CreateSidebarSection.test.tsx index 3273892e97..705af4a8a9 100644 --- a/src/bundled/channels/CreateSidebarSection.test.tsx +++ b/src/bundled/channels/CreateSidebarSection.test.tsx @@ -1,12 +1,18 @@ // @vitest-environment jsdom import "@testing-library/jest-dom/vitest"; -import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { + cleanup, + fireEvent, + render, + screen, + waitFor, +} from "@testing-library/react"; import { afterEach, expect, it, vi } from "vitest"; import { CreateSidebarSection } from "./CreateSidebarSection"; afterEach(cleanup); -it("retains a rejected draft and its identity through preference recovery", () => { +it("retains a rejected draft and its identity through preference recovery", async () => { const create = vi.fn().mockReturnValue(false); const retry = vi.fn().mockResolvedValue(undefined); const props = { @@ -23,13 +29,15 @@ it("retains a rejected draft and its identity through preference recovery", () = const form = input.closest("form"); if (!form) throw new Error("Create form missing"); const submit = () => fireEvent.submit(form); - submit(); + await submit(); expect(create).toHaveBeenCalledWith({ id: expect.any(String), name: "Launch", }); expect(input).toHaveValue(" Launch "); - expect(screen.getByRole("alert")).toHaveTextContent("Retry preferences"); + await waitFor(() => + expect(screen.getByRole("alert")).toHaveTextContent("Retry preferences"), + ); fireEvent.click(screen.getByRole("button", { name: "Retry preferences" })); expect(retry).toHaveBeenCalledTimes(1); view.rerender(); @@ -41,9 +49,49 @@ it("retains a rejected draft and its identity through preference recovery", () = create.mockReturnValue(true); view.rerender(); expect(screen.queryByRole("alert")).toBeNull(); - submit(); + await submit(); expect(create).toHaveBeenCalledTimes(2); expect(create.mock.calls[1]).toEqual(create.mock.calls[0]); - submit(); + await submit(); expect(create).toHaveBeenCalledTimes(2); }); + +it("keeps an empty-section draft and identity through a failed save, blocking dismissal while pending", async () => { + let reject!: (cause: Error) => void; + const create = vi + .fn() + .mockImplementationOnce( + () => + new Promise((_, no) => { + reject = no; + }), + ) + .mockResolvedValue(true); + const close = vi.fn(); + render( + , + ); + const input = screen.getByRole("textbox", { name: "Section name" }); + fireEvent.change(input, { target: { value: " Launch " } }); + const submit = screen.getByRole("button", { name: "Create section" }); + fireEvent.click(submit); + expect(input).toBeDisabled(); + expect(screen.getByRole("button", { name: "Cancel" })).toBeDisabled(); + const form = input.closest("form"); + if (!form) throw new Error("Create form missing"); + fireEvent.submit(form); + expect(create).toHaveBeenCalledTimes(1); + reject(new Error("Could not confirm section")); + await screen.findByRole("alert"); + expect(input).toHaveValue(" Launch "); + expect(close).not.toHaveBeenCalled(); + fireEvent.click(submit); + await waitFor(() => expect(input).toBeEnabled()); + expect(create.mock.calls[1]).toEqual(create.mock.calls[0]); +}); diff --git a/src/bundled/channels/CreateSidebarSection.tsx b/src/bundled/channels/CreateSidebarSection.tsx index a1ff685889..669e44b5a1 100644 --- a/src/bundled/channels/CreateSidebarSection.tsx +++ b/src/bundled/channels/CreateSidebarSection.tsx @@ -1,4 +1,4 @@ -import { useId, useRef, useState } from "react"; +import { useId, useRef, useState, type ComponentProps } from "react"; import { Button } from "../../shared/design-system/ui/Button"; import { Dialog } from "../../shared/design-system/ui/Dialog"; import { Input } from "../../shared/design-system/ui/Input"; @@ -12,14 +12,16 @@ export function CreateSidebarSection({ writable, refreshing, retry, + finalFocus = false, }: { - channelName: string; - /** True only after the session accepts the optimistic move. */ - create: (section: { id: string; name: string }) => boolean; + channelName?: string | undefined; + /** True only after the session accepts the move or confirms empty creation. */ + create: (section: { id: string; name: string }) => boolean | Promise; close: () => void; writable: boolean; refreshing: boolean; retry: () => Promise; + finalFocus?: ComponentProps["finalFocus"]; }) { const formId = useId(); const input = useRef(null); @@ -27,33 +29,61 @@ export function CreateSidebarSection({ const submitted = useRef(false); const sectionId = useRef(undefined); const [blocked, setBlocked] = useState(false); + const [pending, setPending] = useState(false); + const [error, setError] = useState(""); + const submit = async () => { + if (submitted.current || pending || !name.trim()) return; + submitted.current = true; + sectionId.current ??= crypto.randomUUID(); + setPending(true); + setError(""); + try { + const accepted = await create({ + id: sectionId.current, + name: name.trim(), + }); + submitted.current = accepted; + setBlocked(!accepted); + } catch (cause) { + submitted.current = false; + setError(cause instanceof Error ? cause.message : String(cause)); + } finally { + setPending(false); + } + }; return ( { if (!open) close(); }} title="Create new section" description={ - - Move {channelName} into a new - section. - + channelName ? ( + + Move {channelName} into a new + section. + + ) : undefined } initialFocus={input} - finalFocus={false} + finalFocus={finalFocus} actions={ <> - + } @@ -62,11 +92,7 @@ export function CreateSidebarSection({ id={formId} onSubmit={(event) => { event.preventDefault(); - if (submitted.current || !name.trim()) return; - sectionId.current ??= crypto.randomUUID(); - const accepted = create({ id: sectionId.current, name: name.trim() }); - submitted.current = accepted; - setBlocked(!accepted); + void submit(); }} > setName(event.target.value)} /> + {error &&

{error}

} {blocked && !writable && (

diff --git a/src/bundled/channels/SidebarSection.tsx b/src/bundled/channels/SidebarSection.tsx index bd7bb941ce..699a19d301 100644 --- a/src/bundled/channels/SidebarSection.tsx +++ b/src/bundled/channels/SidebarSection.tsx @@ -63,6 +63,7 @@ export function SidebarSection({ headerNotice, hideMoreActions = false, createChannel, + createSection, newMessage, sort, filter, @@ -97,6 +98,9 @@ export function SidebarSection({ | { available: boolean; open: (trigger: HTMLButtonElement) => void } | undefined; newMessage?: (() => void) | undefined; + createSection?: + | { available: boolean; open: (trigger: HTMLButtonElement) => void } + | undefined; sort?: | { value: "alpha" | "recent"; @@ -236,6 +240,24 @@ export function SidebarSection({ )} /> + {createSection && ( + <> + { + setMenuOpen(false); + if (actionsTrigger.current) + createSection.open(actionsTrigger.current); + }} + > + + + + Create section + + + + )} {filter && ( diff --git a/src/bundled/channels/SoloWorkspaces.test.tsx b/src/bundled/channels/SoloWorkspaces.test.tsx index bb4c47640c..a11e3d5ca0 100644 --- a/src/bundled/channels/SoloWorkspaces.test.tsx +++ b/src/bundled/channels/SoloWorkspaces.test.tsx @@ -44,7 +44,7 @@ function fixture(projectName = "Work") { }; const write = vi.fn( async (intent: { - channelId: string; + channelId?: string; sectionId?: string; createSection?: { id: string; name: string }; }) => { @@ -55,8 +55,10 @@ function fixture(projectName = "Work") { }; const assignments = { ...saved.assignments }; const target = intent.createSection?.id ?? intent.sectionId; - if (target) assignments[intent.channelId] = target; - else delete assignments[intent.channelId]; + if (intent.channelId !== undefined) { + if (target) assignments[intent.channelId] = target; + else delete assignments[intent.channelId]; + } saved = { ...saved, assignments }; return saved; }, diff --git a/src/bundled/channels/useSidebarPreferences.ts b/src/bundled/channels/useSidebarPreferences.ts index fac553bc02..9cd3a383d6 100644 --- a/src/bundled/channels/useSidebarPreferences.ts +++ b/src/bundled/channels/useSidebarPreferences.ts @@ -26,6 +26,8 @@ export function useSidebarPreferences( reorderSections: queries.reorderSections, dismissOrderError: queries.dismissOrderError, createAndAssign: queries.createAndAssign, + createSection: queries.createSection, + sectionCreationWritable: queries.sectionCreationWritable, retryMove: queries.retryMove, dismissMoveError: queries.dismissMoveError, starWritable: queries.starWritable, diff --git a/src/features/channel-navigation/ChannelInventory.test.tsx b/src/features/channel-navigation/ChannelInventory.test.tsx index 302748f936..ae14285e5a 100644 --- a/src/features/channel-navigation/ChannelInventory.test.tsx +++ b/src/features/channel-navigation/ChannelInventory.test.tsx @@ -738,7 +738,9 @@ it("offers DMs a Move conversation menu and relocates them into a saved group", let assignments: Record = {}; const read = vi.fn(async () => ({ ...saved, assignments })); const write = vi.fn( - async (intent: { channelId: string; sectionId?: string }) => { + async (intent: { channelId?: string; sectionId?: string }) => { + if (intent.channelId === undefined) + throw new Error("Expected channel move"); const changedAssignments = intent.sectionId ? { ...assignments, [intent.channelId]: intent.sectionId } : Object.fromEntries( diff --git a/src/features/channel-navigation/ChannelInventory.tsx b/src/features/channel-navigation/ChannelInventory.tsx index 076e20755e..db7cb5b66b 100644 --- a/src/features/channel-navigation/ChannelInventory.tsx +++ b/src/features/channel-navigation/ChannelInventory.tsx @@ -264,6 +264,8 @@ export function ChannelInventory({ { channelId: string; target: HTMLElement } | undefined >(undefined); const pendingCreate = useRef(undefined); + const [creatingSection, setCreatingSection] = useState(false); + const createSectionTrigger = useRef(null); const [creatingFor, setCreatingFor] = useState<{ channel: ChannelSummary; surface?: ChannelMenuSurface; @@ -1121,6 +1123,18 @@ export function ChannelInventory({ <> { + const channel = sidebarChannels.find(({ id }) => id === channelId); + // Finish releasing the pointer before giving the modal its focus. + requestAnimationFrame(() => { + if ( + mounted.current && + channel && + queries.sidebarPreferences.writable + ) + setCreatingFor({ channel }); + }); + }} onMove={(channelId, sectionKey) => sectionKey === "starred" ? setChannelStar(channelId, true) @@ -1285,6 +1299,19 @@ export function ChannelInventory({ } : undefined } + createSection={ + section.key === "channels" + ? { + available: preferences.sectionCreationWritable, + open: (trigger) => { + createSectionTrigger.current = trigger; + requestAnimationFrame(() => { + if (mounted.current) setCreatingSection(true); + }); + }, + } + : undefined + } newMessage={ section.key === "dms" ? () => { @@ -1624,6 +1651,23 @@ export function ChannelInventory({ ), dialogs: ( <> + {creatingSection && ( + setCreatingSection(false)} + create={async (section) => { + if (!queries.sidebarPreferences.sectionCreationWritable) + return false; + await preferences.createSection(section); + sidebar.toggle(`group:${section.id}`, true); + setCreatingSection(false); + return true; + }} + /> + )} {creatingFor && !creatingFor.surface?.signal?.aborted && ( { @@ -79,12 +83,14 @@ export function ChannelSidebarDnd({ writable, onMove, onReorder, + onCreateSection, overlay, children, }: { writable: boolean; onMove: (channelId: string, sectionKey: string) => void; onReorder: (sectionKey: string, overKey: string) => void; + onCreateSection?: ((channelId: string) => void) | undefined; overlay: (id: string, group: boolean) => ReactNode; children: ReactNode; }) { @@ -112,11 +118,16 @@ export function ChannelSidebarDnd({ if (active.data.current?.group) { if (over.id !== active.id) onReorder(String(active.id), String(over.id)); + } else if (over.id === CREATE_SECTION) { + if (writable) onCreateSection?.(String(active.id)); } else if (over.id !== active.data.current?.sectionKey) onMove(String(active.id), String(over.id)); }} > - {children} + + {children} + {onCreateSection && } + {createPortal( // The optimistic move relocates the row, so nothing remains to settle back onto. @@ -128,6 +139,28 @@ export function ChannelSidebarDnd({ ); } +function CreateSectionDropTarget({ disabled }: { disabled: boolean }) { + const { setNodeRef, isOver, active } = useDroppable({ + id: CREATE_SECTION, + disabled, + }); + if (disabled || !active || active.data.current?.group) return null; + return ( +

+ } + highlighted={isOver} + tabIndex={-1} + /> +
+ ); +} + /** * Frames only a channel's select surface, so its sessions never start a move. * The frame stays mounted while placement is read-only: the row keeps its diff --git a/src/features/relay/native-sidebar.test.ts b/src/features/relay/native-sidebar.test.ts index 9aa46891ab..34e1617074 100644 --- a/src/features/relay/native-sidebar.test.ts +++ b/src/features/relay/native-sidebar.test.ts @@ -926,3 +926,35 @@ it("rejects a same-mode native readback with a conflicting duration", async () = ).rejects.toThrow("Invalid sidebar filter intent"); expect(vi.mocked(invoke).mock.calls).toHaveLength(before); }); + +it("creates an empty Team section and confirms retry without touching placements or stars", async () => { + records.set( + "channel-sections", + signedRecord("channel-sections", { + version: 1, + sections: [{ id: "work", name: "Work", order: 0 }], + assignments: { alpha: "work" }, + }), + ); + const transport = await connectNativeTransport(community); + if (!transport.writeSidebarAssignment) + throw new Error("Missing sidebar writer"); + const intent = { + createSection: { + id: "12345678-1234-1234-1234-123456789abc", + name: " Launch ", + }, + }; + const created = await transport.writeSidebarAssignment(intent, signal); + expect(created.assignments).toEqual({ alpha: "work" }); + expect(created.sections.at(-1)).toMatchObject({ + id: intent.createSection.id, + name: "Launch", + }); + const head = records.get("channel-sections"); + expect(await transport.writeSidebarAssignment(intent, signal)).toEqual( + created, + ); + expect(records.get("channel-sections")).toBe(head); + expect(records.has("channel-stars")).toBe(false); +}); diff --git a/src/features/relay/native-sidebar.ts b/src/features/relay/native-sidebar.ts index f51ce30fc3..1cfd7c33a6 100644 --- a/src/features/relay/native-sidebar.ts +++ b/src/features/relay/native-sidebar.ts @@ -253,7 +253,7 @@ export function nativeSidebar(transport: ReadTransport) { (value) => { const assigned = value.assignments as Record; return ( - assigned[channelId] === section && + (channelId === undefined || assigned[channelId] === section) && (!createSection || (value.sections as SidebarGroups["sections"]).some( (entry) => diff --git a/src/features/relay/sidebar-edits.ts b/src/features/relay/sidebar-edits.ts index aceab82991..d78ab53916 100644 --- a/src/features/relay/sidebar-edits.ts +++ b/src/features/relay/sidebar-edits.ts @@ -86,7 +86,9 @@ export function validSidebarAssignment( ): boolean { const { channelId, sectionId, createSection } = intent; return ( - validSidebarChannelId(channelId) && + (channelId === undefined + ? !!createSection + : validSidebarChannelId(channelId)) && (sectionId === undefined || validSidebarChannelId(sectionId)) && (!createSection || (sectionId === undefined && @@ -174,7 +176,8 @@ export function editSidebarAssignment( } if (section !== undefined && !sections.some((entry) => entry.id === section)) throw new Error("Sidebar group no longer exists"); - const writes: [string[], unknown][] = [[["a", channelId], section ?? null]]; + const writes: [string[], unknown][] = + channelId === undefined ? [] : [[["a", channelId], section ?? null]]; if (createSection && !existing.some(({ id }) => id === section)) { const added = sections.find(({ id }) => id === section); if (!added) throw new Error("Sidebar group no longer exists"); diff --git a/src/features/relay/sidebar-moves-store.test.ts b/src/features/relay/sidebar-moves-store.test.ts index 6af7ee65c3..aeafdbe7d6 100644 --- a/src/features/relay/sidebar-moves-store.test.ts +++ b/src/features/relay/sidebar-moves-store.test.ts @@ -31,8 +31,10 @@ async function setup() { const assignment = vi.fn(async (intent: SidebarAssignmentIntent) => { const id = intent.createSection?.id ?? intent.sectionId; const assignments = { ...data.assignments }; - if (id) assignments[intent.channelId] = id; - else delete assignments[intent.channelId]; + if (intent.channelId !== undefined) { + if (id) assignments[intent.channelId] = id; + else delete assignments[intent.channelId]; + } const sections = intent.createSection && !data.sections.some(({ id }) => id === intent.createSection?.id) @@ -559,3 +561,27 @@ it.each([false, true])( } }, ); + +it("creates an empty section through the queued writer without altering stars or assignments", async () => { + const h = await setup(); + const section = { + id: "12345678-1234-1234-1234-123456789abc", + name: "Launch", + }; + try { + expect(h.prefs.sectionCreationWritable).toBe(true); + await h.prefs.createSection(section); + expect(h.assignment).toHaveBeenCalledWith( + { createSection: section }, + expect.any(AbortSignal), + ); + expect(h.prefs.snapshot().data).toMatchObject({ + assignments: { alpha: "work", beta: "work" }, + starred: ["alpha"], + }); + expect(h.prefs.snapshot().data?.sections.at(-1)).toMatchObject(section); + expect(h.star).not.toHaveBeenCalled(); + } finally { + h.owner.dispose(); + } +}); diff --git a/src/features/relay/sidebar-preferences-store.test.ts b/src/features/relay/sidebar-preferences-store.test.ts index 8c26382cca..8922c782e9 100644 --- a/src/features/relay/sidebar-preferences-store.test.ts +++ b/src/features/relay/sidebar-preferences-store.test.ts @@ -47,8 +47,10 @@ function setup( write ?? (async ({ channelId, sectionId }) => { const assignments = { ...data.assignments }; - if (sectionId) assignments[channelId] = sectionId; - else delete assignments[channelId]; + if (channelId !== undefined) { + if (sectionId) assignments[channelId] = sectionId; + else delete assignments[channelId]; + } return { sections: data.sections, assignments }; }), writeSidebarStar: diff --git a/src/features/relay/sidebar-preferences-store.ts b/src/features/relay/sidebar-preferences-store.ts index 1005616ad4..8c08ab93e6 100644 --- a/src/features/relay/sidebar-preferences-store.ts +++ b/src/features/relay/sidebar-preferences-store.ts @@ -651,13 +651,21 @@ export function createSidebarPreferencesStore({ ); }, get sectionCreationWritable() { - return writable() && !snapshot.cached && !!createSection; + return writable() && !snapshot.cached && !!(createSection || write); }, createSection(section: Readonly<{ id: string; name: string }>) { - if (!writable() || snapshot.cached || !createSection) - return Promise.reject(new Error("Workspaces are unavailable")); - return writeGroups("Workspaces are unavailable", (signal) => - createSection(section, signal), + const create = + createSection ?? + (write + ? ( + section: Readonly<{ id: string; name: string }>, + signal: AbortSignal, + ) => write({ createSection: section }, signal) + : undefined); + if (!writable() || snapshot.cached || !create) + return Promise.reject(new Error("Section creation is unavailable")); + return writeGroups("Section creation is unavailable", (signal) => + create(section, signal), ); }, get sectionOrderWritable() { diff --git a/src/features/relay/sidebar-preferences.ts b/src/features/relay/sidebar-preferences.ts index 0eeb47b59f..fcb3932364 100644 --- a/src/features/relay/sidebar-preferences.ts +++ b/src/features/relay/sidebar-preferences.ts @@ -93,7 +93,8 @@ export type SidebarPreferences = SidebarGroups & activeWindowSeconds?: Readonly>; }>; export type SidebarAssignmentIntent = Readonly<{ - channelId: string; + /** Omit only when creating an empty section. */ + channelId?: string; sectionId?: string; createSection?: Readonly<{ id: string; name: string }>; }>; @@ -113,7 +114,7 @@ export type SidebarSectionOrderMutator = ( sectionIds: readonly string[], signal: AbortSignal, ) => Promise; -/** Adds an empty section; only Solo groups support this today. */ +/** Adds an empty section without assigning a conversation. */ export type SidebarSectionCreationMutator = ( section: Readonly<{ id: string; name: string }>, signal: AbortSignal, diff --git a/src/features/relay/solo-preferences.ts b/src/features/relay/solo-preferences.ts index fdfe39ce97..418d6c2bc4 100644 --- a/src/features/relay/solo-preferences.ts +++ b/src/features/relay/solo-preferences.ts @@ -132,11 +132,14 @@ export function createSoloPreferences(kit: ChannelKit, lifetime: AbortSignal) { if (target && !groups.some((group) => group.id === target)) throw new Error("The Solo group is unavailable"); const assignments = { ...value.assignments }; - if (target) assignments[intent.channelId] = target; - else delete assignments[intent.channelId]; + if (intent.channelId !== undefined) { + if (target) assignments[intent.channelId] = target; + else delete assignments[intent.channelId]; + } if ( groups === value.groups && - value.assignments[intent.channelId] === target + (intent.channelId === undefined || + value.assignments[intent.channelId] === target) ) return project(value); return save({ ...value, groups, assignments }, entry, active); diff --git a/tests/browser/navigation-groups.spec.mjs b/tests/browser/navigation-groups.spec.mjs index cdb6d423e2..c3b2f99a5f 100644 --- a/tests/browser/navigation-groups.spec.mjs +++ b/tests/browser/navigation-groups.spec.mjs @@ -208,9 +208,13 @@ test("row menu moves and removes a channel optimistically, retaining keyboard na .getByRole("button", { name: "More actions for Channels" }) .focus(); await page.keyboard.press("Enter"); - // Keyboard opening highlights the first item, Filter, asynchronously. - const filter = page.getByRole("menuitem", { name: "Filter", exact: true }); - await expect(filter).toHaveAttribute("data-highlighted", ""); + // Opening during a preference refresh can skip the disabled Create item. + // Establish keyboard ownership before moving to Sort in either case. + await expect( + page + .getByRole("menu", { name: "More actions for Channels", exact: true }) + .locator('[role="menuitem"][data-highlighted]'), + ).toBeVisible(); const sort = page.getByRole("menuitem", { name: "Sort", exact: true }); await sort.focus(); await page.keyboard.press("ArrowRight"); diff --git a/tests/browser/sidebar-section-creation.spec.mjs b/tests/browser/sidebar-section-creation.spec.mjs new file mode 100644 index 0000000000..ae437dbe66 --- /dev/null +++ b/tests/browser/sidebar-section-creation.spec.mjs @@ -0,0 +1,125 @@ +import { test, expect } from "./fixture.mjs"; +import { openPage } from "./navigation.mjs"; + +// Browser ownership: real pointer collision/activation, menu-to-modal focus, +// relocation and cold reload through the production preference broker. +// Input/error permutations remain in the colocated component/store tests. +test.use({ productionBroker: true, savedSidebar: true, largeSidebar: false }); + +test("Channels offers empty-section creation with keyboard focus and confirmed persistence", async ({ + page, + app, +}) => { + await page.goto(app.origin); + await openPage(page, "Messages"); + const trigger = page.getByRole("button", { + name: "More actions for Channels", + exact: true, + }); + await trigger.focus(); + await page.keyboard.press("Enter"); + await page + .getByRole("menuitem", { name: "Create section", exact: true }) + .click(); + const dialog = page.getByRole("dialog", { name: "Create new section" }); + const field = dialog.getByRole("textbox", { name: "Section name" }); + await expect(field).toBeFocused(); + await page.keyboard.press("Escape"); + await expect(trigger).toBeFocused(); + expect(app.report.sidebarPublications ?? []).toHaveLength(0); + await trigger.click(); + await page + .getByRole("menuitem", { name: "Create section", exact: true }) + .click(); + await expect(field).toBeFocused(); + await field.fill("Launch"); + await field.press("Enter"); + await expect(dialog).toHaveCount(0); + await expect(trigger).toBeFocused(); + await expect( + page.locator("summary").filter({ hasText: /^Launch$/ }), + ).toBeVisible(); + await expect.poll(() => app.report.sidebarPublications?.length ?? 0).toBe(1); + const { blob } = app.report.sidebarPublications[0]; + expect(blob.assignments).toEqual({ beta: "work" }); + expect(Object.keys(blob.meta.a)).not.toContain("undefined"); + await page.reload(); + await openPage(page, "Messages"); + await expect( + page.locator("summary").filter({ hasText: /^Launch$/ }), + ).toBeVisible(); + await expect( + page.locator( + '[data-sidebar-section="group:work"] [data-channel-id="beta"]', + ), + ).toBeVisible(); +}); + +async function startDrag(page, locator) { + const box = await locator.boundingBox(); + if (!box) throw new Error("Drag source missing"); + await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2); + await page.mouse.down(); + await page.mouse.move(box.x + box.width / 2 + 12, box.y + box.height / 2, { + steps: 3, + }); +} + +async function dropOnCreate(page) { + const target = page.locator("[data-create-section-drop]"); + await expect(target).toBeVisible(); + const box = await target.boundingBox(); + if (!box) throw new Error("Create target missing"); + await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2, { + steps: 4, + }); + await page.mouse.up(); +} + +test("channel drags reveal Create section, cancel safely, and create-and-move on submission", async ({ + page, + app, +}) => { + await page.goto(app.origin); + await openPage(page, "Messages"); + const target = page.locator("[data-create-section-drop]"); + const row = page.locator('[data-channel-id="alpha"]'); + await expect(target).toHaveCount(0); + await startDrag(page, row); + await expect(target).toBeVisible(); + await page.keyboard.press("Escape"); + await page.mouse.up(); + await expect(target).toHaveCount(0); + expect(app.report.sidebarPublications ?? []).toHaveLength(0); + await startDrag(page, row); + await dropOnCreate(page); + const dialog = page.getByRole("dialog", { name: "Create new section" }); + const field = dialog.getByRole("textbox", { name: "Section name" }); + await expect(field).toBeFocused(); + await dialog.getByRole("button", { name: "Cancel", exact: true }).click(); + await expect(row).toBeFocused(); + expect(app.report.sidebarPublications ?? []).toHaveLength(0); + await startDrag(page, row); + await dropOnCreate(page); + await expect(field).toBeFocused(); + await field.fill("Release"); + await dialog.getByRole("button", { name: "Create and move" }).click(); + await expect(dialog).toHaveCount(0); + const section = page + .locator("[data-sidebar-section]") + .filter({ has: page.locator("summary").filter({ hasText: /^Release$/ }) }); + await expect(section.locator('[data-channel-id="alpha"]')).toBeVisible(); + await expect(row).toBeFocused(); + await expect.poll(() => app.report.sidebarPublications?.length ?? 0).toBe(2); + await page.reload(); + await openPage(page, "Messages"); + await expect(section.locator('[data-channel-id="alpha"]')).toBeVisible(); + // The original per-channel creation entry remains available. + await row.click({ button: "right" }); + await page + .getByRole("menuitem", { name: "Move channel", exact: true }) + .hover(); + await expect( + page.getByRole("menuitem", { name: "Create new…", exact: true }), + ).toBeVisible(); +}); From d245c57b35c6aefca866b88b9a916206cab8a2bd Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Tue, 27 Oct 2026 08:59:44 -0700 Subject: [PATCH 02/21] fix(sidebar): refine section creation menu and spacing Co-authored-by: Carl Signed-off-by: Taylor Ho --- src/bundled/channels/CreateSidebarSection.tsx | 2 ++ src/bundled/channels/SidebarSection.tsx | 36 +++++++++---------- .../browser/sidebar-section-creation.spec.mjs | 5 +++ 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/bundled/channels/CreateSidebarSection.tsx b/src/bundled/channels/CreateSidebarSection.tsx index 669e44b5a1..f99a26141a 100644 --- a/src/bundled/channels/CreateSidebarSection.tsx +++ b/src/bundled/channels/CreateSidebarSection.tsx @@ -54,6 +54,8 @@ export function CreateSidebarSection({ return ( - {createSection && ( - <> - { - setMenuOpen(false); - if (actionsTrigger.current) - createSection.open(actionsTrigger.current); - }} - > - - - - Create section - - - - )} {filter && ( @@ -400,6 +382,24 @@ export function SidebarSection({ )} + {createSection && ( + <> + {(filter || sort) && } + { + setMenuOpen(false); + if (actionsTrigger.current) + createSection.open(actionsTrigger.current); + }} + > + + + + Create section + + + )} {move && ( <> Date: Tue, 27 Oct 2026 09:15:28 -0700 Subject: [PATCH 03/21] fix(sidebar): give section creation a labelled form Co-authored-by: Carl Signed-off-by: Taylor Ho --- .../channels/CreateSidebarSection.test.tsx | 3 ++- src/bundled/channels/CreateSidebarSection.tsx | 23 ++++++++++--------- tests/browser/navigation-group-icons.spec.mjs | 2 +- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/src/bundled/channels/CreateSidebarSection.test.tsx b/src/bundled/channels/CreateSidebarSection.test.tsx index 705af4a8a9..3a75cb6035 100644 --- a/src/bundled/channels/CreateSidebarSection.test.tsx +++ b/src/bundled/channels/CreateSidebarSection.test.tsx @@ -25,6 +25,7 @@ it("retains a rejected draft and its identity through preference recovery", asyn }; const view = render(); const input = screen.getByRole("textbox", { name: "Section name" }); + expect(screen.getByText("Section name", { selector: "label" })).toBeVisible(); fireEvent.change(input, { target: { value: " Launch " } }); const form = input.closest("form"); if (!form) throw new Error("Create form missing"); @@ -79,7 +80,7 @@ it("keeps an empty-section draft and identity through a failed save, blocking di ); const input = screen.getByRole("textbox", { name: "Section name" }); fireEvent.change(input, { target: { value: " Launch " } }); - const submit = screen.getByRole("button", { name: "Create section" }); + const submit = screen.getByRole("button", { name: "Create", exact: true }); fireEvent.click(submit); expect(input).toBeDisabled(); expect(screen.getByRole("button", { name: "Cancel" })).toBeDisabled(); diff --git a/src/bundled/channels/CreateSidebarSection.tsx b/src/bundled/channels/CreateSidebarSection.tsx index f99a26141a..b65b628849 100644 --- a/src/bundled/channels/CreateSidebarSection.tsx +++ b/src/bundled/channels/CreateSidebarSection.tsx @@ -2,6 +2,7 @@ import { useId, useRef, useState, type ComponentProps } from "react"; import { Button } from "../../shared/design-system/ui/Button"; import { Dialog } from "../../shared/design-system/ui/Dialog"; import { Input } from "../../shared/design-system/ui/Input"; +import { Field } from "../../shared/design-system/ui/Field"; import styles from "./CreateSidebarSection.module.css"; /** Mounted after the launching menu closes, so focus has one modal owner. */ @@ -85,7 +86,7 @@ export function CreateSidebarSection({ disabled={!name.trim()} loading={pending} > - {channelName ? "Create and move" : "Create section"} + {channelName ? "Create and move" : "Create"} } @@ -97,16 +98,16 @@ export function CreateSidebarSection({ void submit(); }} > - setName(event.target.value)} - /> + + setName(event.target.value)} + /> + {error &&

{error}

} {blocked && !writable && ( diff --git a/tests/browser/navigation-group-icons.spec.mjs b/tests/browser/navigation-group-icons.spec.mjs index b92d13d2af..857721fd2b 100644 --- a/tests/browser/navigation-group-icons.spec.mjs +++ b/tests/browser/navigation-group-icons.spec.mjs @@ -173,7 +173,7 @@ test("group icons resolve custom media without overlapping labels, and the creat exact: true, }); await expect(field).toBeFocused(); - await expect(dialog.locator("label")).toHaveCount(0); + await expect(dialog.locator("label")).toHaveText("Section name"); await expect(dialog.locator("p")).toHaveText( `Move ${channelName} into a new section.`, ); From 3b7478b4cd0d578814bf4de680afb3aa246ee27b Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Tue, 27 Oct 2026 09:20:26 -0700 Subject: [PATCH 04/21] fix(sidebar): remove the section name label Co-authored-by: Carl Signed-off-by: Taylor Ho --- .../channels/CreateSidebarSection.test.tsx | 2 +- src/bundled/channels/CreateSidebarSection.tsx | 21 +++++++++---------- tests/browser/navigation-group-icons.spec.mjs | 2 +- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/bundled/channels/CreateSidebarSection.test.tsx b/src/bundled/channels/CreateSidebarSection.test.tsx index 3a75cb6035..950abce4cc 100644 --- a/src/bundled/channels/CreateSidebarSection.test.tsx +++ b/src/bundled/channels/CreateSidebarSection.test.tsx @@ -25,7 +25,7 @@ it("retains a rejected draft and its identity through preference recovery", asyn }; const view = render(); const input = screen.getByRole("textbox", { name: "Section name" }); - expect(screen.getByText("Section name", { selector: "label" })).toBeVisible(); + expect(input).toHaveAttribute("placeholder", "Section name"); fireEvent.change(input, { target: { value: " Launch " } }); const form = input.closest("form"); if (!form) throw new Error("Create form missing"); diff --git a/src/bundled/channels/CreateSidebarSection.tsx b/src/bundled/channels/CreateSidebarSection.tsx index b65b628849..50c92ad614 100644 --- a/src/bundled/channels/CreateSidebarSection.tsx +++ b/src/bundled/channels/CreateSidebarSection.tsx @@ -2,7 +2,6 @@ import { useId, useRef, useState, type ComponentProps } from "react"; import { Button } from "../../shared/design-system/ui/Button"; import { Dialog } from "../../shared/design-system/ui/Dialog"; import { Input } from "../../shared/design-system/ui/Input"; -import { Field } from "../../shared/design-system/ui/Field"; import styles from "./CreateSidebarSection.module.css"; /** Mounted after the launching menu closes, so focus has one modal owner. */ @@ -98,16 +97,16 @@ export function CreateSidebarSection({ void submit(); }} > - - setName(event.target.value)} - /> - + setName(event.target.value)} + /> {error &&

{error}

} {blocked && !writable && ( diff --git a/tests/browser/navigation-group-icons.spec.mjs b/tests/browser/navigation-group-icons.spec.mjs index 857721fd2b..b92d13d2af 100644 --- a/tests/browser/navigation-group-icons.spec.mjs +++ b/tests/browser/navigation-group-icons.spec.mjs @@ -173,7 +173,7 @@ test("group icons resolve custom media without overlapping labels, and the creat exact: true, }); await expect(field).toBeFocused(); - await expect(dialog.locator("label")).toHaveText("Section name"); + await expect(dialog.locator("label")).toHaveCount(0); await expect(dialog.locator("p")).toHaveText( `Move ${channelName} into a new section.`, ); From 954f1432e28058c2743abcbac3609c6766abcf24 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Tue, 27 Oct 2026 11:11:17 -0700 Subject: [PATCH 05/21] feat(sidebar): reveal new section drop zone above channels Expand the Channels boundary with Motion when a channel enters it, retain the destination through the drag, and preserve create-and-move confirmation. Co-authored-by: Carl Signed-off-by: Taylor Ho --- docs/channels.md | 6 +- src/bundled/channels/Channels.module.css | 38 ++++- .../channels/CreateSidebarSection.test.tsx | 2 +- .../channel-navigation/ChannelSidebarDnd.tsx | 136 +++++++++++------- .../browser/sidebar-section-creation.spec.mjs | 95 +++++++++++- 5 files changed, 213 insertions(+), 64 deletions(-) diff --git a/docs/channels.md b/docs/channels.md index 2ff96094ca..d7a461ff11 100644 --- a/docs/channels.md +++ b/docs/channels.md @@ -153,8 +153,10 @@ can also be dragged with a pointer onto another saved group, **Starred** or placement, rollback notice and Retry. The **Channels** ellipsis menu also offers **Create section**, which creates an empty section without moving a channel. Its dialog retains the name and section -ID on failure and waits for confirmation before closing. While dragging a channel, -a **Create section** drop target stays at the bottom of the visible sidebar. +ID on failure and waits for confirmation before closing. Dragging a channel into +the gap immediately above **Channels** expands an **Add to new section** drop +target inside the sidebar. It stays revealed for the rest of that drag, with +reduced motion opening it immediately; other section destinations remain available. Dropping there opens the existing create-and-move dialog; cancelling leaves the channel in place, and no preference is written until submission. These entry points supplement the per-channel **Move channel → Create new…** route and use diff --git a/src/bundled/channels/Channels.module.css b/src/bundled/channels/Channels.module.css index ac6f10c432..9aacbac89f 100644 --- a/src/bundled/channels/Channels.module.css +++ b/src/bundled/channels/Channels.module.css @@ -296,14 +296,36 @@ border-radius: var(--radius-row); background: var(--affordance-panel-hover); } +.createSectionSlot + .channelSection { + margin-top: 0; +} +.createSectionHitArea { + position: absolute; + inset: calc(-1 * var(--space-4)) 0 0; + pointer-events: none; +} +.createSectionReveal { + overflow: hidden; +} .createSectionDrop { - position: sticky; - bottom: 0; - z-index: 1; - margin-top: var(--space-2); + display: flex; + align-items: center; + gap: var(--space-2); + min-height: var(--size-control-lg); + margin-bottom: var(--space-4); + padding: var(--space-2); border: 1px dashed var(--border-standard); border-radius: var(--radius-row); background: var(--surface-panel); + color: var(--text-subtle); +} +.createSectionDrop > svg { + flex-shrink: 0; +} +.createSectionDrop[data-drop-target] { + border-color: var(--border-prominent); + background: var(--affordance-panel-hover); + color: var(--text-standard); } [data-channel-dragging], .channelSection[data-section-dragging] { @@ -328,8 +350,8 @@ transform: translateY(50%); } .channelDragOverlay { - /* Keep the destination label visible beneath the pointer. */ - translate: var(--space-3) var(--space-6); + /* The new-section slot opens below the gap where the pointer first enters. */ + translate: var(--space-3) calc(-100% - var(--space-3)); display: flex; align-items: center; gap: var(--space-2); @@ -350,6 +372,10 @@ text-overflow: ellipsis; white-space: nowrap; } +/* The highlighted destination replaces the preview while it accepts the drop. */ +body:has([data-create-section-drop][data-drop-target]) .channelDragOverlay { + visibility: hidden; +} .sectionHeader { position: relative; } diff --git a/src/bundled/channels/CreateSidebarSection.test.tsx b/src/bundled/channels/CreateSidebarSection.test.tsx index 950abce4cc..75f128b89c 100644 --- a/src/bundled/channels/CreateSidebarSection.test.tsx +++ b/src/bundled/channels/CreateSidebarSection.test.tsx @@ -80,7 +80,7 @@ it("keeps an empty-section draft and identity through a failed save, blocking di ); const input = screen.getByRole("textbox", { name: "Section name" }); fireEvent.change(input, { target: { value: " Launch " } }); - const submit = screen.getByRole("button", { name: "Create", exact: true }); + const submit = screen.getByRole("button", { name: "Create" }); fireEvent.click(submit); expect(input).toBeDisabled(); expect(screen.getByRole("button", { name: "Cancel" })).toBeDisabled(); diff --git a/src/features/channel-navigation/ChannelSidebarDnd.tsx b/src/features/channel-navigation/ChannelSidebarDnd.tsx index 57e5483caf..8779ce01d1 100644 --- a/src/features/channel-navigation/ChannelSidebarDnd.tsx +++ b/src/features/channel-navigation/ChannelSidebarDnd.tsx @@ -4,6 +4,7 @@ import { PointerSensor, type CollisionDetection, pointerWithin, + useDndContext, useDraggable, useDndMonitor, useDroppable, @@ -20,8 +21,8 @@ import { type ReactNode, } from "react"; import { createPortal } from "react-dom"; +import { motion, useReducedMotion } from "motion/react"; import { SidebarSection } from "../../bundled/channels/SidebarSection"; -import { NavigationItem } from "../../shared/design-system/ui/NavigationItem"; import { PlusIcon } from "../../shared/design-system/icons"; import styles from "../../bundled/channels/Channels.module.css"; @@ -124,10 +125,7 @@ export function ChannelSidebarDnd({ onMove(String(active.id), String(over.id)); }} > - - {children} - {onCreateSection && } - + {children} {createPortal( // The optimistic move relocates the row, so nothing remains to settle back onto. @@ -139,24 +137,57 @@ export function ChannelSidebarDnd({ ); } -function CreateSectionDropTarget({ disabled }: { disabled: boolean }) { - const { setNodeRef, isOver, active } = useDroppable({ +function CreateSectionDropTarget() { + const { active } = useDndContext(); + const enabled = + useContext(Writable) && !!active && !active.data.current?.group; + const reducedMotion = useReducedMotion(); + const [revealed, setRevealed] = useState(false); + const { setNodeRef, isOver } = useDroppable({ id: CREATE_SECTION, - disabled, + disabled: !enabled, + resizeObserverConfig: { updateMeasurementsFor: [] }, }); - if (disabled || !active || active.data.current?.group) return null; + useDndMonitor({ + onDragStart: () => setRevealed(false), + onDragOver: ({ over }) => { + if (enabled && over?.id === CREATE_SECTION) setRevealed(true); + }, + onDragEnd: () => setRevealed(false), + onDragCancel: () => setRevealed(false), + }); + const open = enabled && revealed; return (
- } - highlighted={isOver} - tabIndex={-1} + {/* The original gap stays inside the target throughout expansion. */} +
+ +
+ + Add to new section +
+
); } @@ -250,39 +281,44 @@ export function DroppableSidebarSection({ const group = !!active?.data.current?.group; const from = active?.data.current?.groupIndex; return ( - { - handle.current = node; - drag.setNodeRef(node); - }, - listeners: { - ...drag.listeners, - onPointerDown: (event) => { - suppressClick.current = false; - drag.listeners?.onPointerDown?.(event); + <> + {section.sectionKey === "channels" && section.createSection && ( + + )} + { + handle.current = node; + drag.setNodeRef(node); }, - }, - } - : undefined - } - dragging={drag.isDragging} - /> + listeners: { + ...drag.listeners, + onPointerDown: (event) => { + suppressClick.current = false; + drag.listeners?.onPointerDown?.(event); + }, + }, + } + : undefined + } + dragging={drag.isDragging} + /> + ); } diff --git a/tests/browser/sidebar-section-creation.spec.mjs b/tests/browser/sidebar-section-creation.spec.mjs index 8addbea747..f1dca6b810 100644 --- a/tests/browser/sidebar-section-creation.spec.mjs +++ b/tests/browser/sidebar-section-creation.spec.mjs @@ -1,5 +1,5 @@ import { test, expect } from "./fixture.mjs"; -import { openPage } from "./navigation.mjs"; +import { openPage, settleShellToggle } from "./navigation.mjs"; // Browser ownership: real pointer collision/activation, menu-to-modal focus, // relocation and cold reload through the production preference broker. @@ -61,6 +61,7 @@ test("Channels offers empty-section creation with keyboard focus and confirmed p }); async function startDrag(page, locator) { + await expect(locator).toBeVisible(); const box = await locator.boundingBox(); if (!box) throw new Error("Drag source missing"); await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2); @@ -71,17 +72,36 @@ async function startDrag(page, locator) { } async function dropOnCreate(page) { + await revealCreate(page); const target = page.locator("[data-create-section-drop]"); - await expect(target).toBeVisible(); const box = await target.boundingBox(); if (!box) throw new Error("Create target missing"); - await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2, { + await page.mouse.move(box.x + box.width / 2, box.y + box.height - 2, { steps: 4, }); + await expect(page.locator('[class*="channelDragOverlay"]')).toBeHidden(); await page.mouse.up(); } -test("channel drags reveal Create section, cancel safely, and create-and-move on submission", async ({ +async function revealCreate(page) { + const trigger = page.locator("[data-create-section-trigger]"); + const box = await trigger.boundingBox(); + if (!box) throw new Error("Create boundary missing"); + await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2, { + steps: 4, + }); + await expect(page.locator("[data-create-section-drop]")).toBeVisible(); + // Observe completion rather than releasing into still-expanding geometry. + await expect + .poll(() => + page + .locator("[data-create-section-drop]") + .evaluate((node) => node.parentElement.style.height), + ) + .toBe("auto"); +} + +test("only the Channels boundary reveals new-section creation, latches, cancels and saves", async ({ page, app, }) => { @@ -91,13 +111,34 @@ test("channel drags reveal Create section, cancel safely, and create-and-move on const row = page.locator('[data-channel-id="alpha"]'); await expect(target).toHaveCount(0); await startDrag(page, row); + await expect(target).toHaveCount(0); + await revealCreate(page); + // Leaving the gap must not collapse it or prevent an ordinary section move. + const work = page.locator('[data-sidebar-section="group:work"] summary'); + const workBox = await work.boundingBox(); + if (!workBox) throw new Error("Work destination missing"); + await page.mouse.move( + workBox.x + workBox.width / 2, + workBox.y + workBox.height / 2, + ); await expect(target).toBeVisible(); + await expect( + page.locator('[data-sidebar-section="group:work"]'), + ).toHaveAttribute("data-drop-target", "true"); await page.keyboard.press("Escape"); await page.mouse.up(); await expect(target).toHaveCount(0); expect(app.report.sidebarPublications ?? []).toHaveLength(0); + await startDrag(page, work); + await expect(target).toHaveCount(0); + await page.keyboard.press("Escape"); + await page.mouse.up(); + // Release in the original gap immediately: animation must not move it away. await startDrag(page, row); - await dropOnCreate(page); + const gap = await page.locator("[data-create-section-trigger]").boundingBox(); + if (!gap) throw new Error("Create boundary missing"); + await page.mouse.move(gap.x + gap.width / 2, gap.y + gap.height / 2); + await page.mouse.up(); const dialog = page.getByRole("dialog", { name: "Create new section" }); const field = dialog.getByRole("textbox", { name: "Section name" }); await expect(field).toBeFocused(); @@ -128,3 +169,47 @@ test("channel drags reveal Create section, cancel safely, and create-and-move on page.getByRole("menuitem", { name: "Create new…", exact: true }), ).toBeVisible(); }); + +test.describe("without saved sections", () => { + test.use({ savedSidebar: false }); + + // Only a layout engine proves that the first-section gap is reachable in the + // host sidebar, including its scrollport, enlarged text and immediate motion. + test("the first Channels boundary works with reduced motion in a narrow sidebar", async ({ + page, + app, + }) => { + await page.emulateMedia({ reducedMotion: "reduce", colorScheme: "light" }); + await page.setViewportSize({ width: 480, height: 800 }); + await page.goto(app.origin); + await page.evaluate(() => { + localStorage.setItem("buzz-font-scale.v1", "2"); + window.dispatchEvent( + new StorageEvent("storage", { + key: "buzz-font-scale.v1", + newValue: "2", + storageArea: localStorage, + }), + ); + }); + await openPage(page, "Messages"); + await settleShellToggle(page); + await page + .getByRole("button", { name: "Show navigation", exact: true }) + .click(); + await expect(page.locator('[data-sidebar-section="starred"]')).toHaveCount( + 0, + ); + await expect(page.locator('[data-sidebar-section^="group:"]')).toHaveCount( + 0, + ); + await startDrag(page, page.locator('[data-channel-id="alpha"]')); + await dropOnCreate(page); + const dialog = page.getByRole("dialog", { name: "Create new section" }); + await expect( + dialog.getByRole("textbox", { name: "Section name" }), + ).toBeFocused(); + await dialog.getByRole("button", { name: "Cancel", exact: true }).click(); + expect(app.report.sidebarPublications ?? []).toHaveLength(0); + }); +}); From 6725e414d7a0b2981fb162ebe63bfced8fe884d2 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Tue, 27 Oct 2026 11:36:12 -0700 Subject: [PATCH 06/21] fix(sidebar): align section drop cue with image uploads Use a centered icon and label, inset surface and a clear active boundary while preserving the existing animated reveal and create-and-move flow. Co-authored-by: Carl Signed-off-by: Taylor Ho --- src/bundled/channels/Channels.module.css | 23 ++++++++++++++----- .../channel-navigation/ChannelSidebarDnd.tsx | 2 +- .../browser/sidebar-section-creation.spec.mjs | 8 +++---- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/bundled/channels/Channels.module.css b/src/bundled/channels/Channels.module.css index 9aacbac89f..cba614e9d3 100644 --- a/src/bundled/channels/Channels.module.css +++ b/src/bundled/channels/Channels.module.css @@ -309,24 +309,35 @@ } .createSectionDrop { display: flex; + flex-direction: column; align-items: center; + justify-content: center; gap: var(--space-2); - min-height: var(--size-control-lg); + /* Match the avatar uploader's compact 80px drop area at the default scale. */ + min-height: calc(5 * var(--space-4)); margin-bottom: var(--space-4); padding: var(--space-2); - border: 1px dashed var(--border-standard); - border-radius: var(--radius-row); - background: var(--surface-panel); + border: 2px solid transparent; + border-radius: var(--radius-control); + background: var(--surface-inset); color: var(--text-subtle); + text-align: center; + transition: background-color var(--duration-state) var(--easing-state); } .createSectionDrop > svg { flex-shrink: 0; } .createSectionDrop[data-drop-target] { - border-color: var(--border-prominent); - background: var(--affordance-panel-hover); + border-color: var(--text-standard); + border-style: dashed; + background: var(--affordance-subtle-hover); color: var(--text-standard); } +@media (prefers-reduced-motion: reduce) { + .createSectionDrop { + transition: none; + } +} [data-channel-dragging], .channelSection[data-section-dragging] { opacity: 0.3; diff --git a/src/features/channel-navigation/ChannelSidebarDnd.tsx b/src/features/channel-navigation/ChannelSidebarDnd.tsx index 8779ce01d1..62477d1e40 100644 --- a/src/features/channel-navigation/ChannelSidebarDnd.tsx +++ b/src/features/channel-navigation/ChannelSidebarDnd.tsx @@ -184,7 +184,7 @@ function CreateSectionDropTarget() { data-create-section-drop={open ? "" : undefined} data-drop-target={isOver || undefined} > - + Add to new section
diff --git a/tests/browser/sidebar-section-creation.spec.mjs b/tests/browser/sidebar-section-creation.spec.mjs index f1dca6b810..47eb7bfcad 100644 --- a/tests/browser/sidebar-section-creation.spec.mjs +++ b/tests/browser/sidebar-section-creation.spec.mjs @@ -74,11 +74,9 @@ async function startDrag(page, locator) { async function dropOnCreate(page) { await revealCreate(page); const target = page.locator("[data-create-section-drop]"); - const box = await target.boundingBox(); - if (!box) throw new Error("Create target missing"); - await page.mouse.move(box.x + box.width / 2, box.y + box.height - 2, { - steps: 4, - }); + // Bring the actual target into view before releasing; at enlarged text its + // lower edge can sit outside the narrow sidebar's scrollport. + await target.hover(); await expect(page.locator('[class*="channelDragOverlay"]')).toBeHidden(); await page.mouse.up(); } From 643890c777e2179277fe087c346898735a8b718c Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Tue, 27 Oct 2026 11:46:54 -0700 Subject: [PATCH 07/21] fix(sidebar): tighten the section drop cue label Use the shared caption type role and reduce the icon-to-label gap to the 4px spacing token. Co-authored-by: Carl Signed-off-by: Taylor Ho --- src/bundled/channels/Channels.module.css | 2 +- src/features/channel-navigation/ChannelSidebarDnd.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bundled/channels/Channels.module.css b/src/bundled/channels/Channels.module.css index cba614e9d3..46da9db399 100644 --- a/src/bundled/channels/Channels.module.css +++ b/src/bundled/channels/Channels.module.css @@ -312,7 +312,7 @@ flex-direction: column; align-items: center; justify-content: center; - gap: var(--space-2); + gap: var(--space-1); /* Match the avatar uploader's compact 80px drop area at the default scale. */ min-height: calc(5 * var(--space-4)); margin-bottom: var(--space-4); diff --git a/src/features/channel-navigation/ChannelSidebarDnd.tsx b/src/features/channel-navigation/ChannelSidebarDnd.tsx index 62477d1e40..45d98dd167 100644 --- a/src/features/channel-navigation/ChannelSidebarDnd.tsx +++ b/src/features/channel-navigation/ChannelSidebarDnd.tsx @@ -180,7 +180,7 @@ function CreateSectionDropTarget() { inert={!open} >
From b2d6e5cbfac072fe34b453745fe4a9039bc88796 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Tue, 27 Oct 2026 12:04:17 -0700 Subject: [PATCH 08/21] fix(sidebar): clarify the new-section move dialog Use Move to new section for channel moves and remove the repeated description. Verify initial field focus and menu handoff with the existing dialog focus target. Co-authored-by: Carl Signed-off-by: Taylor Ho --- .../channels/CreateSidebarSection.module.css | 11 ------- .../channels/CreateSidebarSection.test.tsx | 8 +++++ src/bundled/channels/CreateSidebarSection.tsx | 11 +------ .../ChannelInventory.test.tsx | 2 +- tests/browser/channel-header-menu.spec.mjs | 4 +-- tests/browser/navigation-group-icons.spec.mjs | 31 ++++++++++--------- tests/browser/navigation-groups.spec.mjs | 8 ++--- .../browser/sidebar-section-creation.spec.mjs | 17 +++++++--- 8 files changed, 45 insertions(+), 47 deletions(-) delete mode 100644 src/bundled/channels/CreateSidebarSection.module.css diff --git a/src/bundled/channels/CreateSidebarSection.module.css b/src/bundled/channels/CreateSidebarSection.module.css deleted file mode 100644 index f6cb823ae0..0000000000 --- a/src/bundled/channels/CreateSidebarSection.module.css +++ /dev/null @@ -1,11 +0,0 @@ -.description { - display: flex; - gap: var(--space-1); - min-width: 0; - white-space: nowrap; -} -.description > span { - min-width: 0; - overflow: hidden; - text-overflow: ellipsis; -} diff --git a/src/bundled/channels/CreateSidebarSection.test.tsx b/src/bundled/channels/CreateSidebarSection.test.tsx index 75f128b89c..da646b23b7 100644 --- a/src/bundled/channels/CreateSidebarSection.test.tsx +++ b/src/bundled/channels/CreateSidebarSection.test.tsx @@ -25,6 +25,10 @@ it("retains a rejected draft and its identity through preference recovery", asyn }; const view = render(); const input = screen.getByRole("textbox", { name: "Section name" }); + expect( + screen.getByRole("dialog", { name: "Move to new section" }), + ).not.toHaveAccessibleDescription(); + await waitFor(() => expect(input).toHaveFocus()); expect(input).toHaveAttribute("placeholder", "Section name"); fireEvent.change(input, { target: { value: " Launch " } }); const form = input.closest("form"); @@ -79,6 +83,10 @@ it("keeps an empty-section draft and identity through a failed save, blocking di />, ); const input = screen.getByRole("textbox", { name: "Section name" }); + expect( + screen.getByRole("dialog", { name: "Create new section" }), + ).toBeInTheDocument(); + await waitFor(() => expect(input).toHaveFocus()); fireEvent.change(input, { target: { value: " Launch " } }); const submit = screen.getByRole("button", { name: "Create" }); fireEvent.click(submit); diff --git a/src/bundled/channels/CreateSidebarSection.tsx b/src/bundled/channels/CreateSidebarSection.tsx index 50c92ad614..e0a4965590 100644 --- a/src/bundled/channels/CreateSidebarSection.tsx +++ b/src/bundled/channels/CreateSidebarSection.tsx @@ -2,7 +2,6 @@ import { useId, useRef, useState, type ComponentProps } from "react"; import { Button } from "../../shared/design-system/ui/Button"; import { Dialog } from "../../shared/design-system/ui/Dialog"; import { Input } from "../../shared/design-system/ui/Input"; -import styles from "./CreateSidebarSection.module.css"; /** Mounted after the launching menu closes, so focus has one modal owner. */ export function CreateSidebarSection({ @@ -62,15 +61,7 @@ export function CreateSidebarSection({ onOpenChange={(open) => { if (!open) close(); }} - title="Create new section" - description={ - channelName ? ( - - Move {channelName} into a new - section. - - ) : undefined - } + title={channelName ? "Move to new section" : "Create new section"} initialFocus={input} finalFocus={finalFocus} actions={ diff --git a/src/features/channel-navigation/ChannelInventory.test.tsx b/src/features/channel-navigation/ChannelInventory.test.tsx index ae14285e5a..8b378cca1e 100644 --- a/src/features/channel-navigation/ChannelInventory.test.tsx +++ b/src/features/channel-navigation/ChannelInventory.test.tsx @@ -1183,7 +1183,7 @@ it("rejects a deferred header Create section after its navigation origin retires for (const frame of frames.splice(0)) frame(0); }); expect( - screen.queryByRole("dialog", { name: "Create new section" }), + screen.queryByRole("dialog", { name: "Move to new section" }), ).not.toBeInTheDocument(); expect(preferences.queries.snapshot().data?.sections).toEqual([]); preferences.dispose(); diff --git a/tests/browser/channel-header-menu.spec.mjs b/tests/browser/channel-header-menu.spec.mjs index d450956564..3ca46bb1b2 100644 --- a/tests/browser/channel-header-menu.spec.mjs +++ b/tests/browser/channel-header-menu.spec.mjs @@ -434,7 +434,7 @@ test.describe("sidebar actions in the header", () => { .getByRole("menuitem", { name: "Create new…", exact: true }) .click(); const create = page.getByRole("dialog", { - name: "Create new section", + name: "Move to new section", exact: true, }); await expect( @@ -576,7 +576,7 @@ test.describe("dialog origin retirement", () => { .getByRole("menuitem", { name: "Delete channel", exact: true }) .click(); const dialog = page.getByRole("dialog", { - name: action === "create" ? "Create new section" : /^Delete .+\?$/, + name: action === "create" ? "Move to new section" : /^Delete .+\?$/, }); await expect(dialog).toBeVisible(); await back(page); diff --git a/tests/browser/navigation-group-icons.spec.mjs b/tests/browser/navigation-group-icons.spec.mjs index b92d13d2af..ac9a60b414 100644 --- a/tests/browser/navigation-group-icons.spec.mjs +++ b/tests/browser/navigation-group-icons.spec.mjs @@ -10,8 +10,8 @@ test.use({ }); // Real catalog → session media URL → sidebar/menu image; browser layout owns -// loading placeholders and single-line dialog truncation, not a DOM emulator. -test("group icons resolve custom media without overlapping labels, and the create dialog stays one line", async ({ +// loading placeholders and narrow-dialog geometry, not a DOM emulator. +test("group icons resolve custom media without overlapping labels, and the move dialog fits narrow widths", async ({ page, app, }, testInfo) => { @@ -167,33 +167,34 @@ test("group icons resolve custom media without overlapping labels, and the creat await menu .getByRole("menuitem", { name: "Create new…", exact: true }) .click(); - const dialog = page.getByRole("dialog", { name: "Create new section" }); + const dialog = page.getByRole("dialog", { name: "Move to new section" }); const field = dialog.getByRole("textbox", { name: "Section name", exact: true, }); await expect(field).toBeFocused(); + await expect(page.getByRole("menu")).toHaveCount(0); + await dialog.evaluate(async (element) => { + await Promise.allSettled( + element.getAnimations().map((animation) => animation.finished), + ); + }); + await expect(field).toBeFocused(); await expect(dialog.locator("label")).toHaveCount(0); - await expect(dialog.locator("p")).toHaveText( - `Move ${channelName} into a new section.`, - ); + await expect(dialog).not.toHaveAttribute("aria-describedby"); for (const width of [1440, 360]) { await page.setViewportSize({ width, height: 950 }); - const layout = await dialog.locator("p").evaluate((element) => { - const name = element.querySelector("span[title]"); - const range = document.createRange(); - range.selectNodeContents(name); + const layout = await dialog.evaluate((element) => { const box = element.getBoundingClientRect(); return { - height: box.height, - nameHeight: range.getBoundingClientRect().height, width: element.clientWidth, scrollWidth: element.scrollWidth, + left: box.left, right: box.right, viewport: window.innerWidth, }; }); - expect(layout.height).toBeLessThan(layout.nameHeight * 2); + expect(layout.left).toBeGreaterThanOrEqual(0); expect(layout.scrollWidth).toBeLessThanOrEqual(layout.width); expect(layout.right).toBeLessThanOrEqual(layout.viewport); } @@ -202,7 +203,7 @@ test("group icons resolve custom media without overlapping labels, and the creat }); await page.setViewportSize({ width: 1440, height: 950 }); await dialog.screenshot({ - path: testInfo.outputPath("create-section-single-line.png"), + path: testInfo.outputPath("move-section-wide.png"), }); await page.keyboard.press("Escape"); await expect(row).toBeFocused(); @@ -232,7 +233,7 @@ test("section labels fade on overflow and recover when widened", async ({ await page .getByRole("menuitem", { name: "Create new…", exact: true }) .click(); - const dialog = page.getByRole("dialog", { name: "Create new section" }); + const dialog = page.getByRole("dialog", { name: "Move to new section" }); const name = "Interface gang collaboration"; const field = dialog.getByRole("textbox", { name: "Section name" }); // The menu-to-dialog handoff owns initial focus; mounting alone does not diff --git a/tests/browser/navigation-groups.spec.mjs b/tests/browser/navigation-groups.spec.mjs index c3b2f99a5f..24ce7e3167 100644 --- a/tests/browser/navigation-groups.spec.mjs +++ b/tests/browser/navigation-groups.spec.mjs @@ -699,9 +699,9 @@ test("Create new supports cancel, moves before publication, and retries the same await expect(beta).toBeVisible(); await openMove(page, beta); await page.getByRole("menuitem", { name: "Create new…" }).click(); - const dialog = page.getByRole("dialog", { name: "Create new section" }); + const dialog = page.getByRole("dialog", { name: "Move to new section" }); const field = dialog.getByRole("textbox", { name: "Section name" }); - await expect(dialog.locator("p")).toHaveText("Move Beta into a new section."); + await expect(dialog).not.toHaveAttribute("aria-describedby"); await expect(dialog.locator("label")).toHaveCount(0); await expect(field).toBeFocused(); await expect(page.getByRole("menu")).toHaveCount(0); @@ -878,7 +878,7 @@ test("Create new retains its draft when preferences fail before submission and r await started.promise; await openMove(page, beta); await page.getByRole("menuitem", { name: "Create new…" }).click(); - const dialog = page.getByRole("dialog", { name: "Create new section" }); + const dialog = page.getByRole("dialog", { name: "Move to new section" }); const field = dialog.getByRole("textbox", { name: "Section name" }); await field.fill("Retained launch"); const failed = page.waitForResponse( @@ -1065,7 +1065,7 @@ test("Move and Create stay available on Workflows", async ({ page, app }) => { await menu .getByRole("menuitem", { name: "Create new…", exact: true }) .click(); - const dialog = page.getByRole("dialog", { name: "Create new section" }); + const dialog = page.getByRole("dialog", { name: "Move to new section" }); await dialog.getByRole("textbox").fill("Off-page group"); await dialog .getByRole("button", { name: "Create and move", exact: true }) diff --git a/tests/browser/sidebar-section-creation.spec.mjs b/tests/browser/sidebar-section-creation.spec.mjs index 47eb7bfcad..7aad2884a7 100644 --- a/tests/browser/sidebar-section-creation.spec.mjs +++ b/tests/browser/sidebar-section-creation.spec.mjs @@ -37,7 +37,9 @@ test("Channels offers empty-section creation with keyboard focus and confirmed p .getByRole("menuitem", { name: "Create section", exact: true }) .click(); await expect(field).toBeFocused(); - await field.fill("Launch"); + await expect(page.getByRole("menu")).toHaveCount(0); + await page.keyboard.type("Launch"); + await expect(field).toHaveValue("Launch"); await field.press("Enter"); await expect(dialog).toHaveCount(0); await expect(trigger).toBeFocused(); @@ -137,7 +139,7 @@ test("only the Channels boundary reveals new-section creation, latches, cancels if (!gap) throw new Error("Create boundary missing"); await page.mouse.move(gap.x + gap.width / 2, gap.y + gap.height / 2); await page.mouse.up(); - const dialog = page.getByRole("dialog", { name: "Create new section" }); + const dialog = page.getByRole("dialog", { name: "Move to new section" }); const field = dialog.getByRole("textbox", { name: "Section name" }); await expect(field).toBeFocused(); await dialog.getByRole("button", { name: "Cancel", exact: true }).click(); @@ -146,7 +148,14 @@ test("only the Channels boundary reveals new-section creation, latches, cancels await startDrag(page, row); await dropOnCreate(page); await expect(field).toBeFocused(); - await field.fill("Release"); + await dialog.evaluate(async (element) => { + await Promise.allSettled( + element.getAnimations().map((animation) => animation.finished), + ); + }); + await expect(field).toBeFocused(); + await page.keyboard.type("Release"); + await expect(field).toHaveValue("Release"); await dialog.getByRole("button", { name: "Create and move" }).click(); await expect(dialog).toHaveCount(0); const section = page @@ -203,7 +212,7 @@ test.describe("without saved sections", () => { ); await startDrag(page, page.locator('[data-channel-id="alpha"]')); await dropOnCreate(page); - const dialog = page.getByRole("dialog", { name: "Create new section" }); + const dialog = page.getByRole("dialog", { name: "Move to new section" }); await expect( dialog.getByRole("textbox", { name: "Section name" }), ).toBeFocused(); From 4b3a0d418761852adfed4dbbe317fa84ff6ea195 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Tue, 27 Oct 2026 12:35:23 -0700 Subject: [PATCH 09/21] fix(sidebar): reveal section creation above the first section Anchor the drag target before the first custom channel section, falling back to Channels. Create the dropped channel's section at that same position in one assignment write; preserve menu creation order. Co-authored-by: Carl Signed-off-by: Taylor Ho --- browser-host/sidebar-create-section.test.mjs | 28 +++++++ browser-host/sidebar-preferences.mjs | 2 +- docs/channels.md | 6 +- .../ChannelInventory.test.tsx | 10 +++ .../channel-navigation/ChannelInventory.tsx | 14 +++- .../channel-navigation/ChannelSidebarDnd.tsx | 6 +- src/features/relay/native-sidebar.test.ts | 19 +++++ src/features/relay/sidebar-edits.test.ts | 6 ++ src/features/relay/sidebar-edits.ts | 6 +- .../relay/sidebar-moves-store.test.ts | 82 +++++++++++-------- .../relay/sidebar-preferences-store.ts | 25 +++--- src/features/relay/sidebar-preferences.ts | 2 +- src/features/relay/sidebar-registers.test.ts | 18 ++++ src/features/relay/sidebar-registers.ts | 31 +++---- .../browser/sidebar-section-creation.spec.mjs | 47 ++++++++++- 15 files changed, 228 insertions(+), 74 deletions(-) diff --git a/browser-host/sidebar-create-section.test.mjs b/browser-host/sidebar-create-section.test.mjs index e6e970b4e7..1cff589993 100644 --- a/browser-host/sidebar-create-section.test.mjs +++ b/browser-host/sidebar-create-section.test.mjs @@ -73,6 +73,8 @@ it("creates and assigns together, imports legacy fields into registers and keeps it.each([ { ...intent, createSection: { id, name: " " } }, { ...intent, createSection: { id: "__proto__", name: "Launch" } }, + { ...intent, createSection: { id, name: "Launch", position: "last" } }, + { ...intent, createSection: { id, name: "Launch", position: 0 } }, ])("rejects malformed create intent before writing: %j", (value) => { expect(() => assertSidebarAssignmentIntent(value)).toThrow("Invalid"); }); @@ -187,6 +189,32 @@ it("decodes all encrypted preferences despite oversized deleted text without rew expect(decode(head)).toEqual(blob); }); +it("creates at the top in one event, preserves existing orders and keeps retry idempotent", () => { + const first = { + ...intent, + createSection: { ...intent.createSection, position: "first" }, + }; + const created = prepareSidebarAssignment( + [ + event({ + version: 1, + sections: [{ id: "work", name: "Work", order: -7 }], + assignments: {}, + }), + ], + first, + secret, + ); + const saved = decode(created.event); + expect(saved.sections.map(({ id }) => id)).toEqual([id, "work"]); + expect(saved.assignments).toEqual({ alpha: id }); + expect(saved.meta.s[id].order[2]).toBe(-8); + expect(saved.meta.s.work.order[2]).toBe(-7); + expect( + prepareSidebarAssignment([created.event], first, secret).event, + ).toBeUndefined(); +}); + it("appends after the rounded order imported from a fractional legacy head", () => { const created = prepareSidebarAssignment( [ diff --git a/browser-host/sidebar-preferences.mjs b/browser-host/sidebar-preferences.mjs index 2fff380175..87a270585e 100644 --- a/browser-host/sidebar-preferences.mjs +++ b/browser-host/sidebar-preferences.mjs @@ -86,7 +86,7 @@ function validAssignmentIntent(intent) { typeof intent.createSection.id === "string" && typeof intent.createSection.name === "string" && Object.keys(intent.createSection).every((key) => - ["id", "name"].includes(key), + ["id", "name", "position"].includes(key), ))) && validSidebarAssignment(intent) && Object.keys(intent).every((key) => diff --git a/docs/channels.md b/docs/channels.md index d7a461ff11..a4793b4183 100644 --- a/docs/channels.md +++ b/docs/channels.md @@ -154,8 +154,10 @@ placement, rollback notice and Retry. The **Channels** ellipsis menu also offers **Create section**, which creates an empty section without moving a channel. Its dialog retains the name and section ID on failure and waits for confirmation before closing. Dragging a channel into -the gap immediately above **Channels** expands an **Add to new section** drop -target inside the sidebar. It stays revealed for the rest of that drag, with +the gap above the first custom section expands an **Add to new section** drop +target inside the sidebar. With no custom sections, the target is above +**Channels**; **Starred** keeps its pinned position. The created section appears +at that same first position. The target stays revealed for the rest of that drag, with reduced motion opening it immediately; other section destinations remain available. Dropping there opens the existing create-and-move dialog; cancelling leaves the channel in place, and no preference is written until submission. These entry diff --git a/src/features/channel-navigation/ChannelInventory.test.tsx b/src/features/channel-navigation/ChannelInventory.test.tsx index 8b378cca1e..fb3b1a73e4 100644 --- a/src/features/channel-navigation/ChannelInventory.test.tsx +++ b/src/features/channel-navigation/ChannelInventory.test.tsx @@ -1643,6 +1643,11 @@ it("moves a saved group from its menu, rolls back a failed save, and retries it" await waitFor(() => expect(groups()).toEqual(["group:work", "group:later"]), ); + expect( + document.querySelector( + "[data-create-section-slot] + [data-sidebar-section]", + ), + ).toHaveAttribute("data-sidebar-section", "group:work"); await user.click( screen.getByRole("button", { name: "More actions for Work" }), ); @@ -1673,6 +1678,11 @@ it("moves a saved group from its menu, rolls back a failed save, and retries it" await waitFor(() => expect(groups()).toEqual(["group:later", "group:work"]), ); + expect( + document.querySelector( + "[data-create-section-slot] + [data-sidebar-section]", + ), + ).toHaveAttribute("data-sidebar-section", "group:later"); expect(order).toHaveBeenLastCalledWith( ["later", "work"], expect.any(AbortSignal), diff --git a/src/features/channel-navigation/ChannelInventory.tsx b/src/features/channel-navigation/ChannelInventory.tsx index db7cb5b66b..744835319c 100644 --- a/src/features/channel-navigation/ChannelInventory.tsx +++ b/src/features/channel-navigation/ChannelInventory.tsx @@ -269,6 +269,7 @@ export function ChannelInventory({ const [creatingFor, setCreatingFor] = useState<{ channel: ChannelSummary; surface?: ChannelMenuSurface; + position?: "first"; }>(); const createOrigin = creatingFor?.surface?.signal; @@ -493,6 +494,9 @@ export function ChannelInventory({ ), list.activityStatus === "ready", ); + const firstChannelSection = filteredSections.find(({ key }) => + isChannelSectionKey(key), + )?.key; // Menu dismissal may run before a moved row mounts at its new placement. // Only an actual filter exclusion should return focus to a section summary. const hiddenRowSections = useRef(new Map()); @@ -1132,7 +1136,7 @@ export function ChannelInventory({ channel && queries.sidebarPreferences.writable ) - setCreatingFor({ channel }); + setCreatingFor({ channel, position: "first" }); }); }} onMove={(channelId, sectionKey) => @@ -1195,6 +1199,7 @@ export function ChannelInventory({ {filteredSections.map((section) => ( - preferences.createAndAssign(creatingFor.channel.id, section), + preferences.createAndAssign(creatingFor.channel.id, { + ...section, + ...(creatingFor.position + ? { position: creatingFor.position } + : {}), + }), creatingFor.surface, ); setCreatingFor(undefined); diff --git a/src/features/channel-navigation/ChannelSidebarDnd.tsx b/src/features/channel-navigation/ChannelSidebarDnd.tsx index 45d98dd167..7f8539f512 100644 --- a/src/features/channel-navigation/ChannelSidebarDnd.tsx +++ b/src/features/channel-navigation/ChannelSidebarDnd.tsx @@ -228,12 +228,14 @@ export function DroppableSidebarSection({ reorderable, groupIndex, groupDomain = "channels", + createSectionTarget, ...section }: ComponentProps & { disabled: boolean; reorderable: boolean; groupIndex: number; groupDomain?: string; + createSectionTarget?: boolean; }) { const { setNodeRef, isOver, active } = useDroppable({ id: section.sectionKey, @@ -282,9 +284,7 @@ export function DroppableSidebarSection({ const from = active?.data.current?.groupIndex; return ( <> - {section.sectionKey === "channels" && section.createSection && ( - - )} + {createSectionTarget && } { + const transport = await connectNativeTransport(community); + const existing = "11111111-1111-1111-1111-111111111111"; + const id = "22222222-2222-2222-2222-222222222222"; + await transport.writeSidebarAssignment?.( + { channelId: "c1", createSection: { id: existing, name: "Work" } }, + signal, + ); + const groups = await transport.writeSidebarAssignment?.( + { + channelId: "c1", + createSection: { id, name: "Launch", position: "first" }, + }, + signal, + ); + expect(groups?.sections.map(({ id }) => id)).toEqual([id, existing]); + expect(groups?.assignments).toEqual({ c1: id }); +}); + it("creates a section, moves a channel, stars, mutes and sorts with encrypted readback", async () => { const transport = await connectNativeTransport(community); const id = "12345678-1234-1234-1234-123456789abc"; diff --git a/src/features/relay/sidebar-edits.test.ts b/src/features/relay/sidebar-edits.test.ts index 1734f38033..4f5625f706 100644 --- a/src/features/relay/sidebar-edits.test.ts +++ b/src/features/relay/sidebar-edits.test.ts @@ -103,6 +103,12 @@ it("validates create-section policy separately from the host envelope", () => { name: " Work ", }; expect(validSidebarAssignment({ channelId: "c", createSection })).toBe(true); + expect( + validSidebarAssignment({ + channelId: "c", + createSection: { ...createSection, position: "first" }, + }), + ).toBe(true); expect(validSidebarAssignment({ channelId: "c" })).toBe(true); for (const intent of [ { channelId: "c", sectionId: "" }, diff --git a/src/features/relay/sidebar-edits.ts b/src/features/relay/sidebar-edits.ts index d78ab53916..fc7fa8309d 100644 --- a/src/features/relay/sidebar-edits.ts +++ b/src/features/relay/sidebar-edits.ts @@ -93,7 +93,9 @@ export function validSidebarAssignment( (!createSection || (sectionId === undefined && /^[0-9a-f]{8}(-[0-9a-f]{4}){3}-[0-9a-f]{12}$/.test(createSection.id) && - validSidebarChannelId(createSection.name))) + validSidebarChannelId(createSection.name) && + (createSection.position === undefined || + createSection.position === "first"))) ); } @@ -171,7 +173,7 @@ export function editSidebarAssignment( sections.push({ id: createSection.id, name, - order: nextSidebarSectionOrder(current), + order: nextSidebarSectionOrder(current, createSection.position), }); } if (section !== undefined && !sections.some((entry) => entry.id === section)) diff --git a/src/features/relay/sidebar-moves-store.test.ts b/src/features/relay/sidebar-moves-store.test.ts index aeafdbe7d6..d889d2d890 100644 --- a/src/features/relay/sidebar-moves-store.test.ts +++ b/src/features/relay/sidebar-moves-store.test.ts @@ -38,7 +38,16 @@ async function setup() { const sections = intent.createSection && !data.sections.some(({ id }) => id === intent.createSection?.id) - ? [...data.sections, { ...intent.createSection, order: 2 }] + ? intent.createSection.position === "first" + ? [ + { + id: intent.createSection.id, + name: intent.createSection.name, + order: -1, + }, + ...data.sections, + ] + : [...data.sections, { ...intent.createSection, order: 2 }] : data.sections; data = { ...data, sections, assignments }; return { sections, assignments }; @@ -143,37 +152,46 @@ it("failure rolls back just its channel, persists a retry across refresh, and cl } }); -it("create-and-move appears before the first write, rolls back on failure, and reuses its ID on retry", async () => { - const h = await setup(); - const gate = deferred<{ - sections: SidebarPreferences["sections"]; - assignments: SidebarPreferences["assignments"]; - }>(); - h.assignment.mockImplementationOnce(() => gate.promise); - const section = { - id: "12345678-1234-1234-1234-123456789abc", - name: "Launch", - }; - try { - const creating = h.prefs.createAndAssign("alpha", section); - const failed = expect(creating).rejects.toThrow("offline"); - expect(h.prefs.snapshot().data?.sections.at(-1)?.id).toBe(section.id); - expect(h.prefs.snapshot().data?.assignments.alpha).toBe(section.id); - expect(h.prefs.snapshot().data?.starred).toEqual([]); - await flush(); - gate.reject(new Error("offline")); - await failed; - expect(h.prefs.snapshot().data?.sections).toHaveLength(2); - expect(h.prefs.snapshot().data?.starred).toEqual(["alpha"]); - await h.prefs.retryMove("alpha"); - expect( - h.assignment.mock.calls.map(([intent]) => intent.createSection?.id), - ).toEqual([section.id, section.id]); - expect(h.prefs.snapshot().data?.sections).toHaveLength(3); - } finally { - h.owner.dispose(); - } -}); +it.each([undefined, "first"] as const)( + "create-and-move at %s appears before the first write, rolls back on failure, and reuses its ID on retry", + async (position) => { + const h = await setup(); + const gate = deferred<{ + sections: SidebarPreferences["sections"]; + assignments: SidebarPreferences["assignments"]; + }>(); + h.assignment.mockImplementationOnce(() => gate.promise); + const section = { + id: "12345678-1234-1234-1234-123456789abc", + name: "Launch", + ...(position ? { position } : {}), + }; + try { + const creating = h.prefs.createAndAssign("alpha", section); + const failed = expect(creating).rejects.toThrow("offline"); + expect(h.prefs.snapshot().data?.sections.at(position ? 0 : -1)?.id).toBe( + section.id, + ); + expect(h.prefs.snapshot().data?.assignments.alpha).toBe(section.id); + expect(h.prefs.snapshot().data?.starred).toEqual([]); + await flush(); + gate.reject(new Error("offline")); + await failed; + expect(h.prefs.snapshot().data?.sections).toHaveLength(2); + expect(h.prefs.snapshot().data?.starred).toEqual(["alpha"]); + await h.prefs.retryMove("alpha"); + expect( + h.assignment.mock.calls.map(([intent]) => intent.createSection?.id), + ).toEqual([section.id, section.id]); + expect(h.prefs.snapshot().data?.sections).toHaveLength(3); + expect(h.prefs.snapshot().data?.sections.at(position ? 0 : -1)?.id).toBe( + section.id, + ); + } finally { + h.owner.dispose(); + } + }, +); it.each(["clear", "dispose"] as const)( "%s clears optimistic placement/errors and rejects late repopulation", diff --git a/src/features/relay/sidebar-preferences-store.ts b/src/features/relay/sidebar-preferences-store.ts index 8c08ab93e6..e355c5bef7 100644 --- a/src/features/relay/sidebar-preferences-store.ts +++ b/src/features/relay/sidebar-preferences-store.ts @@ -140,15 +140,20 @@ export function createSidebarPreferencesStore({ else { starred.delete(channelId); const created = destination.createSection; - if (created && !sections.some(({ id }) => id === created.id)) - sections = [ - ...sections, - { - ...created, - name: created.name.trim(), - order: Math.max(-1, ...sections.map(({ order }) => order)) + 1, - }, - ]; + if (created && !sections.some(({ id }) => id === created.id)) { + const section = { + id: created.id, + name: created.name.trim(), + order: + created.position === "first" + ? Math.min(0, ...sections.map(({ order }) => order)) - 1 + : Math.max(-1, ...sections.map(({ order }) => order)) + 1, + }; + sections = + created.position === "first" + ? [section, ...sections] + : [...sections, section]; + } const target = created?.id ?? destination.sectionId; if (target) assignments[channelId] = target; else delete assignments[channelId]; @@ -964,7 +969,7 @@ export function createSidebarPreferencesStore({ }, createAndAssign( channelId: string, - section: { id: string; name: string }, + section: NonNullable, signal?: AbortSignal, ) { return move(channelId, { createSection: section }, signal); diff --git a/src/features/relay/sidebar-preferences.ts b/src/features/relay/sidebar-preferences.ts index fcb3932364..7b5bf3c813 100644 --- a/src/features/relay/sidebar-preferences.ts +++ b/src/features/relay/sidebar-preferences.ts @@ -96,7 +96,7 @@ export type SidebarAssignmentIntent = Readonly<{ /** Omit only when creating an empty section. */ channelId?: string; sectionId?: string; - createSection?: Readonly<{ id: string; name: string }>; + createSection?: Readonly<{ id: string; name: string; position?: "first" }>; }>; export type SidebarAssignmentMutator = ( intent: SidebarAssignmentIntent, diff --git a/src/features/relay/sidebar-registers.test.ts b/src/features/relay/sidebar-registers.test.ts index 16616f3c3d..57608a259c 100644 --- a/src/features/relay/sidebar-registers.test.ts +++ b/src/features/relay/sidebar-registers.test.ts @@ -53,6 +53,24 @@ it("reads metadata rather than stale legacy projections and preserves deletion r ).toBe(next); expect(nextSidebarSectionOrder(current)).toBe(100); }); +it("prepends before canonical negative orders without counting deleted sections", () => { + const current = editSidebarRecord( + "channel-sections", + sections(), + 1, + [ + [["s", "live", "order"], -7], + [["s", "dead", "order"], -100], + ], + 110, + ); + expect(nextSidebarSectionOrder(current, "first")).toBe(-8); + expect( + nextSidebarSectionOrder({ sections: [{ order: -2.4 }] }, "first"), + ).toBe(-3); + expect(nextSidebarSectionOrder({ sections: [] }, "first")).toBe(-1); +}); + it("imports a meta-less head at event milliseconds and edits only the requested leaf", () => { const next = editSidebarRecord( "channel-sort", diff --git a/src/features/relay/sidebar-registers.ts b/src/features/relay/sidebar-registers.ts index f74c88dfc5..29e4559839 100644 --- a/src/features/relay/sidebar-registers.ts +++ b/src/features/relay/sidebar-registers.ts @@ -285,27 +285,20 @@ export function editSidebarRecord( return { ...current, ...projected, meta }; } -/** Appending uses canonical orders, not the densely numbered legacy projection. */ +/** Insertion uses canonical orders, not the densely numbered legacy projection. */ export function nextSidebarSectionOrder( current: Record, + position: "first" | "last" = "last", ): number { - if (current.meta === undefined) - return ( - Math.max( - -1, - ...(current.sections as { order: number }[]).map(({ order }) => + const orders = + current.meta === undefined + ? (current.sections as { order: number }[]).map(({ order }) => Math.round(order), - ), - ) + 1 - ); - const tree = metadata("channel-sections", current.meta); - return ( - Object.values(tree.s as Tree).reduce( - (max, node) => - val(node as Tree, "live") === true - ? Math.max(max, Number(val(node as Tree, "order") ?? 0)) - : max, - -1, - ) + 1 - ); + ) + : Object.values(metadata("channel-sections", current.meta).s as Tree) + .filter((node) => val(node as Tree, "live") === true) + .map((node) => Number(val(node as Tree, "order") ?? 0)); + return position === "first" + ? Math.min(0, ...orders) - 1 + : Math.max(-1, ...orders) + 1; } diff --git a/tests/browser/sidebar-section-creation.spec.mjs b/tests/browser/sidebar-section-creation.spec.mjs index 7aad2884a7..aed6ec6180 100644 --- a/tests/browser/sidebar-section-creation.spec.mjs +++ b/tests/browser/sidebar-section-creation.spec.mjs @@ -101,10 +101,10 @@ async function revealCreate(page) { .toBe("auto"); } -test("only the Channels boundary reveals new-section creation, latches, cancels and saves", async ({ +test("only the top channel-section boundary reveals creation, latches, cancels and saves first", async ({ page, app, -}) => { +}, testInfo) => { await page.goto(app.origin); await openPage(page, "Messages"); const target = page.locator("[data-create-section-drop]"); @@ -112,7 +112,41 @@ test("only the Channels boundary reveals new-section creation, latches, cancels await expect(target).toHaveCount(0); await startDrag(page, row); await expect(target).toHaveCount(0); + // Channels is not first when Work exists: its old gap is an ordinary move area. + const channels = page.locator('[data-sidebar-section="channels"]'); + const channelsBox = await channels.boundingBox(); + if (!channelsBox) throw new Error("Channels destination missing"); + await page.mouse.move( + channelsBox.x + channelsBox.width / 2, + channelsBox.y - 8, + ); + await page.mouse.move( + channelsBox.x + channelsBox.width / 2, + channelsBox.y + 12, + ); + await expect(channels).toHaveAttribute("data-drop-target", "true"); + await expect(target).toHaveCount(0); + await expect( + page.locator("[data-create-section-slot] + [data-sidebar-section]"), + ).toHaveAttribute("data-sidebar-section", "group:work"); await revealCreate(page); + await page + .locator(".shell-sidebar") + .screenshot({ path: testInfo.outputPath("top-section-target.png") }); + await page.evaluate(() => { + localStorage.setItem("buzz-appearance.v1", "dark"); + window.dispatchEvent( + new StorageEvent("storage", { + key: "buzz-appearance.v1", + newValue: "dark", + storageArea: localStorage, + }), + ); + }); + await expect(page.locator("html")).toHaveAttribute("data-color-mode", "dark"); + await page.locator(".shell-sidebar").screenshot({ + path: testInfo.outputPath("top-section-target-dark.png"), + }); // Leaving the gap must not collapse it or prevent an ordinary section move. const work = page.locator('[data-sidebar-section="group:work"] summary'); const workBox = await work.boundingBox(); @@ -162,11 +196,20 @@ test("only the Channels boundary reveals new-section creation, latches, cancels .locator("[data-sidebar-section]") .filter({ has: page.locator("summary").filter({ hasText: /^Release$/ }) }); await expect(section.locator('[data-channel-id="alpha"]')).toBeVisible(); + await expect( + page.locator('[data-sidebar-section^="group:"] summary'), + ).toHaveText(["Release", "Work"]); await expect(row).toBeFocused(); await expect.poll(() => app.report.sidebarPublications?.length ?? 0).toBe(2); await page.reload(); await openPage(page, "Messages"); await expect(section.locator('[data-channel-id="alpha"]')).toBeVisible(); + await expect( + page.locator('[data-sidebar-section^="group:"] summary'), + ).toHaveText(["Release", "Work"]); + await page.locator(".shell-sidebar").screenshot({ + path: testInfo.outputPath("created-section-first.png"), + }); // The original per-channel creation entry remains available. await row.click({ button: "right" }); await page From 3c797bc2f037bab0b1b20f6d013719ea14fce9e6 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Tue, 27 Oct 2026 13:16:30 -0700 Subject: [PATCH 10/21] test(channels): await the members dialog focus handoff Observe the existing initial-focus lifecycle before sending Escape. Retain the search focus and trigger restoration assertions after the faster mainline fixture setup. Co-authored-by: Carl Signed-off-by: Taylor Ho --- src/bundled/channels/ChannelMembersDialog.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bundled/channels/ChannelMembersDialog.test.tsx b/src/bundled/channels/ChannelMembersDialog.test.tsx index 12e62cc372..0e1a3870fc 100644 --- a/src/bundled/channels/ChannelMembersDialog.test.tsx +++ b/src/bundled/channels/ChannelMembersDialog.test.tsx @@ -355,7 +355,7 @@ it("DMs are view-only and never search outside their members", async () => { }); it("Escape closes the dialog and restores focus to its header button", async () => { const t = await setup(); - expect(screen.getByRole("searchbox")).toHaveFocus(); + await vi.waitFor(() => expect(screen.getByRole("searchbox")).toHaveFocus()); await t.user.keyboard("{Escape}"); await vi.waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument(), From 60b1eafd8e368d64a16d11b45a366087ea9cc8a5 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Tue, 27 Oct 2026 13:24:17 -0700 Subject: [PATCH 11/21] fix(sidebar): preserve solo wording and drag previews Keep workspace creation errors specific to Solo, restrict the create-target preview offset to Messages channel drags, and document canonical insertion order and resize measurement semantics. Co-authored-by: Carl Signed-off-by: Taylor Ho --- docs/channels.md | 5 ++-- src/bundled/channels/Channels.module.css | 6 +++-- .../channel-navigation/ChannelInventory.tsx | 5 +++- .../channel-navigation/ChannelSidebarDnd.tsx | 2 ++ .../relay/sidebar-preferences-store.test.ts | 27 +++++++++++++++++++ .../relay/sidebar-preferences-store.ts | 9 ++++--- tests/browser/navigation-group-order.spec.mjs | 5 ++++ .../browser/sidebar-section-creation.spec.mjs | 7 +++++ 8 files changed, 57 insertions(+), 9 deletions(-) diff --git a/docs/channels.md b/docs/channels.md index a4793b4183..07f5d0d242 100644 --- a/docs/channels.md +++ b/docs/channels.md @@ -280,8 +280,9 @@ A register is `[version, device, value]`, with a nonnegative safe-integer millis version and 16 lowercase hex device ID. Meta-less relay heads import at event seconds × 1,000 with the zero device ID. Edits exceed every observed register version and use a random in-memory device ID. Sections sort by canonical `(order,id)` -and project dense display orders; new sections append after the canonical live -maximum. Orphan assignments are omitted from the legacy projection, not deleted +and project dense display orders; menu-created sections append after the canonical +live maximum, while drop-created sections precede the live minimum. Orphan +assignments are omitted from the legacy projection, not deleted from metadata. Read projection accepts Desktop string values on retained section name/icon registers; only projected live text receives UI length limits. Already satisfied intents return without rewriting the head, including assignment removal diff --git a/src/bundled/channels/Channels.module.css b/src/bundled/channels/Channels.module.css index 46da9db399..61669d676e 100644 --- a/src/bundled/channels/Channels.module.css +++ b/src/bundled/channels/Channels.module.css @@ -361,8 +361,6 @@ transform: translateY(50%); } .channelDragOverlay { - /* The new-section slot opens below the gap where the pointer first enters. */ - translate: var(--space-3) calc(-100% - var(--space-3)); display: flex; align-items: center; gap: var(--space-2); @@ -378,6 +376,10 @@ line-height: var(--text-body-sm--line-height); cursor: grabbing; } +.channelDragOverlay[data-section-creation-preview] { + /* The new-section slot opens below the gap where the pointer first enters. */ + translate: var(--space-3) calc(-100% - var(--space-3)); +} .channelDragOverlay > span { overflow: hidden; text-overflow: ellipsis; diff --git a/src/features/channel-navigation/ChannelInventory.tsx b/src/features/channel-navigation/ChannelInventory.tsx index 744835319c..be24ab8cbd 100644 --- a/src/features/channel-navigation/ChannelInventory.tsx +++ b/src/features/channel-navigation/ChannelInventory.tsx @@ -1189,7 +1189,10 @@ export function ChannelInventory({ if (!channel) return null; const Icon = channelIcon(channel); return ( -
+
{channel.name}
diff --git a/src/features/channel-navigation/ChannelSidebarDnd.tsx b/src/features/channel-navigation/ChannelSidebarDnd.tsx index 7f8539f512..68ff502567 100644 --- a/src/features/channel-navigation/ChannelSidebarDnd.tsx +++ b/src/features/channel-navigation/ChannelSidebarDnd.tsx @@ -146,6 +146,8 @@ function CreateSectionDropTarget() { const { setNodeRef, isOver } = useDroppable({ id: CREATE_SECTION, disabled: !enabled, + // dnd-kit treats an empty list as all droppables on resize. Expansion also + // shifts the sections below this slot, so their collision rects must refresh. resizeObserverConfig: { updateMeasurementsFor: [] }, }); useDndMonitor({ diff --git a/src/features/relay/sidebar-preferences-store.test.ts b/src/features/relay/sidebar-preferences-store.test.ts index 8922c782e9..64ab91c457 100644 --- a/src/features/relay/sidebar-preferences-store.test.ts +++ b/src/features/relay/sidebar-preferences-store.test.ts @@ -1,5 +1,6 @@ import { expect, it, vi } from "vitest"; import { createRelaySession } from "./session"; +import { createSidebarPreferencesStore } from "./sidebar-preferences-store"; import { flush, keypair, scriptedTransport } from "./testing"; import type { SidebarAssignmentMutator, @@ -20,6 +21,32 @@ function deferred() { return { promise, resolve, reject }; } +it.each([ + { groupsOnly: false, failure: "Section creation is unavailable" }, + { groupsOnly: true, failure: "Workspaces are unavailable" }, +])( + "keeps creation errors specific to the groupsOnly=$groupsOnly surface", + async ({ groupsOnly, failure }) => { + const store = createSidebarPreferencesStore({ + read: async () => ({ + sections: [], + assignments: {}, + starred: [], + muted: [], + }), + available: true, + groupsOnly, + }); + try { + await expect( + store.queries.createSection({ id: "new", name: "New" }), + ).rejects.toThrow(failure); + } finally { + store.dispose(); + } + }, +); + const data: SidebarPreferences = { sections: [{ id: "work", name: "Work", order: 0 }], assignments: { alpha: "work" }, diff --git a/src/features/relay/sidebar-preferences-store.ts b/src/features/relay/sidebar-preferences-store.ts index e355c5bef7..a41b90cc48 100644 --- a/src/features/relay/sidebar-preferences-store.ts +++ b/src/features/relay/sidebar-preferences-store.ts @@ -659,6 +659,9 @@ export function createSidebarPreferencesStore({ return writable() && !snapshot.cached && !!(createSection || write); }, createSection(section: Readonly<{ id: string; name: string }>) { + const failure = groupsOnly + ? "Workspaces are unavailable" + : "Section creation is unavailable"; const create = createSection ?? (write @@ -668,10 +671,8 @@ export function createSidebarPreferencesStore({ ) => write({ createSection: section }, signal) : undefined); if (!writable() || snapshot.cached || !create) - return Promise.reject(new Error("Section creation is unavailable")); - return writeGroups("Section creation is unavailable", (signal) => - create(section, signal), - ); + return Promise.reject(new Error(failure)); + return writeGroups(failure, (signal) => create(section, signal)); }, get sectionOrderWritable() { return orderWritable(); diff --git a/tests/browser/navigation-group-order.spec.mjs b/tests/browser/navigation-group-order.spec.mjs index f3fc1458e1..b59fcf0a50 100644 --- a/tests/browser/navigation-group-order.spec.mjs +++ b/tests/browser/navigation-group-order.spec.mjs @@ -58,6 +58,11 @@ test("dragging a group header moves the whole group, keeps its state, and persis // A cancelled drag shows its landing edge, then leaves everything unchanged. await pull(page, "laptop", "work"); await expect(laptop).toHaveAttribute("data-section-dragging"); + await expect(page.locator("[data-section-creation-preview]")).toHaveCount(0); + await expect(page.locator('[class*="channelDragOverlay"]')).toHaveCSS( + "translate", + "none", + ); await expect(work).toHaveAttribute("data-drop-edge", "before"); await expect(work).not.toHaveAttribute("data-drop-target"); await page.keyboard.press("Escape"); diff --git a/tests/browser/sidebar-section-creation.spec.mjs b/tests/browser/sidebar-section-creation.spec.mjs index aed6ec6180..12ec9b3b52 100644 --- a/tests/browser/sidebar-section-creation.spec.mjs +++ b/tests/browser/sidebar-section-creation.spec.mjs @@ -111,6 +111,11 @@ test("only the top channel-section boundary reveals creation, latches, cancels a const row = page.locator('[data-channel-id="alpha"]'); await expect(target).toHaveCount(0); await startDrag(page, row); + await expect(page.locator("[data-section-creation-preview]")).toBeVisible(); + await expect(page.locator("[data-section-creation-preview]")).not.toHaveCSS( + "translate", + "none", + ); await expect(target).toHaveCount(0); // Channels is not first when Work exists: its old gap is an ordinary move area. const channels = page.locator('[data-sidebar-section="channels"]'); @@ -163,7 +168,9 @@ test("only the top channel-section boundary reveals creation, latches, cancels a await page.mouse.up(); await expect(target).toHaveCount(0); expect(app.report.sidebarPublications ?? []).toHaveLength(0); + // A lone saved section is not reorderable and cannot reveal creation. await startDrag(page, work); + await expect(page.locator("[data-section-creation-preview]")).toHaveCount(0); await expect(target).toHaveCount(0); await page.keyboard.press("Escape"); await page.mouse.up(); From 83e14f7632fb1c493b1287baf8a889fd0a912ffb Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Tue, 27 Oct 2026 14:05:45 -0700 Subject: [PATCH 12/21] test(sidebar): synchronize drag targets with sidebar layout Wait for collapse completion and rendered acceptance before the next drag action. Follow the destination geometry after a creation-slot reveal, and scope the large sidebar fixture to its scroll-preservation test. Co-authored-by: Carl Signed-off-by: Taylor Ho --- tests/browser/navigation-groups.spec.mjs | 205 ++++++++++-------- .../browser/sidebar-section-creation.spec.mjs | 10 +- 2 files changed, 120 insertions(+), 95 deletions(-) diff --git a/tests/browser/navigation-groups.spec.mjs b/tests/browser/navigation-groups.spec.mjs index 24ce7e3167..b5ecf1d7f0 100644 --- a/tests/browser/navigation-groups.spec.mjs +++ b/tests/browser/navigation-groups.spec.mjs @@ -4,7 +4,12 @@ import { test, expect } from "./fixture.mjs"; import { open } from "./timeline.mjs"; import { holdReadingFocus } from "./reading.mjs"; -test.use({ productionBroker: true, savedSidebar: true }); +test.use({ + productionBroker: true, + savedSidebar: true, + developmentReact: true, + historyCounts: { alpha: 1, beta: 1 }, +}); async function openMove(page, row, channelName = "Beta") { await row.focus(); @@ -29,15 +34,27 @@ const sidebar = (page) => page.getByRole("navigation", { name: "Subscribed channels" }); // Hold a real pointer drag over a section header; the caller releases it. async function pull(page, row, section) { + await row.hover(); const from = await row.boundingBox(); - const to = await page - .locator(`[data-sidebar-section="${section}"] summary`) - .boundingBox(); + const destination = page.locator( + `[data-sidebar-section="${section}"] summary`, + ); + const to = await destination.boundingBox(); await page.mouse.move(from.x + from.width / 2, from.y + from.height / 2); await page.mouse.down(); await page.mouse.move(to.x + to.width / 2, to.y + to.height / 2, { steps: 8, }); + // Crossing the creation gap can expand it and shift this header. Follow the + // actual destination after its layout settles rather than its old centre. + // The drag overlay owns pointer events, so use the stable layout check rather + // than hover's hit-target check, then move the real pointer to that rectangle. + await destination.scrollIntoViewIfNeeded(); + const current = await destination.boundingBox(); + await page.mouse.move( + current.x + current.width / 2, + current.y + current.height / 2, + ); } // No saving UI is a completion signal: wait for the final confirmed Star command // of each move (including no-op Star writes), not merely relay publication. @@ -946,102 +963,102 @@ test("Create new retains its draft when preferences fail before submission and r } }); -test.use({ - productionBroker: true, - savedSidebar: true, +// Keep the scroll workload local: test.use at file scope would also give every +// pointer-move journey this long, independently scrolling sidebar. +const scrollTest = test.extend({ largeSidebar: true, - developmentReact: true, - historyCounts: { alpha: 1, beta: 1 }, }); -test("Workflows → Messages keeps saved groups, selected channel, and a visible entry's scroll on every frame without re-decoding", async ({ - page, - app, -}) => { - await open(page, app); - const sidebar = page.getByRole("navigation", { - name: "Subscribed channels", - }); - await expect(sidebar.locator("summary", { hasText: /^Work$/ })).toBeVisible(); - await expect( - sidebar.locator("summary", { hasText: /Starred$/ }), - ).toBeVisible(); - await sidebar.locator('button[data-channel-id="beta"]').click(); - await expect( - page.getByRole("textbox", { name: "Message #Beta", exact: true }), - ).toBeVisible(); - await openPage(page, "Workflows"); - await expect(sidebar).toBeVisible(); - // Navigation reveals the current entry; keep Beta in view at a nonzero - // offset so the warm return has no reason to move the list. - const scroll = await sidebar.evaluate((element) => { - element.scrollTop = 0; - const row = element.querySelector('button[data-channel-id="beta"]'); - element.scrollTop = - row.getBoundingClientRect().top - element.getBoundingClientRect().top; - return element.scrollTop; - }); - expect(scroll).toBeGreaterThan(100); - let release; - const held = new Promise((resolve) => { - release = resolve; - }); - let decodes = 0; - await page.route("**/sidebar-preferences", async (route) => { - decodes++; - await held; - await route.continue(); - }); - await page.evaluate(() => { - window.sidebarFrames = []; - window.captureSidebar = true; - const frame = () => { - const list = document.querySelector( - 'nav[aria-label="Subscribed channels"]', - ); - if (list) - window.sidebarFrames.push({ - top: list.scrollTop, - groups: Array.from(list.querySelectorAll("summary"), (el) => - el.textContent.replace("★", "").trim(), - ), - }); - if (window.captureSidebar) requestAnimationFrame(frame); - }; - requestAnimationFrame(frame); - }); - try { - await openPage(page, "Messages"); - await expect(sidebar).toBeVisible(); +scrollTest( + "Workflows → Messages keeps saved groups, selected channel, and a visible entry's scroll on every frame without re-decoding", + async ({ page, app }) => { + await open(page, app); + const sidebar = page.getByRole("navigation", { + name: "Subscribed channels", + }); + await expect( + sidebar.locator("summary", { hasText: /^Work$/ }), + ).toBeVisible(); + await expect( + sidebar.locator("summary", { hasText: /Starred$/ }), + ).toBeVisible(); + await sidebar.locator('button[data-channel-id="beta"]').click(); await expect( page.getByRole("textbox", { name: "Message #Beta", exact: true }), ).toBeVisible(); - // Wall time does not guarantee RAF callbacks on a busy runner. Wait for - // samples, not correct samples: every earlier frame stays in the assertion. - // The decode path stays held until finally, across all sampled frames. - await page.waitForFunction(() => window.sidebarFrames.length > 3); - const frames = await page.evaluate(() => { - window.captureSidebar = false; - return window.sidebarFrames; + await openPage(page, "Workflows"); + await expect(sidebar).toBeVisible(); + // Navigation reveals the current entry; keep Beta in view at a nonzero + // offset so the warm return has no reason to move the list. + const scroll = await sidebar.evaluate((element) => { + element.scrollTop = 0; + const row = element.querySelector('button[data-channel-id="beta"]'); + element.scrollTop = + row.getBoundingClientRect().top - element.getBoundingClientRect().top; + return element.scrollTop; }); - expect(frames.length).toBeGreaterThan(3); - expect( - frames.filter( - (frame) => - !frame.groups.includes("Work") || - !frame.groups.includes("Starred") || - Math.abs(frame.top - scroll) > 1, - ), - "No fallback grouping or top-of-list frame on warm return", - ).toEqual([]); - expect( - decodes, - "Page return must reuse the engine snapshot, not fetch/decode again", - ).toBe(0); - } finally { - release(); - await page.unroute("**/sidebar-preferences"); - } -}); + expect(scroll).toBeGreaterThan(100); + let release; + const held = new Promise((resolve) => { + release = resolve; + }); + let decodes = 0; + await page.route("**/sidebar-preferences", async (route) => { + decodes++; + await held; + await route.continue(); + }); + await page.evaluate(() => { + window.sidebarFrames = []; + window.captureSidebar = true; + const frame = () => { + const list = document.querySelector( + 'nav[aria-label="Subscribed channels"]', + ); + if (list) + window.sidebarFrames.push({ + top: list.scrollTop, + groups: Array.from(list.querySelectorAll("summary"), (el) => + el.textContent.replace("★", "").trim(), + ), + }); + if (window.captureSidebar) requestAnimationFrame(frame); + }; + requestAnimationFrame(frame); + }); + try { + await openPage(page, "Messages"); + await expect(sidebar).toBeVisible(); + await expect( + page.getByRole("textbox", { name: "Message #Beta", exact: true }), + ).toBeVisible(); + // Wall time does not guarantee RAF callbacks on a busy runner. Wait for + // samples, not correct samples: every earlier frame stays in the assertion. + // The decode path stays held until finally, across all sampled frames. + await page.waitForFunction(() => window.sidebarFrames.length > 3); + const frames = await page.evaluate(() => { + window.captureSidebar = false; + return window.sidebarFrames; + }); + expect(frames.length).toBeGreaterThan(3); + expect( + frames.filter( + (frame) => + !frame.groups.includes("Work") || + !frame.groups.includes("Starred") || + Math.abs(frame.top - scroll) > 1, + ), + "No fallback grouping or top-of-list frame on warm return", + ).toEqual([]); + expect( + decodes, + "Page return must reuse the engine snapshot, not fetch/decode again", + ).toBe(0); + } finally { + release(); + await page.unroute("**/sidebar-preferences"); + } + }, +); // Browser-owned integration: the persistent sidebar's // menu/dialog portals, and encrypted saves while Messages is not mounted. diff --git a/tests/browser/sidebar-section-creation.spec.mjs b/tests/browser/sidebar-section-creation.spec.mjs index 12ec9b3b52..b171d1b83f 100644 --- a/tests/browser/sidebar-section-creation.spec.mjs +++ b/tests/browser/sidebar-section-creation.spec.mjs @@ -167,6 +167,12 @@ test("only the top channel-section boundary reveals creation, latches, cancels a await page.keyboard.press("Escape"); await page.mouse.up(); await expect(target).toHaveCount(0); + // The cue retires before its height finishes collapsing. Wait for the layout + // boundary before sampling Work's header for the next pointer gesture. + await expect(page.locator("[data-create-section-trigger] + div")).toHaveCSS( + "height", + "0px", + ); expect(app.report.sidebarPublications ?? []).toHaveLength(0); // A lone saved section is not reorderable and cannot reveal creation. await startDrag(page, work); @@ -174,11 +180,13 @@ test("only the top channel-section boundary reveals creation, latches, cancels a await expect(target).toHaveCount(0); await page.keyboard.press("Escape"); await page.mouse.up(); - // Release in the original gap immediately: animation must not move it away. + // Release as soon as the original gap accepts the drop, without waiting for + // expansion. dnd-kit drops on its rendered target, not the last mouse event. await startDrag(page, row); const gap = await page.locator("[data-create-section-trigger]").boundingBox(); if (!gap) throw new Error("Create boundary missing"); await page.mouse.move(gap.x + gap.width / 2, gap.y + gap.height / 2); + await expect(target).toHaveAttribute("data-drop-target", "true"); await page.mouse.up(); const dialog = page.getByRole("dialog", { name: "Move to new section" }); const field = dialog.getByRole("textbox", { name: "Section name" }); From 863fedbe9e11a0a583d023b1c8915fcccd352a85 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Tue, 27 Oct 2026 15:01:38 -0700 Subject: [PATCH 13/21] fix(sidebar): retain submitted section names on retry Keep the submitted ID and trimmed name together and lock the field after submission so an unconfirmed write can be retried without renaming the retained ID. Cover attempted edits, failed encrypted readback and an accepted broker write with a lost response. Co-authored-by: Carl Signed-off-by: Taylor Ho --- browser-host/sidebar-create-section.test.mjs | 49 +++++++++++-------- .../channels/CreateSidebarSection.test.tsx | 13 ++++- src/bundled/channels/CreateSidebarSection.tsx | 13 +++-- .../browser/sidebar-section-creation.spec.mjs | 29 +++++++++++ 4 files changed, 75 insertions(+), 29 deletions(-) diff --git a/browser-host/sidebar-create-section.test.mjs b/browser-host/sidebar-create-section.test.mjs index 1cff589993..ad30f1edaa 100644 --- a/browser-host/sidebar-create-section.test.mjs +++ b/browser-host/sidebar-create-section.test.mjs @@ -103,26 +103,35 @@ it("enforces section limits and refuses an invalid or unreadable current head", ), ).toThrow("Invalid"); }); -it("failed confirmation may follow publication; retry confirms the same section without another write", async () => { - let head; - let reads = 0; - let publications = 0; - const read = async () => { - if (++reads === 2) throw new Error("confirmation offline"); - return head ? [head] : []; - }; - const publish = async (value) => { - publications++; - head = value; - }; - await expect( - mutateSidebarAssignment(intent, secret, read, publish), - ).rejects.toThrow("confirmation offline"); - const groups = await mutateSidebarAssignment(intent, secret, read, publish); - expect(groups.sections).toEqual([{ id, name: "Launch", order: 0 }]); - expect(groups.assignments).toEqual({ alpha: id }); - expect(publications).toBe(1); -}); +it.each([intent, { createSection: intent.createSection }])( + "failed confirmation may follow publication; retry confirms the same section without another write: %j", + async (creation) => { + let head; + let reads = 0; + let publications = 0; + const read = async () => { + if (++reads === 2) throw new Error("confirmation offline"); + return head ? [head] : []; + }; + const publish = async (value) => { + publications++; + head = value; + }; + await expect( + mutateSidebarAssignment(creation, secret, read, publish), + ).rejects.toThrow("confirmation offline"); + expect(decode(head).sections).toEqual([{ id, name: "Launch", order: 0 }]); + const groups = await mutateSidebarAssignment( + creation, + secret, + read, + publish, + ); + expect(groups.sections).toEqual([{ id, name: "Launch", order: 0 }]); + expect(groups.assignments).toEqual(creation.channelId ? { alpha: id } : {}); + expect(publications).toBe(1); + }, +); it("a failed initial read never seeds or publishes", async () => { let publications = 0; await expect( diff --git a/src/bundled/channels/CreateSidebarSection.test.tsx b/src/bundled/channels/CreateSidebarSection.test.tsx index da646b23b7..3ff0dfd4d7 100644 --- a/src/bundled/channels/CreateSidebarSection.test.tsx +++ b/src/bundled/channels/CreateSidebarSection.test.tsx @@ -8,6 +8,7 @@ import { waitFor, } from "@testing-library/react"; import { afterEach, expect, it, vi } from "vitest"; +import userEvent from "@testing-library/user-event"; import { CreateSidebarSection } from "./CreateSidebarSection"; afterEach(cleanup); @@ -61,7 +62,8 @@ it("retains a rejected draft and its identity through preference recovery", asyn expect(create).toHaveBeenCalledTimes(2); }); -it("keeps an empty-section draft and identity through a failed save, blocking dismissal while pending", async () => { +it("keeps the submitted name and identity through a failed save, blocking dismissal while pending", async () => { + const user = userEvent.setup(); let reject!: (cause: Error) => void; const create = vi .fn() @@ -98,9 +100,16 @@ it("keeps an empty-section draft and identity through a failed save, blocking di expect(create).toHaveBeenCalledTimes(1); reject(new Error("Could not confirm section")); await screen.findByRole("alert"); + // The write may already have reached the relay. Editing before retry must + // not pair the retained ID with a different name. + await user.type(input, "es"); expect(input).toHaveValue(" Launch "); + expect(input).toBeDisabled(); expect(close).not.toHaveBeenCalled(); fireEvent.click(submit); - await waitFor(() => expect(input).toBeEnabled()); + await waitFor(() => + expect(screen.getByRole("button", { name: "Cancel" })).toBeEnabled(), + ); + expect(create).toHaveBeenCalledTimes(2); expect(create.mock.calls[1]).toEqual(create.mock.calls[0]); }); diff --git a/src/bundled/channels/CreateSidebarSection.tsx b/src/bundled/channels/CreateSidebarSection.tsx index e0a4965590..d58827d90c 100644 --- a/src/bundled/channels/CreateSidebarSection.tsx +++ b/src/bundled/channels/CreateSidebarSection.tsx @@ -26,21 +26,20 @@ export function CreateSidebarSection({ const input = useRef(null); const [name, setName] = useState(""); const submitted = useRef(false); - const sectionId = useRef(undefined); + const section = useRef<{ id: string; name: string } | undefined>(undefined); const [blocked, setBlocked] = useState(false); const [pending, setPending] = useState(false); const [error, setError] = useState(""); const submit = async () => { if (submitted.current || pending || !name.trim()) return; submitted.current = true; - sectionId.current ??= crypto.randomUUID(); + // A failed confirmation may follow a successful write. Retry the same + // identity and name rather than trying to rename an unknown publication. + section.current ??= { id: crypto.randomUUID(), name: name.trim() }; setPending(true); setError(""); try { - const accepted = await create({ - id: sectionId.current, - name: name.trim(), - }); + const accepted = await create(section.current); submitted.current = accepted; setBlocked(!accepted); } catch (cause) { @@ -93,7 +92,7 @@ export function CreateSidebarSection({ aria-label="Section name" placeholder="Section name" required - disabled={pending} + disabled={pending || !!section.current} maxLength={256} value={name} onChange={(event) => setName(event.target.value)} diff --git a/tests/browser/sidebar-section-creation.spec.mjs b/tests/browser/sidebar-section-creation.spec.mjs index b171d1b83f..cf7417d28f 100644 --- a/tests/browser/sidebar-section-creation.spec.mjs +++ b/tests/browser/sidebar-section-creation.spec.mjs @@ -40,8 +40,37 @@ test("Channels offers empty-section creation with keyboard focus and confirmed p await expect(page.getByRole("menu")).toHaveCount(0); await page.keyboard.type("Launch"); await expect(field).toHaveValue("Launch"); + const creations = []; + let lostResponse = false; + await page.route("**/sidebar-assignment", async (route) => { + creations.push(route.request().postDataJSON()); + const response = await route.fetch(); + if (!lostResponse && response.ok()) { + // The real broker accepts and persists the write, but its response never + // reaches the dialog. The retry must reconcile that exact creation. + lostResponse = true; + app.report.sidebarAssignmentFailures ??= []; + app.report.sidebarAssignmentFailures.push(route.request().url()); + await route.fulfill({ + status: 502, + contentType: "application/json", + body: JSON.stringify({ error: "Section confirmation unavailable" }), + }); + } else await route.fulfill({ response }); + }); await field.press("Enter"); + await expect(dialog.getByRole("alert")).toContainText( + "Relay request failed (502)", + ); + await expect.poll(() => app.report.sidebarPublications?.length ?? 0).toBe(1); + await expect(field).toBeDisabled(); + await page.keyboard.type("es"); + await expect(field).toHaveValue("Launch"); + await dialog.getByRole("button", { name: "Create", exact: true }).click(); await expect(dialog).toHaveCount(0); + expect(creations).toHaveLength(2); + expect(creations[1]).toEqual(creations[0]); + await page.unroute("**/sidebar-assignment"); await expect(trigger).toBeFocused(); await expect( page.locator("summary").filter({ hasText: /^Launch$/ }), From 3f8f78b7561b34393e90f69db22b22caa08de764 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Tue, 27 Oct 2026 15:36:36 -0700 Subject: [PATCH 14/21] fix(sidebar): reconcile uncertain section creations before dismissal Refresh confirmed preferences after a failed empty-section write before the form can discard its creation ID. Supersede older reads, retain the existing error and write gate if reconciliation fails, and cover Cancel/reopen with an accepted broker write and lost response. Co-authored-by: Carl Signed-off-by: Taylor Ho --- docs/channels.md | 5 +- .../relay/sidebar-moves-store.test.ts | 118 +++++++++++++++++- .../relay/sidebar-preferences-store.ts | 15 ++- .../browser/sidebar-section-creation.spec.mjs | 42 ++++++- 4 files changed, 175 insertions(+), 5 deletions(-) diff --git a/docs/channels.md b/docs/channels.md index 07f5d0d242..8282d2fdee 100644 --- a/docs/channels.md +++ b/docs/channels.md @@ -153,7 +153,10 @@ can also be dragged with a pointer onto another saved group, **Starred** or placement, rollback notice and Retry. The **Channels** ellipsis menu also offers **Create section**, which creates an empty section without moving a channel. Its dialog retains the name and section -ID on failure and waits for confirmation before closing. Dragging a channel into +ID on failure and waits for confirmation before closing. After a failed creation, +the session refreshes preferences before allowing dismissal so an accepted save +stays visible even after Cancel. If that read fails too, new creation is gated +until preferences can be read successfully. Dragging a channel into the gap above the first custom section expands an **Add to new section** drop target inside the sidebar. With no custom sections, the target is above **Channels**; **Starred** keeps its pinned position. The created section appears diff --git a/src/features/relay/sidebar-moves-store.test.ts b/src/features/relay/sidebar-moves-store.test.ts index d889d2d890..f82c1a5583 100644 --- a/src/features/relay/sidebar-moves-store.test.ts +++ b/src/features/relay/sidebar-moves-store.test.ts @@ -1,4 +1,4 @@ -import { expect, it, vi } from "vitest"; +import { assert, expect, it, vi } from "vitest"; import { createSidebarPreferencesStore } from "./sidebar-preferences-store"; import type { SidebarAssignmentIntent, @@ -603,3 +603,119 @@ it("creates an empty section through the queued writer without altering stars or h.owner.dispose(); } }); + +it("refreshes an accepted creation before reporting a lost response, superseding an older read", async () => { + const h = await setup(); + const before = h.prefs.snapshot().data; + assert(before); + const stale = deferred(); + const save = h.assignment.getMockImplementation(); + assert(save); + h.assignment.mockImplementationOnce(async (intent) => { + await save(intent); + throw new Error("response lost"); + }); + h.read.mockImplementationOnce(() => stale.promise); + const oldRefresh = h.prefs.refresh(); + const section = { id: "accepted", name: "Launch" }; + try { + await expect(h.prefs.createSection(section)).rejects.toThrow( + "response lost", + ); + expect(h.prefs.snapshot().data?.sections.at(-1)).toMatchObject(section); + expect(h.assignment).toHaveBeenCalledOnce(); // Recovery never replays a write. + expect(h.prefs.sectionCreationWritable).toBe(true); + stale.resolve(before); + await oldRefresh; + expect(h.prefs.snapshot().data?.sections.at(-1)).toMatchObject(section); + } finally { + stale.resolve(before); + h.owner.dispose(); + } +}); + +it("gates new creation when reconciliation fails until preferences can be read", async () => { + const h = await setup(); + const save = h.assignment.getMockImplementation(); + assert(save); + h.assignment.mockImplementationOnce(async (intent) => { + await save(intent); + throw new Error("response lost"); + }); + h.read.mockRejectedValueOnce(new Error("read offline")); + const section = { id: "accepted", name: "Launch" }; + try { + await expect(h.prefs.createSection(section)).rejects.toThrow( + "response lost", + ); + expect(h.prefs.snapshot()).toMatchObject({ + status: "error", + error: "read offline", + }); + expect(h.prefs.sectionCreationWritable).toBe(false); + await expect( + h.prefs.createSection({ id: "duplicate", name: "Launch" }), + ).rejects.toThrow("unavailable"); + expect(h.assignment).toHaveBeenCalledOnce(); + await h.prefs.refresh(); + expect(h.prefs.sectionCreationWritable).toBe(true); + expect(h.prefs.snapshot().data?.sections.at(-1)).toMatchObject(section); + } finally { + h.owner.dispose(); + } +}); + +it.each(["clear", "dispose"] as const)( + "fences creation reconciliation after %s", + async (retire) => { + const h = await setup(); + const read = deferred(); + const entered = deferred(); + h.assignment.mockRejectedValueOnce(new Error("response lost")); + h.read.mockImplementationOnce(() => { + entered.resolve(); + return read.promise; + }); + const result = expect( + h.prefs.createSection({ id: "accepted", name: "Launch" }), + ).rejects.toThrow("response lost"); + try { + await entered.promise; + h.owner[retire](); + read.resolve({ + sections: [{ id: "accepted", name: "Launch", order: 2 }], + assignments: {}, + starred: [], + muted: [], + }); + await result; + expect(h.prefs.snapshot().data).toBeUndefined(); + } finally { + read.resolve({ sections: [], assignments: {}, starred: [], muted: [] }); + h.owner.dispose(); + } + }, +); + +it("reconciles a failed creation after a queued move without deadlocking or losing its result", async () => { + const h = await setup(); + const save = h.assignment.getMockImplementation(); + assert(save); + h.assignment.mockImplementationOnce(async (intent) => { + await save(intent); + throw new Error("response lost"); + }); + const section = { id: "accepted", name: "Launch" }; + try { + const creating = expect(h.prefs.createSection(section)).rejects.toThrow( + "response lost", + ); + const moving = h.prefs.assign("beta", "later"); + await Promise.all([creating, moving]); + expect(h.prefs.snapshot().data?.sections.at(-1)).toMatchObject(section); + expect(h.prefs.snapshot().data?.assignments.beta).toBe("later"); + expect(h.assignment).toHaveBeenCalledTimes(2); + } finally { + h.owner.dispose(); + } +}); diff --git a/src/features/relay/sidebar-preferences-store.ts b/src/features/relay/sidebar-preferences-store.ts index a41b90cc48..fdaac12ed7 100644 --- a/src/features/relay/sidebar-preferences-store.ts +++ b/src/features/relay/sidebar-preferences-store.ts @@ -672,7 +672,20 @@ export function createSidebarPreferencesStore({ : undefined); if (!writable() || snapshot.cached || !create) return Promise.reject(new Error(failure)); - return writeGroups(failure, (signal) => create(section, signal)); + const writeGeneration = generation; + return writeGroups(failure, (signal) => create(section, signal)).catch( + async (error) => { + if (!closed && generation === writeGeneration) { + // The relay may have accepted the creation. Reconcile before + // the form can discard its ID, without replaying the write or + // joining a read that began before publication. + active?.controller.abort(); + active = undefined; + await refresh(); + } + throw error; + }, + ); }, get sectionOrderWritable() { return orderWritable(); diff --git a/tests/browser/sidebar-section-creation.spec.mjs b/tests/browser/sidebar-section-creation.spec.mjs index cf7417d28f..9fe7d0a0c9 100644 --- a/tests/browser/sidebar-section-creation.spec.mjs +++ b/tests/browser/sidebar-section-creation.spec.mjs @@ -42,7 +42,7 @@ test("Channels offers empty-section creation with keyboard focus and confirmed p await expect(field).toHaveValue("Launch"); const creations = []; let lostResponse = false; - await page.route("**/sidebar-assignment", async (route) => { + const loseCreationResponse = async (route) => { creations.push(route.request().postDataJSON()); const response = await route.fetch(); if (!lostResponse && response.ok()) { @@ -57,7 +57,8 @@ test("Channels offers empty-section creation with keyboard focus and confirmed p body: JSON.stringify({ error: "Section confirmation unavailable" }), }); } else await route.fulfill({ response }); - }); + }; + await page.route("**/sidebar-assignment", loseCreationResponse); await field.press("Enter"); await expect(dialog.getByRole("alert")).toContainText( "Relay request failed (502)", @@ -89,6 +90,43 @@ test("Channels offers empty-section creation with keyboard focus and confirmed p '[data-sidebar-section="group:work"] [data-channel-id="beta"]', ), ).toBeVisible(); + // An accepted creation must also appear if the user cancels instead of + // retrying. Reopening the form must not hide that confirmed section. + lostResponse = false; + await page.route("**/sidebar-assignment", loseCreationResponse); + await trigger.click(); + await page + .getByRole("menuitem", { name: "Create section", exact: true }) + .click(); + await field.fill("Cancel check"); + await dialog.getByRole("button", { name: "Create", exact: true }).click(); + await expect(dialog.getByRole("alert")).toContainText( + "Relay request failed (502)", + ); + const accepted = page + .locator("summary") + .filter({ hasText: /^Cancel check$/ }); + await expect(accepted).toHaveCount(1); + await dialog.getByRole("button", { name: "Cancel", exact: true }).click(); + await expect(trigger).toBeFocused(); + await expect(accepted).toBeVisible(); + await trigger.click(); + await page + .getByRole("menuitem", { name: "Create section", exact: true }) + .click(); + await expect(field).toHaveValue(""); + await expect(accepted).toHaveCount(1); + await dialog.getByRole("button", { name: "Cancel", exact: true }).click(); + await expect(dialog).toHaveCount(0); + expect(creations).toHaveLength(3); + expect(app.report.sidebarPublications).toHaveLength(2); + await page.unroute("**/sidebar-assignment"); + await page.reload(); + await openPage(page, "Messages"); + await expect(accepted).toHaveCount(1); + await expect( + page.locator("summary").filter({ hasText: /^Launch$/ }), + ).toHaveCount(1); }); async function startDrag(page, locator) { From c007c44b188b1182e53507d88eece1902b4fd896 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Wed, 28 Oct 2026 01:34:43 -0700 Subject: [PATCH 15/21] fix(sidebar): reconcile uncertain creations with writer-backed reads Route uncertain empty-section recovery through a fresh writer-backed preference read and retain that requirement across failed reconciliation, explicit Retry and cache clearing. Admit only the supported consistency hint on the bounded preference batch and cover replica lag through the real session/broker and browser flows. Co-authored-by: Carl Signed-off-by: Taylor Ho --- .../sidebar-preference-writes.test.mjs | 75 ++++++++++++- docs/channels.md | 7 +- src/features/relay/session.ts | 40 ++++--- .../relay/sidebar-moves-store.test.ts | 104 ++++++++++++------ .../relay/sidebar-preferences-store.ts | 22 +++- .../relay/sidebar-preferences.test.ts | 34 ++++++ src/features/relay/sidebar-preferences.ts | 11 +- .../browser/sidebar-section-creation.spec.mjs | 29 +++++ 8 files changed, 260 insertions(+), 62 deletions(-) diff --git a/browser-host/sidebar-preference-writes.test.mjs b/browser-host/sidebar-preference-writes.test.mjs index ae0f000909..957fcee898 100644 --- a/browser-host/sidebar-preference-writes.test.mjs +++ b/browser-host/sidebar-preference-writes.test.mjs @@ -12,6 +12,7 @@ import { SIDEBAR_HEAD_BYTES } from "./sidebar-preferences.mjs"; import { relayBrokerPlugin } from "./relay-broker.mjs"; import { prepareSidebarStar } from "./sidebar-toggle.mjs"; import { connectBrokerTransport } from "../src/features/relay/transport.ts"; +import { createRelaySession } from "../src/features/relay/session.ts"; import { fixtureRelayUrl, fixtureAliases } from "../tests/relay-config.ts"; const disposals = []; @@ -84,11 +85,15 @@ async function harness(chunkBytes) { } if (queryFailure) return queryFailure; // Model an empty stale replica: only strong reads observe the writer head. - const head = - body[0].consistency === "strong" - ? heads.get(body[0]["#d"][0]) - : undefined; - return Response.json(head ? [head] : []); + return Response.json( + body.flatMap((filter) => { + const head = + filter.consistency === "strong" + ? heads.get(filter["#d"][0]) + : undefined; + return head ? [head] : []; + }), + ); }, }).configureServer({ httpServer: server, @@ -146,6 +151,66 @@ async function harness(chunkBytes) { }, }; } +it.each([false, true])( + "uncertain creation reconciles through the writer despite replica lag (recovery initially fails: %s)", + async (fails) => { + const h = await harness(); + const owner = createRelaySession({ + ...h.transport, + writeSidebarAssignment: async (intent, signal) => { + await h.transport.writeSidebarAssignment(intent, signal); + if (fails) + h.failQuery( + Response.json({ error: "writer unavailable" }, { status: 503 }), + ); + throw new Error("response lost"); + }, + }); + const preferences = owner.session.sidebarPreferences; + const section = { + id: "12345678-1234-1234-1234-123456789abc", + name: "Launch", + }; + try { + await preferences.ensure(); + await expect(preferences.createSection(section)).rejects.toThrow( + "response lost", + ); + if (fails) { + expect(preferences.sectionCreationWritable).toBe(false); + await expect( + preferences.createSection({ + ...section, + id: "22345678-1234-1234-1234-123456789abc", + }), + ).rejects.toThrow("unavailable"); + h.failQuery(undefined); + await preferences.refresh(); + } + expect(preferences.snapshot().data.sections).toEqual([ + { ...section, order: 0 }, + ]); + expect(preferences.sectionCreationWritable).toBe(true); + const batches = h.calls.filter( + ({ body }) => Array.isArray(body) && body.length === 5, + ); + expect(batches).toHaveLength(fails ? 3 : 2); + expect( + batches[0].body.every((filter) => filter.consistency === undefined), + ).toBe(true); + for (const { body } of batches.slice(1)) + expect(body.every((filter) => filter.consistency === "strong")).toBe( + true, + ); + expect(h.calls.filter(({ url }) => url.endsWith("/events"))).toHaveLength( + 1, + ); + } finally { + owner.dispose(); + } + }, +); + it("real broker Star roundtrip signs scoped requests and confirms before projecting", async () => { const h = await harness(), signal = new AbortController().signal; diff --git a/docs/channels.md b/docs/channels.md index 8282d2fdee..80206e367c 100644 --- a/docs/channels.md +++ b/docs/channels.md @@ -154,9 +154,10 @@ placement, rollback notice and Retry. The **Channels** ellipsis menu also offers **Create section**, which creates an empty section without moving a channel. Its dialog retains the name and section ID on failure and waits for confirmation before closing. After a failed creation, -the session refreshes preferences before allowing dismissal so an accepted save -stays visible even after Cancel. If that read fails too, new creation is gated -until preferences can be read successfully. Dragging a channel into +the session refreshes preferences from the writer before allowing dismissal so +an accepted save stays visible even after Cancel. If that read fails too, new +creation is gated and Retry retains writer routing until reconciliation succeeds. +Ordinary startup remains replica-eligible. Dragging a channel into the gap above the first custom section expands an **Add to new section** drop target inside the sidebar. With no custom sections, the target is above **Channels**; **Starred** keeps its pinned position. The created section appears diff --git a/src/features/relay/session.ts b/src/features/relay/session.ts index 4d79019372..f5f9229852 100644 --- a/src/features/relay/session.ts +++ b/src/features/relay/session.ts @@ -1365,25 +1365,29 @@ export function createRelaySession( signal: lifetime.signal, }) : undefined; + const readPreferences = async ( + signal?: AbortSignal, + consistency?: "strong", + ) => { + const decode = transport?.decodeSidebarPreferences; + if (closed || !transport || !decode) + throw new Error("Saved sidebar preferences are unavailable in this host"); + const combined = AbortSignal.any([ + lifetime.signal, + AbortSignal.timeout(10_000), + ...(signal ? [signal] : []), + ]); + return readSidebarPreferences( + preferenceReader, + transport.viewer, + decode, + combined, + consistency, + ); + }; const sidebarPreferences = createSidebarPreferencesStore({ - read: async (signal?: AbortSignal) => { - const decode = transport?.decodeSidebarPreferences; - if (closed || !transport || !decode) - throw new Error( - "Saved sidebar preferences are unavailable in this host", - ); - const combined = AbortSignal.any([ - lifetime.signal, - AbortSignal.timeout(10_000), - ...(signal ? [signal] : []), - ]); - return readSidebarPreferences( - preferenceReader, - transport.viewer, - decode, - combined, - ); - }, + read: readPreferences, + readAfterWrite: (signal) => readPreferences(signal, "strong"), available: !!transport?.decodeSidebarPreferences, write: (() => { const write = transport?.writeSidebarAssignment; diff --git a/src/features/relay/sidebar-moves-store.test.ts b/src/features/relay/sidebar-moves-store.test.ts index f82c1a5583..10a17b315e 100644 --- a/src/features/relay/sidebar-moves-store.test.ts +++ b/src/features/relay/sidebar-moves-store.test.ts @@ -28,6 +28,7 @@ async function setup() { muted: [], }; const read = vi.fn(async () => data); + const readAfterWrite = vi.fn(async () => data); const assignment = vi.fn(async (intent: SidebarAssignmentIntent) => { const id = intent.createSection?.id ?? intent.sectionId; const assignments = { ...data.assignments }; @@ -77,6 +78,7 @@ async function setup() { }); const owner = createSidebarPreferencesStore({ read, + readAfterWrite, available: true, write: assignment, writeStar: star, @@ -84,7 +86,16 @@ async function setup() { writeSort: sort, }); await owner.queries.ensure(); - return { owner, prefs: owner.queries, assignment, star, read, mute, sort }; + return { + owner, + prefs: owner.queries, + assignment, + star, + read, + readAfterWrite, + mute, + sort, + }; } it("moves immediately, keeps later intent through older failure, and serializes both records", async () => { @@ -634,36 +645,42 @@ it("refreshes an accepted creation before reporting a lost response, superseding } }); -it("gates new creation when reconciliation fails until preferences can be read", async () => { - const h = await setup(); - const save = h.assignment.getMockImplementation(); - assert(save); - h.assignment.mockImplementationOnce(async (intent) => { - await save(intent); - throw new Error("response lost"); - }); - h.read.mockRejectedValueOnce(new Error("read offline")); - const section = { id: "accepted", name: "Launch" }; - try { - await expect(h.prefs.createSection(section)).rejects.toThrow( - "response lost", - ); - expect(h.prefs.snapshot()).toMatchObject({ - status: "error", - error: "read offline", +it.each([false, true])( + "gates new creation until writer reconciliation succeeds (clear cache: %s)", + async (clearCache) => { + const h = await setup(); + const save = h.assignment.getMockImplementation(); + assert(save); + h.assignment.mockImplementationOnce(async (intent) => { + await save(intent); + throw new Error("response lost"); }); - expect(h.prefs.sectionCreationWritable).toBe(false); - await expect( - h.prefs.createSection({ id: "duplicate", name: "Launch" }), - ).rejects.toThrow("unavailable"); - expect(h.assignment).toHaveBeenCalledOnce(); - await h.prefs.refresh(); - expect(h.prefs.sectionCreationWritable).toBe(true); - expect(h.prefs.snapshot().data?.sections.at(-1)).toMatchObject(section); - } finally { - h.owner.dispose(); - } -}); + h.readAfterWrite.mockRejectedValueOnce(new Error("read offline")); + const section = { id: "accepted", name: "Launch" }; + try { + await expect(h.prefs.createSection(section)).rejects.toThrow( + "response lost", + ); + expect(h.prefs.snapshot()).toMatchObject({ + status: "error", + error: "read offline", + }); + expect(h.prefs.sectionCreationWritable).toBe(false); + await expect( + h.prefs.createSection({ id: "duplicate", name: "Launch" }), + ).rejects.toThrow("unavailable"); + expect(h.assignment).toHaveBeenCalledOnce(); + if (clearCache) h.owner.clear(); + await h.prefs.refresh(); + expect(h.prefs.sectionCreationWritable).toBe(true); + expect(h.prefs.snapshot().data?.sections.at(-1)).toMatchObject(section); + expect(h.readAfterWrite).toHaveBeenCalledTimes(2); + expect(h.read).toHaveBeenCalledOnce(); + } finally { + h.owner.dispose(); + } + }, +); it.each(["clear", "dispose"] as const)( "fences creation reconciliation after %s", @@ -672,7 +689,7 @@ it.each(["clear", "dispose"] as const)( const read = deferred(); const entered = deferred(); h.assignment.mockRejectedValueOnce(new Error("response lost")); - h.read.mockImplementationOnce(() => { + h.readAfterWrite.mockImplementationOnce(() => { entered.resolve(); return read.promise; }); @@ -719,3 +736,28 @@ it("reconciles a failed creation after a queued move without deadlocking or losi h.owner.dispose(); } }); + +it("reconciles an accepted creation against the writer while ordinary reads lag", async () => { + const h = await setup(); + const before = h.prefs.snapshot().data; + assert(before); + h.read.mockResolvedValue(before); + const save = h.assignment.getMockImplementation(); + assert(save); + h.assignment.mockImplementationOnce(async (intent) => { + await save(intent); + throw new Error("response lost"); + }); + const section = { id: "accepted", name: "Launch" }; + try { + await expect(h.prefs.createSection(section)).rejects.toThrow( + "response lost", + ); + expect(h.prefs.snapshot().data?.sections.at(-1)).toMatchObject(section); + expect(h.read).toHaveBeenCalledOnce(); + expect(h.readAfterWrite).toHaveBeenCalledOnce(); + expect(h.assignment).toHaveBeenCalledOnce(); + } finally { + h.owner.dispose(); + } +}); diff --git a/src/features/relay/sidebar-preferences-store.ts b/src/features/relay/sidebar-preferences-store.ts index fdaac12ed7..be8dd5c101 100644 --- a/src/features/relay/sidebar-preferences-store.ts +++ b/src/features/relay/sidebar-preferences-store.ts @@ -60,6 +60,7 @@ type Snapshot = Readonly<{ /** One bounded account-preference projection per relay session, not per page. */ export function createSidebarPreferencesStore({ read, + readAfterWrite = read, available, write, writeStar, @@ -75,6 +76,10 @@ export function createSidebarPreferencesStore({ writeFilter, }: Readonly<{ read: (signal?: AbortSignal) => Promise; + /** Authoritative reconciliation after uncertain creation, including Retry. */ + readAfterWrite?: + | ((signal?: AbortSignal) => Promise) + | undefined; available: boolean; write?: SidebarAssignmentMutator | undefined; writeStar?: SidebarStarMutator | undefined; @@ -104,6 +109,8 @@ export function createSidebarPreferencesStore({ let confirmed: SidebarPreferences | undefined; let readFailure: string | undefined; let readTransient = false; + // Retain writer routing through failed reads and cache clears until reconciled. + let creationUncertain = false; const writable = () => !closed && !!confirmed && @@ -334,7 +341,9 @@ export function createSidebarPreferencesStore({ job.promise = Promise.resolve().then(async () => { if (closed || controller.signal.aborted) return; try { - const data = await read(controller.signal); + const data = await (creationUncertain ? readAfterWrite : read)( + controller.signal, + ); if ( closed || controller.signal.aborted || @@ -343,6 +352,7 @@ export function createSidebarPreferencesStore({ ) return; readFailure = undefined; + creationUncertain = false; confirmed = data; project(); } catch (error) { @@ -656,7 +666,12 @@ export function createSidebarPreferencesStore({ ); }, get sectionCreationWritable() { - return writable() && !snapshot.cached && !!(createSection || write); + return ( + writable() && + !creationUncertain && + !snapshot.cached && + !!(createSection || write) + ); }, createSection(section: Readonly<{ id: string; name: string }>) { const failure = groupsOnly @@ -670,7 +685,7 @@ export function createSidebarPreferencesStore({ signal: AbortSignal, ) => write({ createSection: section }, signal) : undefined); - if (!writable() || snapshot.cached || !create) + if (!writable() || creationUncertain || snapshot.cached || !create) return Promise.reject(new Error(failure)); const writeGeneration = generation; return writeGroups(failure, (signal) => create(section, signal)).catch( @@ -681,6 +696,7 @@ export function createSidebarPreferencesStore({ // joining a read that began before publication. active?.controller.abort(); active = undefined; + creationUncertain = true; await refresh(); } throw error; diff --git a/src/features/relay/sidebar-preferences.test.ts b/src/features/relay/sidebar-preferences.test.ts index f09ad9005a..75f9bcc151 100644 --- a/src/features/relay/sidebar-preferences.test.ts +++ b/src/features/relay/sidebar-preferences.test.ts @@ -670,6 +670,19 @@ it.each([ }, ); +it("writer reconciliation cannot join an older in-flight preference read", async () => { + const read = vi.fn(async () => []); + const decode = vi.fn(async () => expected); + const signal = new AbortController().signal; + await readSidebarPreferences({ read }, "viewer", decode, signal, "strong"); + expect(read).toHaveBeenCalledWith( + expect.arrayContaining([ + expect.objectContaining({ consistency: "strong" }), + ]), + { signal, priority: "foreground", fresh: true }, + ); +}); + it("admits only the complete bounded sidebar batch beyond the normal reader limit", async () => { const { SIDEBAR_COORDINATES, isSidebarPreferencesBatch } = await import( "./sidebar-preferences" @@ -681,6 +694,27 @@ it("admits only the complete bounded sidebar batch beyond the normal reader limi limit: 1, })); expect(isSidebarPreferencesBatch(filters)).toBe(true); + expect( + isSidebarPreferencesBatch( + filters.map((filter) => ({ ...filter, consistency: "strong" })), + ), + ).toBe(true); + expect( + isSidebarPreferencesBatch( + filters.map((filter) => ({ + ...filter, + consistency: "weak", + })) as unknown as typeof filters, + ), + ).toBe(false); + expect( + isSidebarPreferencesBatch( + filters.map((filter) => ({ + ...filter, + consistency: true, + })) as unknown as typeof filters, + ), + ).toBe(false); expect(isSidebarPreferencesBatch(filters.slice(1))).toBe(false); expect( isSidebarPreferencesBatch([ diff --git a/src/features/relay/sidebar-preferences.ts b/src/features/relay/sidebar-preferences.ts index 7b5bf3c813..96aedaeb2b 100644 --- a/src/features/relay/sidebar-preferences.ts +++ b/src/features/relay/sidebar-preferences.ts @@ -35,8 +35,9 @@ export function isSidebarPreferencesBatch( filter["#d"][0] as (typeof SIDEBAR_COORDINATES)[number], ) && filter.limit === 1 && + (filter.consistency === undefined || filter.consistency === "strong") && Object.keys(filter).every((key) => - ["kinds", "authors", "#d", "limit"].includes(key), + ["kinds", "authors", "#d", "limit", "consistency"].includes(key), ), ) ); @@ -323,6 +324,7 @@ export async function readSidebarPreferences( viewer: string, decode: SidebarDecoder, signal: AbortSignal, + consistency?: "strong", ): Promise { let events: readonly RelayEvent[]; try { @@ -332,8 +334,13 @@ export async function readSidebarPreferences( authors: [viewer], "#d": [coordinate], limit: 1, + ...(consistency ? { consistency } : {}), })), - { signal, priority: "foreground" }, + { + signal, + priority: "foreground", + ...(consistency ? { fresh: true } : {}), + }, ); signal.throwIfAborted(); } catch (error) { diff --git a/tests/browser/sidebar-section-creation.spec.mjs b/tests/browser/sidebar-section-creation.spec.mjs index 9fe7d0a0c9..786cd7868c 100644 --- a/tests/browser/sidebar-section-creation.spec.mjs +++ b/tests/browser/sidebar-section-creation.spec.mjs @@ -42,6 +42,32 @@ test("Channels offers empty-section creation with keyboard focus and confirmed p await expect(field).toHaveValue("Launch"); const creations = []; let lostResponse = false; + let awaitingReconciliation = false; + let writerReads = 0; + await page.route("**/query", async (route) => { + const filters = route.request().postDataJSON(); + if ( + awaitingReconciliation && + Array.isArray(filters) && + filters.length === 5 && + filters.some((filter) => filter["#d"]?.includes("channel-sections")) + ) { + if (filters.every((filter) => filter.consistency === "strong")) { + writerReads++; + awaitingReconciliation = false; + } else { + // A replica is still behind the accepted write. Only writer routing + // can reconcile this creation before the dialog discards its ID. + await route.fulfill({ + status: 200, + contentType: "application/json", + body: "[]", + }); + return; + } + } + await route.continue(); + }); const loseCreationResponse = async (route) => { creations.push(route.request().postDataJSON()); const response = await route.fetch(); @@ -49,6 +75,7 @@ test("Channels offers empty-section creation with keyboard focus and confirmed p // The real broker accepts and persists the write, but its response never // reaches the dialog. The retry must reconcile that exact creation. lostResponse = true; + awaitingReconciliation = true; app.report.sidebarAssignmentFailures ??= []; app.report.sidebarAssignmentFailures.push(route.request().url()); await route.fulfill({ @@ -63,6 +90,7 @@ test("Channels offers empty-section creation with keyboard focus and confirmed p await expect(dialog.getByRole("alert")).toContainText( "Relay request failed (502)", ); + expect(writerReads).toBe(1); await expect.poll(() => app.report.sidebarPublications?.length ?? 0).toBe(1); await expect(field).toBeDisabled(); await page.keyboard.type("es"); @@ -103,6 +131,7 @@ test("Channels offers empty-section creation with keyboard focus and confirmed p await expect(dialog.getByRole("alert")).toContainText( "Relay request failed (502)", ); + expect(writerReads).toBe(2); const accepted = page .locator("summary") .filter({ hasText: /^Cancel check$/ }); From f5b4215580272ae46c726e4fdd1c522c5996b5be Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Wed, 28 Oct 2026 10:10:55 -0700 Subject: [PATCH 16/21] fix(sidebar): reconcile creation across concurrent preference writes Co-authored-by: Carl Signed-off-by: Taylor Ho --- docs/channels.md | 4 +- .../ChannelInventory.test.tsx | 57 +++++++++ .../channel-navigation/ChannelInventory.tsx | 13 +- .../relay/sidebar-moves-store.test.ts | 119 ++++++++++++++++++ .../relay/sidebar-preferences-store.ts | 77 +++++++----- 5 files changed, 234 insertions(+), 36 deletions(-) diff --git a/docs/channels.md b/docs/channels.md index 80206e367c..8467f0dc17 100644 --- a/docs/channels.md +++ b/docs/channels.md @@ -156,7 +156,9 @@ empty section without moving a channel. Its dialog retains the name and section ID on failure and waits for confirmation before closing. After a failed creation, the session refreshes preferences from the writer before allowing dismissal so an accepted save stays visible even after Cancel. If that read fails too, new -creation is gated and Retry retains writer routing until reconciliation succeeds. +creation is gated across menu, drop and create-and-move paths, and Retry retains +writer routing until reconciliation succeeds. Recovery waits for all queued +preference writes and repeats a writer read invalidated by a concurrent write. Ordinary startup remains replica-eligible. Dragging a channel into the gap above the first custom section expands an **Add to new section** drop target inside the sidebar. With no custom sections, the target is above diff --git a/src/features/channel-navigation/ChannelInventory.test.tsx b/src/features/channel-navigation/ChannelInventory.test.tsx index fb3b1a73e4..d0bdcb095b 100644 --- a/src/features/channel-navigation/ChannelInventory.test.tsx +++ b/src/features/channel-navigation/ChannelInventory.test.tsx @@ -1189,6 +1189,63 @@ it("rejects a deferred header Create section after its navigation origin retires preferences.dispose(); }); +it("gates all creation entry points while uncertain creation reconciles, preserving ordinary moves", async () => { + const data = { sections: [], assignments: {}, starred: [], muted: [] }; + let finish!: (value: SidebarPreferences) => void; + let entered!: () => void; + const started = new Promise((resolve) => { + entered = resolve; + }); + const read = new Promise((resolve) => { + finish = resolve; + }); + const preferences = createSidebarPreferencesStore({ + read: async () => data, + readAfterWrite: () => { + entered(); + return read; + }, + available: true, + write: vi.fn().mockRejectedValue(new Error("response lost")), + writeStar: vi.fn(async () => []), + }); + await preferences.queries.ensure(); + const creating = expect( + preferences.queries.createSection({ id: "saved", name: "Saved" }), + ).rejects.toThrow("response lost"); + try { + await started; + const h = fixture(preferences.queries); + const user = userEvent.setup(); + render(h.view("alpha")); + const row = await screen.findByRole("button", { name: "beta" }); + expect(document.querySelector("[data-create-section-trigger]")).toBeNull(); + fireEvent.keyDown(row, { key: "ContextMenu" }); + await user.hover( + await screen.findByRole("menuitem", { name: "Move channel" }), + ); + expect( + await screen.findByRole("menuitem", { name: "Create new…" }), + ).toHaveAttribute("aria-disabled", "true"); + expect( + screen.getByRole("menuitemradio", { name: "Starred" }), + ).not.toHaveAttribute("aria-disabled", "true"); + await act(async () => { + finish(data); + await creating; + }); + await waitFor(() => + expect( + document.querySelector("[data-create-section-trigger]"), + ).not.toBeNull(), + ); + } finally { + finish(data); + await creating; + preferences.dispose(); + } +}); + it("clears only the removed section's collapsed intent after confirmation, never on cancel or failure", async () => { const user = userEvent.setup(); const current: SidebarPreferences = { diff --git a/src/features/channel-navigation/ChannelInventory.tsx b/src/features/channel-navigation/ChannelInventory.tsx index be24ab8cbd..3dbed1d359 100644 --- a/src/features/channel-navigation/ChannelInventory.tsx +++ b/src/features/channel-navigation/ChannelInventory.tsx @@ -724,6 +724,7 @@ export function ChannelInventory({ { if (surface) { surface.close(() => false); @@ -731,6 +732,7 @@ export function ChannelInventory({ if ( mounted.current && !surface.signal?.aborted && + queries.sidebarPreferences.sectionCreationWritable && relay.snapshot().session === queries ) setCreatingFor({ channel, surface }); @@ -1134,7 +1136,7 @@ export function ChannelInventory({ if ( mounted.current && channel && - queries.sidebarPreferences.writable + queries.sidebarPreferences.sectionCreationWritable ) setCreatingFor({ channel, position: "first" }); }); @@ -1202,7 +1204,10 @@ export function ChannelInventory({ {filteredSections.map((section) => ( { @@ -1687,7 +1692,7 @@ export function ChannelInventory({ // the live gate before handing its draft to the optimistic store. if ( creatingFor.surface?.signal?.aborted || - !queries.sidebarPreferences.writable + !queries.sidebarPreferences.sectionCreationWritable ) return false; moveChannel( diff --git a/src/features/relay/sidebar-moves-store.test.ts b/src/features/relay/sidebar-moves-store.test.ts index 10a17b315e..a63d5a47b4 100644 --- a/src/features/relay/sidebar-moves-store.test.ts +++ b/src/features/relay/sidebar-moves-store.test.ts @@ -761,3 +761,122 @@ it("reconciles an accepted creation against the writer while ordinary reads lag" h.owner.dispose(); } }); + +it("waits for a queued non-move write before reconciling an uncertain creation", async () => { + const h = await setup(); + const mute = deferred(); + const started = deferred(); + h.assignment.mockRejectedValueOnce(new Error("response lost")); + h.mute.mockImplementationOnce(() => { + started.resolve(); + return mute.promise; + }); + const creating = expect( + h.prefs.createSection({ id: "accepted", name: "Launch" }), + ).rejects.toThrow("response lost"); + const muting = h.prefs.setMute("alpha", true); + try { + await started.promise; + await flush(); + expect(h.readAfterWrite).not.toHaveBeenCalled(); + mute.resolve(["alpha"]); + await Promise.all([creating, muting]); + expect(h.readAfterWrite).toHaveBeenCalledOnce(); + expect(h.prefs.sectionCreationWritable).toBe(true); + } finally { + mute.resolve([]); + await Promise.allSettled([creating, muting]); + h.owner.dispose(); + } +}); + +it.each(["mute", "sort", "writer-failure"] as const)( + "reconciles again when %s invalidates an in-flight writer read", + async (kind) => { + const h = await setup(); + const read = deferred(); + const entered = deferred(); + const save = h.assignment.getMockImplementation(); + assert(save); + const authoritative = h.readAfterWrite.getMockImplementation(); + assert(authoritative); + h.assignment.mockImplementationOnce(async (intent) => { + await save(intent); + throw new Error("response lost"); + }); + let first!: SidebarPreferences; + h.readAfterWrite.mockImplementationOnce(async () => { + first = await authoritative(); + entered.resolve(); + return read.promise; + }); + if (kind === "writer-failure") + h.readAfterWrite.mockRejectedValueOnce(new Error("writer offline")); + const section = { id: "accepted", name: "Launch" }; + const creating = expect(h.prefs.createSection(section)).rejects.toThrow( + "response lost", + ); + try { + await entered.promise; + if (kind === "sort") + await h.prefs.setSort("channels", "recent", ["work", "later"]); + else await h.prefs.setMute("alpha", true); + read.resolve(first); + await creating; + expect(h.readAfterWrite).toHaveBeenCalledTimes(2); + if (kind === "writer-failure") { + expect(h.prefs.snapshot()).toMatchObject({ + status: "error", + error: "writer offline", + }); + expect(h.prefs.sectionCreationWritable).toBe(false); + await h.prefs.refresh(); + } + expect(h.prefs.snapshot().data?.sections.at(-1)).toMatchObject(section); + expect(h.prefs.snapshot().data).toMatchObject( + kind === "sort" + ? { sort: { channels: "recent" } } + : { muted: ["alpha"] }, + ); + expect(h.prefs.sectionCreationWritable).toBe(true); + expect(h.assignment).toHaveBeenCalledOnce(); + } finally { + read.resolve(first); + await creating; + h.owner.dispose(); + } + }, +); + +it("gates create-and-move during reconciliation while allowing existing destinations", async () => { + const h = await setup(); + const entered = deferred(); + const read = deferred(); + const authoritative = h.readAfterWrite.getMockImplementation(); + assert(authoritative); + h.assignment.mockRejectedValueOnce(new Error("response lost")); + h.readAfterWrite.mockImplementationOnce(() => { + entered.resolve(); + return read.promise; + }); + const creating = expect( + h.prefs.createSection({ id: "accepted", name: "Launch" }), + ).rejects.toThrow("response lost"); + try { + await entered.promise; + expect(h.prefs.writable).toBe(true); + await expect( + h.prefs.createAndAssign("beta", { id: "duplicate", name: "Launch" }), + ).rejects.toThrow("unavailable"); + expect(h.assignment).toHaveBeenCalledOnce(); + await h.prefs.assign("beta", "later"); + read.resolve(await authoritative()); + await creating; + expect(h.prefs.snapshot().data?.assignments.beta).toBe("later"); + expect(h.prefs.sectionCreationWritable).toBe(true); + } finally { + read.resolve(await authoritative()); + await creating; + h.owner.dispose(); + } +}); diff --git a/src/features/relay/sidebar-preferences-store.ts b/src/features/relay/sidebar-preferences-store.ts index be8dd5c101..774fff8e7f 100644 --- a/src/features/relay/sidebar-preferences-store.ts +++ b/src/features/relay/sidebar-preferences-store.ts @@ -335,42 +335,54 @@ export function createSidebarPreferencesStore({ } if (active) return active.promise; const controller = new AbortController(); - const refreshMutation = mutation; + const requestedMutation = mutation; const job = { controller, promise: Promise.resolve() }; active = job; job.promise = Promise.resolve().then(async () => { if (closed || controller.signal.aborted) return; + const current = () => + !closed && !controller.signal.aborted && active === job; try { - const data = await (creationUncertain ? readAfterWrite : read)( - controller.signal, - ); - if ( - closed || - controller.signal.aborted || - active !== job || - mutation !== refreshMutation - ) - return; - readFailure = undefined; - creationUncertain = false; - confirmed = data; - project(); - } catch (error) { - if ( - !closed && - !controller.signal.aborted && - active === job && - mutation === refreshMutation - ) { - readFailure = error instanceof Error ? error.message : String(error); - readTransient = isTransientFailure(error); - const { transient: _transient, ...rest } = snapshot; - publish({ - ...rest, - status: "error", - error: readFailure, - ...(readTransient ? { transient: true as const } : {}), - }); + while (current()) { + // Other preference families share the queue. Read after they settle, + // and repeat if a write admitted during the read invalidates it. + if (creationUncertain) await writeQueue; + if (!current()) return; + const refreshMutation = creationUncertain + ? mutation + : requestedMutation; + try { + const data = await (creationUncertain ? readAfterWrite : read)( + controller.signal, + ); + if (!current()) return; + if (mutation !== refreshMutation) { + if (creationUncertain) continue; + return; + } + readFailure = undefined; + creationUncertain = false; + confirmed = data; + project(); + return; + } catch (error) { + if (!current()) return; + if (mutation !== refreshMutation) { + if (creationUncertain) continue; + return; + } + readFailure = + error instanceof Error ? error.message : String(error); + readTransient = isTransientFailure(error); + const { transient: _transient, ...rest } = snapshot; + publish({ + ...rest, + status: "error", + error: readFailure, + ...(readTransient ? { transient: true as const } : {}), + }); + return; + } } } finally { if (active === job) active = undefined; @@ -401,6 +413,7 @@ export function createSidebarPreferencesStore({ ): Promise { if ( !writable() || + (creationUncertain && "createSection" in destination) || !snapshot.data || !confirmed || !write || @@ -432,6 +445,8 @@ export function createSidebarPreferencesStore({ .then(async () => { try { check(); + if (creationUncertain && "createSection" in destination) + throw new Error("Section creation is unavailable"); const starring = "starred" in destination; // Always check the fresh assignment head on removal, even if cached // placement has no assignment. Another device may have changed it. From 05a20e52d45ded83ecf72308e9c33ee7668f97ea Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Wed, 28 Oct 2026 10:11:42 -0700 Subject: [PATCH 17/21] fix(sidebar): preserve creation focus and unlock unwritten drafts Co-authored-by: Carl Signed-off-by: Taylor Ho --- .../channels/CreateSidebarSection.test.tsx | 30 ++++++++++++++----- src/bundled/channels/CreateSidebarSection.tsx | 11 +++++-- .../browser/sidebar-section-creation.spec.mjs | 13 ++++++-- 3 files changed, 41 insertions(+), 13 deletions(-) diff --git a/src/bundled/channels/CreateSidebarSection.test.tsx b/src/bundled/channels/CreateSidebarSection.test.tsx index 3ff0dfd4d7..a7e07ca2c8 100644 --- a/src/bundled/channels/CreateSidebarSection.test.tsx +++ b/src/bundled/channels/CreateSidebarSection.test.tsx @@ -13,14 +13,15 @@ import { CreateSidebarSection } from "./CreateSidebarSection"; afterEach(cleanup); -it("retains a rejected draft and its identity through preference recovery", async () => { +it("keeps a gated draft editable and submits its edited name with a fresh identity", async () => { + const user = userEvent.setup(); const create = vi.fn().mockReturnValue(false); const retry = vi.fn().mockResolvedValue(undefined); const props = { channelName: "Beta", create, close: vi.fn(), - writable: false, + writable: true, refreshing: false, retry, }; @@ -41,23 +42,34 @@ it("retains a rejected draft and its identity through preference recovery", asyn name: "Launch", }); expect(input).toHaveValue(" Launch "); + await waitFor(() => expect(input).not.toHaveAttribute("readonly")); + await user.clear(input); + await user.type(input, "Release"); + expect(input).toHaveValue("Release"); + view.rerender(); await waitFor(() => expect(screen.getByRole("alert")).toHaveTextContent("Retry preferences"), ); fireEvent.click(screen.getByRole("button", { name: "Retry preferences" })); expect(retry).toHaveBeenCalledTimes(1); - view.rerender(); + view.rerender( + , + ); expect( screen.getByRole("button", { name: "Retry preferences" }), ).toBeDisabled(); - view.rerender(); - expect(input).toHaveValue(" Launch "); + view.rerender(); + expect(input).toHaveValue("Release"); create.mockReturnValue(true); view.rerender(); expect(screen.queryByRole("alert")).toBeNull(); await submit(); expect(create).toHaveBeenCalledTimes(2); - expect(create.mock.calls[1]).toEqual(create.mock.calls[0]); + expect(create.mock.calls[1]?.[0]).toEqual({ + id: expect.any(String), + name: "Release", + }); + expect(create.mock.calls[1]?.[0].id).not.toBe(create.mock.calls[0]?.[0].id); await submit(); expect(create).toHaveBeenCalledTimes(2); }); @@ -92,7 +104,8 @@ it("keeps the submitted name and identity through a failed save, blocking dismis fireEvent.change(input, { target: { value: " Launch " } }); const submit = screen.getByRole("button", { name: "Create" }); fireEvent.click(submit); - expect(input).toBeDisabled(); + expect(input).toHaveAttribute("readonly"); + expect(input).toBeEnabled(); expect(screen.getByRole("button", { name: "Cancel" })).toBeDisabled(); const form = input.closest("form"); if (!form) throw new Error("Create form missing"); @@ -104,7 +117,8 @@ it("keeps the submitted name and identity through a failed save, blocking dismis // not pair the retained ID with a different name. await user.type(input, "es"); expect(input).toHaveValue(" Launch "); - expect(input).toBeDisabled(); + expect(input).toHaveAttribute("readonly"); + expect(input).toBeEnabled(); expect(close).not.toHaveBeenCalled(); fireEvent.click(submit); await waitFor(() => diff --git a/src/bundled/channels/CreateSidebarSection.tsx b/src/bundled/channels/CreateSidebarSection.tsx index d58827d90c..2ed13cdd95 100644 --- a/src/bundled/channels/CreateSidebarSection.tsx +++ b/src/bundled/channels/CreateSidebarSection.tsx @@ -35,14 +35,19 @@ export function CreateSidebarSection({ submitted.current = true; // A failed confirmation may follow a successful write. Retry the same // identity and name rather than trying to rename an unknown publication. - section.current ??= { id: crypto.randomUUID(), name: name.trim() }; + const attempt = section.current ?? { + id: crypto.randomUUID(), + name: name.trim(), + }; setPending(true); setError(""); try { - const accepted = await create(section.current); + const accepted = await create(attempt); + section.current = accepted ? attempt : undefined; submitted.current = accepted; setBlocked(!accepted); } catch (cause) { + section.current = attempt; submitted.current = false; setError(cause instanceof Error ? cause.message : String(cause)); } finally { @@ -92,7 +97,7 @@ export function CreateSidebarSection({ aria-label="Section name" placeholder="Section name" required - disabled={pending || !!section.current} + readOnly={pending || !!section.current} maxLength={256} value={name} onChange={(event) => setName(event.target.value)} diff --git a/tests/browser/sidebar-section-creation.spec.mjs b/tests/browser/sidebar-section-creation.spec.mjs index 786cd7868c..34380b4d9d 100644 --- a/tests/browser/sidebar-section-creation.spec.mjs +++ b/tests/browser/sidebar-section-creation.spec.mjs @@ -92,10 +92,19 @@ test("Channels offers empty-section creation with keyboard focus and confirmed p ); expect(writerReads).toBe(1); await expect.poll(() => app.report.sidebarPublications?.length ?? 0).toBe(1); - await expect(field).toBeDisabled(); + await expect(field).toBeFocused(); + await expect(field).toHaveAttribute("readonly", ""); await page.keyboard.type("es"); await expect(field).toHaveValue("Launch"); - await dialog.getByRole("button", { name: "Create", exact: true }).click(); + await page.keyboard.press("Tab"); + await expect( + dialog.getByRole("button", { name: "Cancel", exact: true }), + ).toBeFocused(); + await page.keyboard.press("Tab"); + await expect( + dialog.getByRole("button", { name: "Create", exact: true }), + ).toBeFocused(); + await page.keyboard.press("Enter"); await expect(dialog).toHaveCount(0); expect(creations).toHaveLength(2); expect(creations[1]).toEqual(creations[0]); From 8ed70f0c04a60cc7bdaa799373018020c7147922 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Wed, 28 Oct 2026 10:14:40 -0700 Subject: [PATCH 18/21] fix(sidebar): offer preference recovery after uncertain creation Co-authored-by: Carl Signed-off-by: Taylor Ho --- docs/channels.md | 6 +- .../channels/CreateSidebarSection.test.tsx | 93 +++++++++++++++++++ src/bundled/channels/CreateSidebarSection.tsx | 43 +++++---- .../browser/sidebar-section-creation.spec.mjs | 29 +++++- 4 files changed, 152 insertions(+), 19 deletions(-) diff --git a/docs/channels.md b/docs/channels.md index 8467f0dc17..c479e3b36f 100644 --- a/docs/channels.md +++ b/docs/channels.md @@ -153,7 +153,11 @@ can also be dragged with a pointer onto another saved group, **Starred** or placement, rollback notice and Retry. The **Channels** ellipsis menu also offers **Create section**, which creates an empty section without moving a channel. Its dialog retains the name and section -ID on failure and waits for confirmation before closing. After a failed creation, +ID after an attempted write fails and waits for confirmation before closing. +The submitted name stays read-only and focused; a rejected gate that attempted +no write leaves the draft editable. Failed confirmation uses plain copy and +offers Retry preferences whenever preferences are unavailable, with Create +disabled until recovery. After a failed creation, the session refreshes preferences from the writer before allowing dismissal so an accepted save stays visible even after Cancel. If that read fails too, new creation is gated across menu, drop and create-and-move paths, and Retry retains diff --git a/src/bundled/channels/CreateSidebarSection.test.tsx b/src/bundled/channels/CreateSidebarSection.test.tsx index a7e07ca2c8..da48784bea 100644 --- a/src/bundled/channels/CreateSidebarSection.test.tsx +++ b/src/bundled/channels/CreateSidebarSection.test.tsx @@ -13,6 +13,39 @@ import { CreateSidebarSection } from "./CreateSidebarSection"; afterEach(cleanup); +it("offers recovery when preferences become unavailable before the first submission", async () => { + const create = vi.fn(); + const view = render( + , + ); + const input = screen.getByRole("textbox", { name: "Section name" }); + await waitFor(() => expect(input).toHaveFocus()); + fireEvent.change(input, { target: { value: "Launch" } }); + view.rerender( + , + ); + expect( + screen.getByRole("button", { name: "Retry preferences" }), + ).toBeEnabled(); + expect(screen.getByRole("button", { name: "Create" })).toBeDisabled(); + const form = input.closest("form"); + if (!form) throw new Error("Create form missing"); + fireEvent.submit(form); + expect(create).not.toHaveBeenCalled(); +}); + it("keeps a gated draft editable and submits its edited name with a fresh identity", async () => { const user = userEvent.setup(); const create = vi.fn().mockReturnValue(false); @@ -127,3 +160,63 @@ it("keeps the submitted name and identity through a failed save, blocking dismis expect(create).toHaveBeenCalledTimes(2); expect(create.mock.calls[1]).toEqual(create.mock.calls[0]); }); + +it("offers preference recovery immediately after an uncertain save and blocks creation until recovered", async () => { + const user = userEvent.setup(); + let finish!: () => void; + const recovered = new Promise((resolve) => { + finish = resolve; + }); + const retry = vi.fn(() => recovered); + const create = vi + .fn() + .mockRejectedValueOnce(new Error("Relay request failed (502)")) + .mockResolvedValue(true); + const props = { + create, + close: vi.fn(), + writable: true, + refreshing: false, + retry, + }; + const view = render(); + const input = screen.getByRole("textbox", { name: "Section name" }); + await waitFor(() => expect(input).toHaveFocus()); + await user.type(input, "Launch{Enter}"); + await screen.findByRole("alert"); + try { + view.rerender(); + expect(screen.getByRole("alert")).toHaveTextContent( + "Couldn't confirm the section was saved", + ); + expect(screen.queryByText(/Relay request failed/)).toBeNull(); + expect(input).toHaveAccessibleDescription( + expect.stringContaining("Couldn't confirm"), + ); + const submit = screen.getByRole("button", { name: "Create" }); + expect(submit).toBeDisabled(); + const form = input.closest("form"); + if (!form) throw new Error("Create form missing"); + fireEvent.submit(form); + expect(create).toHaveBeenCalledTimes(1); + await user.click(screen.getByRole("button", { name: "Retry preferences" })); + expect(retry).toHaveBeenCalledOnce(); + view.rerender( + , + ); + expect( + screen.getByRole("button", { name: "Retry preferences" }), + ).toBeDisabled(); + view.rerender(); + finish(); + await waitFor(() => expect(input).toHaveFocus()); + expect( + screen.queryByRole("button", { name: "Retry preferences" }), + ).toBeNull(); + await user.keyboard("{Enter}"); + await waitFor(() => expect(create).toHaveBeenCalledTimes(2)); + expect(create.mock.calls[1]).toEqual(create.mock.calls[0]); + } finally { + finish(); + } +}); diff --git a/src/bundled/channels/CreateSidebarSection.tsx b/src/bundled/channels/CreateSidebarSection.tsx index 2ed13cdd95..642b5548f9 100644 --- a/src/bundled/channels/CreateSidebarSection.tsx +++ b/src/bundled/channels/CreateSidebarSection.tsx @@ -27,11 +27,12 @@ export function CreateSidebarSection({ const [name, setName] = useState(""); const submitted = useRef(false); const section = useRef<{ id: string; name: string } | undefined>(undefined); - const [blocked, setBlocked] = useState(false); const [pending, setPending] = useState(false); const [error, setError] = useState(""); + const needsRecovery = !pending && !writable; + const errorId = `${formId}-error`; const submit = async () => { - if (submitted.current || pending || !name.trim()) return; + if (submitted.current || pending || !writable || !name.trim()) return; submitted.current = true; // A failed confirmation may follow a successful write. Retry the same // identity and name rather than trying to rename an unknown publication. @@ -45,11 +46,10 @@ export function CreateSidebarSection({ const accepted = await create(attempt); section.current = accepted ? attempt : undefined; submitted.current = accepted; - setBlocked(!accepted); - } catch (cause) { + } catch { section.current = attempt; submitted.current = false; - setError(cause instanceof Error ? cause.message : String(cause)); + setError("Couldn't confirm the section was saved. Try again."); } finally { setPending(false); } @@ -77,7 +77,7 @@ export function CreateSidebarSection({ form={formId} type="submit" variant="prominent" - disabled={!name.trim()} + disabled={!writable || !name.trim()} loading={pending} > {channelName ? "Create and move" : "Create"} @@ -95,6 +95,7 @@ export function CreateSidebarSection({ setName(event.target.value)} /> - {error &&

{error}

} - {blocked && !writable && ( -
-

- Saved preferences are unavailable. Retry preferences to keep - creating this section. -

- + {(error || needsRecovery) && ( + )}
diff --git a/tests/browser/sidebar-section-creation.spec.mjs b/tests/browser/sidebar-section-creation.spec.mjs index 34380b4d9d..3984b24f7e 100644 --- a/tests/browser/sidebar-section-creation.spec.mjs +++ b/tests/browser/sidebar-section-creation.spec.mjs @@ -43,6 +43,7 @@ test("Channels offers empty-section creation with keyboard focus and confirmed p const creations = []; let lostResponse = false; let awaitingReconciliation = false; + let failWriterRead = false; let writerReads = 0; await page.route("**/query", async (route) => { const filters = route.request().postDataJSON(); @@ -54,6 +55,17 @@ test("Channels offers empty-section creation with keyboard focus and confirmed p ) { if (filters.every((filter) => filter.consistency === "strong")) { writerReads++; + if (failWriterRead) { + failWriterRead = false; + app.report.sidebarPreferenceFailures ??= []; + app.report.sidebarPreferenceFailures.push(route.request().url()); + await route.fulfill({ + status: 502, + contentType: "application/json", + body: JSON.stringify({ error: "Preference read unavailable" }), + }); + return; + } awaitingReconciliation = false; } else { // A replica is still behind the accepted write. Only writer routing @@ -88,7 +100,7 @@ test("Channels offers empty-section creation with keyboard focus and confirmed p await page.route("**/sidebar-assignment", loseCreationResponse); await field.press("Enter"); await expect(dialog.getByRole("alert")).toContainText( - "Relay request failed (502)", + "Couldn't confirm the section was saved. Try again.", ); expect(writerReads).toBe(1); await expect.poll(() => app.report.sidebarPublications?.length ?? 0).toBe(1); @@ -130,6 +142,7 @@ test("Channels offers empty-section creation with keyboard focus and confirmed p // An accepted creation must also appear if the user cancels instead of // retrying. Reopening the form must not hide that confirmed section. lostResponse = false; + failWriterRead = true; await page.route("**/sidebar-assignment", loseCreationResponse); await trigger.click(); await page @@ -138,9 +151,21 @@ test("Channels offers empty-section creation with keyboard focus and confirmed p await field.fill("Cancel check"); await dialog.getByRole("button", { name: "Create", exact: true }).click(); await expect(dialog.getByRole("alert")).toContainText( - "Relay request failed (502)", + "Couldn't confirm the section was saved. Try again.", ); expect(writerReads).toBe(2); + await expect( + dialog.getByRole("button", { name: "Create", exact: true }), + ).toBeDisabled(); + await expect(dialog.getByRole("alert")).not.toContainText("502"); + // Recover preferences without replaying creation, then Cancel/reopen. + await dialog.getByRole("button", { name: "Retry preferences" }).click(); + await expect(field).toBeFocused(); + await expect( + dialog.getByRole("button", { name: "Create", exact: true }), + ).toBeEnabled(); + expect(writerReads).toBe(3); + expect(creations).toHaveLength(3); const accepted = page .locator("summary") .filter({ hasText: /^Cancel check$/ }); From 7d6f2a59fd7b87d7ba1b170bce727a90f1afd16f Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Wed, 28 Oct 2026 11:10:46 -0700 Subject: [PATCH 19/21] test(sidebar): assert creation stays disabled during preference recovery Co-authored-by: Carl Signed-off-by: Taylor Ho --- tests/browser/navigation-groups.spec.mjs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/browser/navigation-groups.spec.mjs b/tests/browser/navigation-groups.spec.mjs index b5ecf1d7f0..a9b8875e70 100644 --- a/tests/browser/navigation-groups.spec.mjs +++ b/tests/browser/navigation-groups.spec.mjs @@ -910,7 +910,9 @@ test("Create new retains its draft when preferences fail before submission and r await expect( page.locator('[role="status"][aria-label="Reconnecting to relay…"]'), ).toBeVisible(); - await dialog.getByRole("button", { name: "Create and move" }).click(); + await expect( + dialog.getByRole("button", { name: "Create and move" }), + ).toBeDisabled(); await expect(dialog).toBeVisible(); await expect(field).toHaveValue("Retained launch"); await expect(dialog.getByRole("alert")).toContainText("Retry preferences"); @@ -936,6 +938,10 @@ test("Create new retains its draft when preferences fail before submission and r .click(); await expect(dialog.getByRole("alert")).toHaveCount(0); await expect(field).toHaveValue("Retained launch"); + await expect(field).toBeFocused(); + await expect( + dialog.getByRole("button", { name: "Create and move" }), + ).toBeEnabled(); await dialog.getByRole("button", { name: "Create and move" }).click(); await expect(dialog).toHaveCount(0); const destination = sidebar(page) From d70db7bade1f4fd27ccb17c7f2b95b9754e953eb Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Wed, 28 Oct 2026 11:10:49 -0700 Subject: [PATCH 20/21] fix(sidebar): restore recovery focus after the render commits Co-authored-by: Carl Signed-off-by: Taylor Ho --- .../channels/CreateSidebarSection.test.tsx | 37 +++++++++++++++++++ src/bundled/channels/CreateSidebarSection.tsx | 16 +++++++- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/bundled/channels/CreateSidebarSection.test.tsx b/src/bundled/channels/CreateSidebarSection.test.tsx index da48784bea..d1c85c6780 100644 --- a/src/bundled/channels/CreateSidebarSection.test.tsx +++ b/src/bundled/channels/CreateSidebarSection.test.tsx @@ -10,9 +10,46 @@ import { import { afterEach, expect, it, vi } from "vitest"; import userEvent from "@testing-library/user-event"; import { CreateSidebarSection } from "./CreateSidebarSection"; +import { useState } from "react"; afterEach(cleanup); +it("restores focus after the recovered preference render removes the Retry button", async () => { + let finish!: () => void; + const gate = new Promise((resolve) => { + finish = resolve; + }); + function Harness() { + const [writable, setWritable] = useState(false); + return ( + false} + close={vi.fn()} + writable={writable} + refreshing={false} + retry={async () => { + await gate; + setWritable(true); + }} + /> + ); + } + render(); + const input = screen.getByRole("textbox", { name: "Section name" }); + await waitFor(() => expect(input).toHaveFocus()); + const retry = screen.getByRole("button", { name: "Retry preferences" }); + const focusedBeforeRemoval: boolean[] = []; + input.addEventListener("focus", () => + focusedBeforeRemoval.push(retry.isConnected), + ); + retry.focus(); + fireEvent.click(retry); + finish(); + await waitFor(() => expect(retry).not.toBeInTheDocument()); + await waitFor(() => expect(input).toHaveFocus()); + expect(focusedBeforeRemoval).toEqual([false]); +}); + it("offers recovery when preferences become unavailable before the first submission", async () => { const create = vi.fn(); const view = render( diff --git a/src/bundled/channels/CreateSidebarSection.tsx b/src/bundled/channels/CreateSidebarSection.tsx index 642b5548f9..dc7321fe6e 100644 --- a/src/bundled/channels/CreateSidebarSection.tsx +++ b/src/bundled/channels/CreateSidebarSection.tsx @@ -1,4 +1,10 @@ -import { useId, useRef, useState, type ComponentProps } from "react"; +import { + useId, + useLayoutEffect, + useRef, + useState, + type ComponentProps, +} from "react"; import { Button } from "../../shared/design-system/ui/Button"; import { Dialog } from "../../shared/design-system/ui/Dialog"; import { Input } from "../../shared/design-system/ui/Input"; @@ -29,6 +35,12 @@ export function CreateSidebarSection({ const section = useRef<{ id: string; name: string } | undefined>(undefined); const [pending, setPending] = useState(false); const [error, setError] = useState(""); + const [focusRequest, setFocusRequest] = useState(0); + useLayoutEffect(() => { + // The preference update may remove the focused Retry button. Hand focus + // back only after React commits that removal, including on fast reads. + if (focusRequest) input.current?.focus(); + }, [focusRequest]); const needsRecovery = !pending && !writable; const errorId = `${formId}-error`; const submit = async () => { @@ -117,7 +129,7 @@ export function CreateSidebarSection({ disabled={refreshing} onClick={async () => { await retry(); - input.current?.focus(); + setFocusRequest((request) => request + 1); }} > Retry preferences From cae8747b6065d44558f7a4ef27e596dbd2a1ec33 Mon Sep 17 00:00:00 2001 From: Taylor Ho Date: Wed, 28 Oct 2026 11:33:09 -0700 Subject: [PATCH 21/21] test(sidebar): close forwarded mutation sockets between saves Co-authored-by: Carl Signed-off-by: Taylor Ho --- tests/browser/sidebar-section-creation.spec.mjs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/browser/sidebar-section-creation.spec.mjs b/tests/browser/sidebar-section-creation.spec.mjs index 3984b24f7e..0bccc76d4b 100644 --- a/tests/browser/sidebar-section-creation.spec.mjs +++ b/tests/browser/sidebar-section-creation.spec.mjs @@ -82,7 +82,12 @@ test("Channels offers empty-section creation with keyboard focus and confirmed p }); const loseCreationResponse = async (route) => { creations.push(route.request().postDataJSON()); - const response = await route.fetch(); + // API forwarding pools sockets across the reload. Close each response so + // the next mutation cannot reuse one as the fixture broker expires it. + const response = await route.fetch({ + headers: { ...(await route.request().allHeaders()), connection: "close" }, + }); + expect(response.headers()["connection"]).toBe("close"); if (!lostResponse && response.ok()) { // The real broker accepts and persists the write, but its response never // reaches the dialog. The retry must reconcile that exact creation.