diff --git a/browser-host/sidebar-create-section.test.mjs b/browser-host/sidebar-create-section.test.mjs index a52b438474..ad30f1edaa 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"); }); @@ -101,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( @@ -187,6 +198,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( [ @@ -257,3 +294,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-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/browser-host/sidebar-preferences.mjs b/browser-host/sidebar-preferences.mjs index 19434f5acd..87a270585e 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 && @@ -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 4b0831b6f1..c479e3b36f 100644 --- a/docs/channels.md +++ b/docs/channels.md @@ -151,6 +151,28 @@ 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 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 +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 +**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 +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. @@ -268,8 +290,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/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(), diff --git a/src/bundled/channels/Channels.module.css b/src/bundled/channels/Channels.module.css index 41b72541c5..61669d676e 100644 --- a/src/bundled/channels/Channels.module.css +++ b/src/bundled/channels/Channels.module.css @@ -296,6 +296,48 @@ 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 { + display: flex; + flex-direction: column; + align-items: center; + justify-content: center; + 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); + padding: var(--space-2); + 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(--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; @@ -334,11 +376,19 @@ 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; 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.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 3273892e97..d1c85c6780 100644 --- a/src/bundled/channels/CreateSidebarSection.test.tsx +++ b/src/bundled/channels/CreateSidebarSection.test.tsx @@ -1,49 +1,259 @@ // @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 userEvent from "@testing-library/user-event"; import { CreateSidebarSection } from "./CreateSidebarSection"; +import { useState } from "react"; afterEach(cleanup); -it("retains a rejected draft and its identity through preference recovery", () => { +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( + , + ); + 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); const retry = vi.fn().mockResolvedValue(undefined); const props = { channelName: "Beta", create, close: vi.fn(), - writable: false, + writable: true, refreshing: false, retry, }; 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"); 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(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(); - submit(); + await submit(); expect(create).toHaveBeenCalledTimes(2); - expect(create.mock.calls[1]).toEqual(create.mock.calls[0]); - submit(); + 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); }); + +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() + .mockImplementationOnce( + () => + new Promise((_, no) => { + reject = no; + }), + ) + .mockResolvedValue(true); + const close = vi.fn(); + render( + , + ); + 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); + 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"); + fireEvent.submit(form); + 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).toHaveAttribute("readonly"); + expect(input).toBeEnabled(); + expect(close).not.toHaveBeenCalled(); + fireEvent.click(submit); + await waitFor(() => + expect(screen.getByRole("button", { name: "Cancel" })).toBeEnabled(), + ); + 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 a1ff685889..dc7321fe6e 100644 --- a/src/bundled/channels/CreateSidebarSection.tsx +++ b/src/bundled/channels/CreateSidebarSection.tsx @@ -1,8 +1,13 @@ -import { useId, useRef, useState } 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"; -import styles from "./CreateSidebarSection.module.css"; /** Mounted after the launching menu closes, so focus has one modal owner. */ export function CreateSidebarSection({ @@ -12,48 +17,82 @@ 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); const [name, setName] = useState(""); const submitted = useRef(false); - const sectionId = useRef(undefined); - const [blocked, setBlocked] = useState(false); + 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 () => { + 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. + const attempt = section.current ?? { + id: crypto.randomUUID(), + name: name.trim(), + }; + setPending(true); + setError(""); + try { + const accepted = await create(attempt); + section.current = accepted ? attempt : undefined; + submitted.current = accepted; + } catch { + section.current = attempt; + submitted.current = false; + setError("Couldn't confirm the section was saved. Try again."); + } finally { + setPending(false); + } + }; return ( { if (!open) close(); }} - title="Create new section" - description={ - - Move {channelName} into a new - section. - - } + title={channelName ? "Move to new section" : "Create new section"} initialFocus={input} - finalFocus={false} + finalFocus={finalFocus} actions={ <> - + } @@ -62,32 +101,41 @@ 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)} /> - {blocked && !writable && ( -
-

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

- + {(error || needsRecovery) && ( + )}
diff --git a/src/bundled/channels/SidebarSection.tsx b/src/bundled/channels/SidebarSection.tsx index bd7bb941ce..77ce8e30f9 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"; @@ -378,6 +382,24 @@ export function SidebarSection({ )} + {createSection && ( + <> + {(filter || sort) && } + { + setMenuOpen(false); + if (actionsTrigger.current) + createSection.open(actionsTrigger.current); + }} + > + + + + Create section + + + )} {move && ( <> { @@ -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..d0bdcb095b 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( @@ -1181,12 +1183,69 @@ 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(); }); +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 = { @@ -1641,6 +1700,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" }), ); @@ -1671,6 +1735,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 076e20755e..3dbed1d359 100644 --- a/src/features/channel-navigation/ChannelInventory.tsx +++ b/src/features/channel-navigation/ChannelInventory.tsx @@ -264,9 +264,12 @@ 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; + position?: "first"; }>(); const createOrigin = creatingFor?.surface?.signal; @@ -491,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()); @@ -718,6 +724,7 @@ export function ChannelInventory({ { if (surface) { surface.close(() => false); @@ -725,6 +732,7 @@ export function ChannelInventory({ if ( mounted.current && !surface.signal?.aborted && + queries.sidebarPreferences.sectionCreationWritable && relay.snapshot().session === queries ) setCreatingFor({ channel, surface }); @@ -1121,6 +1129,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.sectionCreationWritable + ) + setCreatingFor({ channel, position: "first" }); + }); + }} onMove={(channelId, sectionKey) => sectionKey === "starred" ? setChannelStar(channelId, true) @@ -1171,7 +1191,10 @@ export function ChannelInventory({ if (!channel) return null; const Icon = channelIcon(channel); return ( -
+
{channel.name}
@@ -1181,6 +1204,10 @@ export function ChannelInventory({ {filteredSections.map((section) => ( { + createSectionTrigger.current = trigger; + requestAnimationFrame(() => { + if (mounted.current) setCreatingSection(true); + }); + }, + } + : undefined + } newMessage={ section.key === "dms" ? () => { @@ -1624,10 +1664,27 @@ 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 && ( { @@ -1635,13 +1692,18 @@ 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( creatingFor.channel.id, () => - 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 639381f5a4..68ff502567 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,9 +21,13 @@ import { type ReactNode, } from "react"; import { createPortal } from "react-dom"; +import { motion, useReducedMotion } from "motion/react"; import { SidebarSection } from "../../bundled/channels/SidebarSection"; +import { PlusIcon } from "../../shared/design-system/icons"; +import styles from "../../bundled/channels/Channels.module.css"; const Writable = createContext(false); +const CREATE_SECTION = "create-section"; // Group gaps target the neighbour whose insertion edge lies in that same gap. const sidebarCollisions: CollisionDetection = (args) => { @@ -79,12 +84,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,6 +119,8 @@ 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)); }} @@ -128,6 +137,63 @@ export function ChannelSidebarDnd({ ); } +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: !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({ + onDragStart: () => setRevealed(false), + onDragOver: ({ over }) => { + if (enabled && over?.id === CREATE_SECTION) setRevealed(true); + }, + onDragEnd: () => setRevealed(false), + onDragCancel: () => setRevealed(false), + }); + const open = enabled && revealed; + return ( +
+ {/* The original gap stays inside the target throughout expansion. */} +
+ +
+ + Add to new section +
+
+
+ ); +} + /** * 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 @@ -164,12 +230,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, @@ -217,39 +285,42 @@ 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); + <> + {createSectionTarget && } + { + 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/src/features/relay/native-sidebar.test.ts b/src/features/relay/native-sidebar.test.ts index 9aa46891ab..3d10d31b9b 100644 --- a/src/features/relay/native-sidebar.test.ts +++ b/src/features/relay/native-sidebar.test.ts @@ -160,6 +160,25 @@ it("decodes four self-encrypted coordinates through the narrow IPC command", asy expect(transport.writer?.kinds).toContain(30078); }); +it("prepends a drag-created section through native encrypted readback", async () => { + 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"; @@ -926,3 +945,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/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-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 aceab82991..fc7fa8309d 100644 --- a/src/features/relay/sidebar-edits.ts +++ b/src/features/relay/sidebar-edits.ts @@ -86,12 +86,16 @@ export function validSidebarAssignment( ): boolean { const { channelId, sectionId, createSection } = intent; return ( - validSidebarChannelId(channelId) && + (channelId === undefined + ? !!createSection + : validSidebarChannelId(channelId)) && (sectionId === undefined || validSidebarChannelId(sectionId)) && (!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"))) ); } @@ -169,12 +173,13 @@ 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)) 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..a63d5a47b4 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, @@ -28,15 +28,27 @@ 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 }; - 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) - ? [...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 }; @@ -66,6 +78,7 @@ async function setup() { }); const owner = createSidebarPreferencesStore({ read, + readAfterWrite, available: true, write: assignment, writeStar: star, @@ -73,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 () => { @@ -141,37 +163,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", @@ -559,3 +590,293 @@ 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(); + } +}); + +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.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"); + }); + 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", + async (retire) => { + const h = await setup(); + const read = deferred(); + const entered = deferred(); + h.assignment.mockRejectedValueOnce(new Error("response lost")); + h.readAfterWrite.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(); + } +}); + +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(); + } +}); + +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.test.ts b/src/features/relay/sidebar-preferences-store.test.ts index 8c26382cca..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" }, @@ -47,8 +74,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..774fff8e7f 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 && @@ -140,15 +147,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]; @@ -323,39 +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 read(controller.signal); - if ( - closed || - controller.signal.aborted || - active !== job || - mutation !== refreshMutation - ) - return; - readFailure = undefined; - 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; @@ -386,6 +413,7 @@ export function createSidebarPreferencesStore({ ): Promise { if ( !writable() || + (creationUncertain && "createSection" in destination) || !snapshot.data || !confirmed || !write || @@ -417,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. @@ -651,13 +681,41 @@ export function createSidebarPreferencesStore({ ); }, get sectionCreationWritable() { - return writable() && !snapshot.cached && !!createSection; + return ( + writable() && + !creationUncertain && + !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 failure = groupsOnly + ? "Workspaces are unavailable" + : "Section creation is unavailable"; + const create = + createSection ?? + (write + ? ( + section: Readonly<{ id: string; name: string }>, + signal: AbortSignal, + ) => write({ createSection: section }, signal) + : undefined); + if (!writable() || creationUncertain || snapshot.cached || !create) + return Promise.reject(new Error(failure)); + 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; + creationUncertain = true; + await refresh(); + } + throw error; + }, ); }, get sectionOrderWritable() { @@ -956,7 +1014,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.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 0eeb47b59f..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), ), ) ); @@ -93,9 +94,10 @@ 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 }>; + createSection?: Readonly<{ id: string; name: string; position?: "first" }>; }>; export type SidebarAssignmentMutator = ( intent: SidebarAssignmentIntent, @@ -113,7 +115,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, @@ -322,6 +324,7 @@ export async function readSidebarPreferences( viewer: string, decode: SidebarDecoder, signal: AbortSignal, + consistency?: "strong", ): Promise { let events: readonly RelayEvent[]; try { @@ -331,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/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/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/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-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/navigation-groups.spec.mjs b/tests/browser/navigation-groups.spec.mjs index cdb6d423e2..a9b8875e70 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. @@ -208,9 +225,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"); @@ -695,9 +716,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); @@ -874,7 +895,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( @@ -889,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"); @@ -915,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) @@ -942,102 +969,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. @@ -1061,7 +1088,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 new file mode 100644 index 0000000000..0bccc76d4b --- /dev/null +++ b/tests/browser/sidebar-section-creation.spec.mjs @@ -0,0 +1,415 @@ +import { test, expect } from "./fixture.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. +// 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 expect( + page + .getByRole("menu", { name: "More actions for Channels", exact: true }) + .getByRole("menuitem"), + ).toHaveText(["Filter", "Sort", "Create section"]); + 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 expect(page.getByRole("menu")).toHaveCount(0); + await page.keyboard.type("Launch"); + await expect(field).toHaveValue("Launch"); + 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(); + if ( + awaitingReconciliation && + Array.isArray(filters) && + filters.length === 5 && + filters.some((filter) => filter["#d"]?.includes("channel-sections")) + ) { + 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 + // 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()); + // 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. + lostResponse = true; + awaitingReconciliation = 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 page.route("**/sidebar-assignment", loseCreationResponse); + await field.press("Enter"); + await expect(dialog.getByRole("alert")).toContainText( + "Couldn't confirm the section was saved. Try again.", + ); + expect(writerReads).toBe(1); + await expect.poll(() => app.report.sidebarPublications?.length ?? 0).toBe(1); + await expect(field).toBeFocused(); + await expect(field).toHaveAttribute("readonly", ""); + await page.keyboard.type("es"); + await expect(field).toHaveValue("Launch"); + 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]); + await page.unroute("**/sidebar-assignment"); + 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(); + // 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 + .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( + "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$/ }); + 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) { + 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); + 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) { + await revealCreate(page); + const target = page.locator("[data-create-section-drop]"); + // 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(); +} + +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 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]"); + 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"]'); + 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(); + 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); + // 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); + await expect(page.locator("[data-section-creation-preview]")).toHaveCount(0); + await expect(target).toHaveCount(0); + await page.keyboard.press("Escape"); + await page.mouse.up(); + // 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" }); + 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 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 + .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 + .getByRole("menuitem", { name: "Move channel", exact: true }) + .hover(); + await expect( + 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: "Move to 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); + }); +});