From 405881ef4e9aad18402a1b397e426ab9d1af52bb Mon Sep 17 00:00:00 2001 From: John Tennant Date: Thu, 22 Oct 2026 13:55:19 -0400 Subject: [PATCH 1/2] feat(todos): filter, edit and delete todos Each todo row gains a More menu with Edit and Delete. Edit swaps the label for an inline field: Enter or leaving the field saves, and Escape cancels. Only the label's source changes, so the checkbox, assignee and subtasks stay as they were. Delete removes the todo's lines with its subtasks, and asks first when it has subtasks. Status and assignee filters sit above the list. A parent stays visible when any of its subtasks match, so a match keeps its context. Co-authored-by: Martha <73923046810b68ae182000e74ef7c444383a907df20f81f2ba1008870a6f45a1@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: John Tennant --- src/bundled/todos/Todos.module.css | 9 + src/bundled/todos/TodosPanel.test.tsx | 96 ++++++++ src/bundled/todos/TodosPanel.tsx | 308 ++++++++++++++++++++++---- src/bundled/todos/model.test.ts | 55 ++++- src/bundled/todos/model.ts | 66 +++++- 5 files changed, 486 insertions(+), 48 deletions(-) diff --git a/src/bundled/todos/Todos.module.css b/src/bundled/todos/Todos.module.css index d090a76a86..b5a2ca0640 100644 --- a/src/bundled/todos/Todos.module.css +++ b/src/bundled/todos/Todos.module.css @@ -129,3 +129,12 @@ text-overflow: ellipsis; white-space: nowrap; } + +.filters { + display: flex; + flex-wrap: wrap; + gap: var(--space-2); +} +.edit { + min-width: 0; +} diff --git a/src/bundled/todos/TodosPanel.test.tsx b/src/bundled/todos/TodosPanel.test.tsx index 1d6b1eeab7..11890f3953 100644 --- a/src/bundled/todos/TodosPanel.test.tsx +++ b/src/bundled/todos/TodosPanel.test.tsx @@ -1084,3 +1084,99 @@ it("clears collapsed positions when refreshing a changed Canvas", async () => { screen.getByRole("button", { name: "Collapse Other!" }), ).toBeInTheDocument(); }); + +it("edits a task's label from its menu, keeping its marker and assignee", async () => { + const f = fixture(), + user = userEvent.setup(); + render(); + await screen.findByRole("checkbox", { name: "First" }); + await user.click(screen.getByRole("button", { name: "More for First" })); + await user.click(await screen.findByRole("menuitem", { name: "Edit" })); + const input = screen.getByRole("textbox", { name: "Edit First" }); + expect(input).toHaveFocus(); + await user.clear(input); + await user.type(input, "First *draft*{Escape}"); + expect(f.canvas.save).not.toHaveBeenCalled(); + expect(screen.getByRole("checkbox", { name: "First" })).toHaveFocus(); + await user.click(screen.getByRole("button", { name: "More for First" })); + await user.click(await screen.findByRole("menuitem", { name: "Edit" })); + const again = screen.getByRole("textbox", { name: "Edit First" }); + await user.clear(again); + await user.type(again, "First *draft*{Enter}"); + await saved(); + expect(f.canvas.save).toHaveBeenCalledTimes(1); + expect(f.canvas.save).toHaveBeenLastCalledWith( + context.channelId, + original.replace("- [ ] First", "- [ ] First *draft*"), + head.id, + ); + expect(screen.getByRole("checkbox", { name: "First draft" })).toHaveFocus(); +}); +it("deletes a task at once, and confirms before deleting one with subtasks", async () => { + const f = fixture(), + user = userEvent.setup(); + const content = "## Todos\n\n- [ ] Ship\n - [ ] Build\n- [ ] Next\n"; + f.canvas.read.mockResolvedValue({ ...head, content }); + render(); + const remove = async (name: string) => { + await user.click(screen.getByRole("button", { name: `More for ${name}` })); + await user.click(await screen.findByRole("menuitem", { name: "Delete" })); + }; + await screen.findByRole("checkbox", { name: "Next" }); + await remove("Next"); + await saved(); + expect(f.canvas.save).toHaveBeenLastCalledWith( + context.channelId, + "## Todos\n\n- [ ] Ship\n - [ ] Build\n", + head.id, + ); + await remove("Ship"); + const dialog = await screen.findByRole("alertdialog", { + name: "Delete “Ship”?", + }); + expect(dialog).toHaveTextContent("Its 1 subtask is deleted too."); + await user.click(within(dialog).getByRole("button", { name: "Cancel" })); + expect(f.canvas.save).toHaveBeenCalledTimes(1); + await remove("Ship"); + await user.click( + within( + await screen.findByRole("alertdialog", { name: "Delete “Ship”?" }), + ).getByRole("button", { name: "Delete" }), + ); + await saved(); + expect(f.canvas.save).toHaveBeenLastCalledWith( + context.channelId, + "## Todos\n\n", + "b".repeat(64), + ); + expect(await screen.findByText("No todos yet")).toBeInTheDocument(); +}); +it("filters by status and assignee, keeping matching subtasks' parents", async () => { + const f = fixture(), + user = userEvent.setup(); + const content = + "## Todos\n\n- [/] Ship · Assignee: Sam\n - [x] Build · Assignee: Alex\n - [ ] Publish\n- [ ] Plan\n- [x] Done\n"; + f.canvas.read.mockResolvedValue({ ...head, content }); + render(); + await screen.findByRole("checkbox", { name: "Plan" }); + const shown = () => + screen + .getAllByRole("checkbox") + .map((box) => box.getAttribute("aria-label")); + const choose = async (filter: string, name: string) => { + await user.click(screen.getByRole("combobox", { name: filter })); + await user.click(await screen.findByRole("option", { name })); + }; + await choose("Filter by status", "Done"); + expect(shown()).toEqual(["Ship", "Build", "Done"]); + await choose("Filter by assignee", "Alex"); + expect(shown()).toEqual(["Ship", "Build"]); + await choose("Filter by status", "To do"); + expect(screen.getByText("No todos match these filters.")).toBeInTheDocument(); + await choose("Filter by assignee", "Unassigned"); + expect(shown()).toEqual(["Plan", "Ship", "Publish"]); + await choose("Filter by status", "All statuses"); + await choose("Filter by assignee", "Anyone"); + expect(shown()).toEqual(["Plan", "Ship", "Build", "Publish", "Done"]); + expect(f.canvas.save).not.toHaveBeenCalled(); +}); diff --git a/src/bundled/todos/TodosPanel.tsx b/src/bundled/todos/TodosPanel.tsx index 76363e118f..1bce369222 100644 --- a/src/bundled/todos/TodosPanel.tsx +++ b/src/bundled/todos/TodosPanel.tsx @@ -33,7 +33,18 @@ import { CircleIcon, CircleHalfIcon, CheckCircleIcon, + DotsThreeIcon, + PencilSimpleIcon, + TrashIcon, } from "../../shared/design-system/icons/index"; +import { + MenuRoot, + MenuTrigger, + MenuPopup, + MenuItem, + MenuIcon, +} from "../../shared/design-system/ui/Menu"; +import { Select } from "../../shared/design-system/ui/Select"; import { AlertDialog } from "../../shared/design-system/ui/AlertDialog"; import { Button } from "../../shared/design-system/ui/Button"; import { Checkbox } from "../../shared/design-system/ui/Checkbox"; @@ -58,6 +69,8 @@ import { todoLabelText, addTodo, assignTodo, + deleteTodo, + editTodo, readTodos, setStatus, type Status, @@ -157,6 +170,20 @@ export function TodosPanel({ const [error, setError] = useState(""); const [conflict, setConflict] = useState(false); const [confirmRefresh, setConfirmRefresh] = useState(false); + const [editing, setEditingState] = useState<{ + offset: number; + text: string; + }>(); + // Escape and blur can both end an edit; the ref lets only the first one count. + const editingRef = useRef(editing); + const setEditing = (next: typeof editing) => { + editingRef.current = next; + setEditingState(next); + }; + const [deleting, setDeleting] = useState(); + // An assignee filter is a pubkey, "none", or "name:" plus a Canvas name. + const [statusFilter, setStatusFilter] = useState("all"); + const [assigneeFilter, setAssigneeFilter] = useState("all"); const refreshRef = useRef(null); const operation = useRef(0); const saving = useRef(false); @@ -424,9 +451,70 @@ export function TodosPanel({ childrenByOffset.set(parent, children); ancestors.push(item); } + const assigneeKey = (item: (typeof items)[number]) => + item.assignee + ? item.assignee.pubkey + : item.namedAssignee + ? `name:${item.namedAssignee.name}` + : "none"; + const assigneeFilters = [ + ...new Map( + items.map((item) => [ + assigneeKey(item), + item.assignee + ? resolveName(item.assignee.pubkey, item.assignee.name) || + keyLabels.get(item.assignee.pubkey) || + item.assignee.pubkey + : (item.namedAssignee?.name ?? "Unassigned"), + ]), + ), + ].sort(([a, x], [b, y]) => + a === "none" ? -1 : b === "none" ? 1 : x.localeCompare(y), + ); + // A filter for an assignee no longer in the list falls back to everyone. + const assignee = assigneeFilters.some(([key]) => key === assigneeFilter) + ? assigneeFilter + : "all"; + const filtering = statusFilter !== "all" || assignee !== "all"; + // A parent stays visible, for context, when any of its subtasks match. + const visible = new Set(); + for (const item of [...items].reverse()) + if ( + ((statusFilter === "all" || item.status === statusFilter) && + (assignee === "all" || assigneeKey(item) === assignee)) || + (childrenByOffset.get(item.offset) ?? []).some((child) => + visible.has(child.offset), + ) + ) + visible.add(item.offset); + const finishEdit = (save: boolean) => { + if (!editingRef.current) return; + const { offset, text } = editingRef.current; + setEditing(undefined); + focusAfterToggle.current = `${prefix}-${offset}`; + const item = items.find((item) => item.offset === offset); + if (save && item && text.trim() !== item.label) + change(() => editTodo(draft.content, offset, text)); + }; + const remove = (offset: number) => { + setDeleting(undefined); + focusAfterToggle.current = `${prefix}-new`; + change(() => deleteTodo(draft.content, offset)); + }; + const deletingItem = items.find((item) => item.offset === deleting); + const deletingSubtasks = deletingItem + ? items.filter( + (item) => + item.offset !== deletingItem.offset && + item.offset > deletingItem.span.start && + item.offset < deletingItem.span.end, + ).length + : 0; const renderItems = (group: typeof items): ReactNode => group.map((item) => { - const children = childrenByOffset.get(item.offset) ?? []; + const children = (childrenByOffset.get(item.offset) ?? []).filter( + (child) => visible.has(child.offset), + ); const taskLabel = todoLabelText(item.label); const assigneeName = item.assignee ? resolveName(item.assignee.pubkey, item.assignee.name) || @@ -479,48 +567,73 @@ export function TodosPanel({ aria-label={taskLabel} label="" /> -
- - parseBuzzLink(value) || profileKey(value) - ? value - : safeLinkUrl(value) - } - allowedElements={[ - "p", - "a", - "strong", - "em", - "del", - "code", - "br", - "img", - ]} - unwrapDisallowed - components={{ - p: ({ children }) => {children}, - a: ({ href, children }) => - href ? ( - active() && onOpenLink(url)} - > - {children} - - ) : ( - {children} - ), - img: ({ alt }) => {alt}, + {editing?.offset === item.offset ? ( +
{ + event.preventDefault(); + finishEdit(true); }} > - {item.label} - -
+ + setEditing({ ...editing, text: event.target.value }) + } + onKeyDown={(event) => { + if (event.key !== "Escape") return; + event.preventDefault(); + finishEdit(false); + }} + onBlur={() => finishEdit(true)} + /> + + ) : ( +
+ + parseBuzzLink(value) || profileKey(value) + ? value + : safeLinkUrl(value) + } + allowedElements={[ + "p", + "a", + "strong", + "em", + "del", + "code", + "br", + "img", + ]} + unwrapDisallowed + components={{ + p: ({ children }) => {children}, + a: ({ href, children }) => + href ? ( + active() && onOpenLink(url)} + > + {children} + + ) : ( + {children} + ), + img: ({ alt }) => {alt}, + }} + > + {item.label} + +
+ )}
+ +
{children.length > 0 && ( @@ -853,9 +1006,62 @@ export function TodosPanel({ description="Add a todo above to track work with this channel. Changes save automatically." /> )} + {items.length > 0 && ( +
+ ({ + value, + label, + })), + ], + }, + ]} + /> +
+ )} + {filtering && + items.length > 0 && + !(childrenByOffset.get(undefined) ?? []).some((item) => + visible.has(item.offset), + ) && ( +

+ No todos match these filters. +

+ )} {statuses.map(([status, title]) => { const group = (childrenByOffset.get(undefined) ?? []).filter( - (item) => item.status === status, + (item) => + item.status === status && visible.has(item.offset), ); return ( group.length > 0 && ( @@ -880,6 +1086,24 @@ export function TodosPanel({ )} + {deletingItem && ( + setDeleting(undefined)} + actions={ + <> + + + + } + /> + )} {confirmRefresh && ( { it("changes only the selected checkbox byte, including duplicate labels and CRLF", () => { @@ -216,3 +223,49 @@ it("does not clear a named assignment lookalike inside multiline code", () => { expect(assignTodo(before, item.offset)).toBe(before); expect(() => assignTodo(before, item.offset, sam)).toThrow(/safely/); }); + +it("edits only a label, keeping status, assignee, subtasks and CRLF", () => { + const target = `[Alex](${profileTarget(alex.pubkey)})`; + const before = `# Notes\r\n\r\n## Todos\r\n\r\n- [/] Ship **it** · Assignee: ${target}\r\n - [ ] Test\r\n- [ ] Ship **it**\r\n`; + const item = first(before); + const edited = editTodo(before, item.offset, " Ship *v2* "); + expect(edited).toBe(before.replace("[/] Ship **it**", "[/] Ship *v2*")); + expect(readTodos(edited).items.map((i) => [i.label, i.status])).toEqual([ + ["Ship *v2*", "doing"], + ["Test", "todo"], + ["Ship **it**", "todo"], + ]); + const named = "## Todos\n- [ ] Plan · Assignee: John\n"; + expect(editTodo(named, first(named).offset, "Plan more")).toBe( + "## Todos\n- [ ] Plan more · Assignee: John\n", + ); +}); +it("rejects edits that are empty, multiline, stale or change the todo's structure", () => { + const before = "## Todos\n- [ ] First\n- [ ] Second\n"; + const { offset } = first(before); + expect(() => editTodo(before, offset, " ")).toThrow(/single line/); + expect(() => editTodo(before, offset, "a\nb")).toThrow(/single line/); + expect(() => editTodo(before, 0, "New")).toThrow(/changed/); + expect(() => editTodo(before, offset, "Mine · Assignee: Sam")).toThrow( + /safely/, + ); + expect(() => editTodo(before, offset, "`open")).not.toThrow(); +}); +it("deletes a todo with its subtasks and leaves every other line", () => { + const before = + "## Todos\r\n\r\n- [ ] Ship\r\n - [ ] Build\r\n - [x] Test\r\n - [ ] Publish\r\n- [ ] Next\r\n\r\n## Keep\r\n- [ ] Not a todo\r\n"; + const [ship, build, , publish] = readTodos(before).items; + if (!ship || !build || !publish) throw new Error("Expected fixture todos"); + expect(deleteTodo(before, build.offset)).toBe( + before.replace(" - [ ] Build\r\n - [x] Test\r\n", ""), + ); + expect(deleteTodo(before, publish.offset)).toBe( + before.replace(" - [ ] Publish\r\n", ""), + ); + expect(deleteTodo(before, ship.offset)).toBe( + "## Todos\r\n\r\n- [ ] Next\r\n\r\n## Keep\r\n- [ ] Not a todo\r\n", + ); + const last = "## Todos\n- [ ] Only"; + expect(deleteTodo(last, first(last).offset)).toBe("## Todos\n"); + expect(() => deleteTodo(before, 0)).toThrow(/changed/); +}); diff --git a/src/bundled/todos/model.ts b/src/bundled/todos/model.ts index 201f7130ea..180c1c3095 100644 --- a/src/bundled/todos/model.ts +++ b/src/bundled/todos/model.ts @@ -14,6 +14,9 @@ export type Todo = { parents: string[]; status: Status; end: number; + /** Where the label text starts, and the list item's whole source, subtasks included. */ + labelStart: number; + span: { start: number; end: number }; namedAssignee?: { name: string; start: number }; assignee?: (Assignee & { start: number }) | undefined; }; @@ -45,7 +48,8 @@ export function readTodos(content: string) { for (const item of children) { const before = items.length; const offset = item.position?.start.offset; - if (offset === undefined) continue; + const itemEnd = item.position?.end.offset; + if (offset === undefined || itemEnd === undefined) continue; const match = /^([-+*][ \t]+\[)([ xX/])\][ \t]+([^\r\n]+)/.exec( content.slice(offset), ); @@ -99,14 +103,14 @@ export function readTodos(content: string) { named?.[1] && namedText ? { name: named[1].trim(), start: offset + named.index } : undefined; + const labelStart = offset + match[0].length - match[3].length; items.push({ offset: offset + match[1].length, label: content - .slice( - offset + match[0].length - match[3].length, - assignee?.start ?? namedAssignee?.start ?? end, - ) + .slice(labelStart, assignee?.start ?? namedAssignee?.start ?? end) .trim(), + labelStart, + span: { start: offset, end: itemEnd }, status: match[2] === " " ? "todo" : match[2] === "/" ? "doing" : "done", end, @@ -181,6 +185,58 @@ export function assignTodo( return next; } +function find(content: string, offset: number) { + const { items } = readTodos(content); + const item = items.find((item) => item.offset === offset); + if (!item) + throw new Error("This todo changed. Reload the Canvas before editing it."); + return { items, item }; +} + +/** Replace only the label, keeping the checkbox, assignment and subtasks. */ +export function editTodo(content: string, offset: number, label: string) { + const text = label.trim(); + if (!text || /[\r\n]/.test(text)) + throw new Error("Use a single line for your todo."); + const { items, item } = find(content, offset); + const end = item.assignee?.start ?? item.namedAssignee?.start ?? item.end; + const next = content.slice(0, item.labelStart) + text + content.slice(end); + const parsed = readTodos(next).items; + const edited = parsed.find((value) => value.offset === offset); + if ( + parsed.length !== items.length || + edited?.label !== text || + edited.status !== item.status || + edited.assignee?.pubkey !== item.assignee?.pubkey || + edited.namedAssignee?.name !== item.namedAssignee?.name + ) + throw new Error( + "Could not edit safely. Check this todo’s Markdown in Canvas.", + ); + return next; +} + +/** Remove the todo's lines, with its subtasks, and nothing else. */ +export function deleteTodo(content: string, offset: number) { + const { items, item } = find(content, offset); + const start = content.lastIndexOf("\n", item.span.start - 1) + 1; + if (content.slice(start, item.span.start).trim()) + throw new Error( + "Could not delete safely. Check this todo’s Markdown in Canvas.", + ); + const newline = /^\r?\n/.exec(content.slice(item.span.end))?.[0] ?? ""; + const next = + content.slice(0, start) + content.slice(item.span.end + newline.length); + const removed = items.filter( + (value) => value.offset >= item.span.start && value.offset < item.span.end, + ).length; + if (readTodos(next).items.length !== items.length - removed) + throw new Error( + "Could not delete safely. Check this todo’s Markdown in Canvas.", + ); + return next; +} + /** Match the inline title renderer without exposing Markdown syntax in control names. */ export function todoLabelText(label: string): string { const text = (node: Root | RootContent): string => From 34602917b79f2483298ca9ef1ae7f327767b77b9 Mon Sep 17 00:00:00 2001 From: John Tennant Date: Thu, 22 Oct 2026 14:08:24 -0400 Subject: [PATCH 2/2] test(todos): cover row-menu edit, delete and filters in browsers A Chromium and WebKit journey opens Edit from the row menu, which returns focus as it closes, and checks that the inline field keeps focus and saves. It then filters to Done and deletes from there. The empty-filter note is plain text, so the panel keeps one save status. Co-authored-by: Martha <73923046810b68ae182000e74ef7c444383a907df20f81f2ba1008870a6f45a1@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: John Tennant --- src/bundled/todos/TodosPanel.tsx | 2 +- tests/browser/todos.spec.mjs | 65 ++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/src/bundled/todos/TodosPanel.tsx b/src/bundled/todos/TodosPanel.tsx index 1bce369222..efa3b4a2b2 100644 --- a/src/bundled/todos/TodosPanel.tsx +++ b/src/bundled/todos/TodosPanel.tsx @@ -1054,7 +1054,7 @@ export function TodosPanel({ !(childrenByOffset.get(undefined) ?? []).some((item) => visible.has(item.offset), ) && ( -

+

No todos match these filters.

)} diff --git a/tests/browser/todos.spec.mjs b/tests/browser/todos.spec.mjs index 43e459bb70..6f7089f0a4 100644 --- a/tests/browser/todos.spec.mjs +++ b/tests/browser/todos.spec.mjs @@ -341,3 +341,68 @@ test("opt-in Todos saves ordinary Canvas and disabling leaves it editable", asyn await expect(canvas).toHaveValue(head.content); await expect(canvas).toBeEditable(); }); + +// Browser-only boundary: the row menu returns focus as it closes, which must +// not end the inline edit it just opened. +test("edits, deletes and filters todos from the row controls", async ({ + page, + app, +}) => { + const original = "## Todos\n\n- [ ] First\n- [x] Second\n"; + let head = app.sign({ + kind: 40100, + content: original, + tags: [["h", "alpha"]], + created_at: 1700000000, + }); + await page.route("**/api/relay/primary/query", async (route) => { + const filters = route.request().postDataJSON(); + if ( + filters.every((f) => f.kinds?.includes(40100) || f.ids?.includes(head.id)) + ) + return route.fulfill({ json: [head] }); + return route.continue(); + }); + await page.route("**/api/relay/primary/sign", async (route) => + route.fulfill({ json: app.sign(route.request().postDataJSON()) }), + ); + await page.route("**/api/relay/primary/publish", async (route) => { + head = route.request().postDataJSON(); + await route.fulfill({ json: { accepted: true, event_id: head.id } }); + }); + await page.goto(app.origin); + await openSettings(page); + await selectSettingsSection(page, "Manage plugins"); + await pluginAction(page, "Todos").click(); + await closeSettings(page); + await openPage(page, "Messages"); + await page + .getByRole("button", { name: "Toggle channel todos", exact: true }) + .click(); + const drawer = page.getByRole("region", { name: "Todos panel", exact: true }); + const saved = () => + expect(drawer.getByRole("status")).toHaveText("Saved in Canvas"); + const menu = async (task, action) => { + await drawer + .getByRole("button", { name: `More for ${task}`, exact: true }) + .click(); + await page.getByRole("menuitem", { name: action, exact: true }).click(); + }; + await menu("First", "Edit"); + const edit = drawer.getByRole("textbox", { name: "Edit First", exact: true }); + await expect(edit).toBeFocused(); + await edit.fill("First edited"); + await edit.press("Enter"); + await saved(); + expect(head.content).toBe(original.replace("First", "First edited")); + await expect( + drawer.getByRole("checkbox", { name: "First edited", exact: true }), + ).toBeFocused(); + await drawer.getByRole("combobox", { name: "Filter by status" }).click(); + await page.getByRole("option", { name: "Done", exact: true }).click(); + await expect(drawer.getByRole("checkbox")).toHaveCount(1); + await menu("Second", "Delete"); + await saved(); + expect(head.content).toBe("## Todos\n\n- [ ] First edited\n"); + await expect(drawer.getByText("No todos match these filters.")).toBeVisible(); +});