diff --git a/apps/web/components/app-sidebar/sections/tasks-view-picker.tsx b/apps/web/components/app-sidebar/sections/tasks-view-picker.tsx index 4252014d76..1534c19ae8 100644 --- a/apps/web/components/app-sidebar/sections/tasks-view-picker.tsx +++ b/apps/web/components/app-sidebar/sections/tasks-view-picker.tsx @@ -1,15 +1,17 @@ "use client"; -import { useMemo, useState } from "react"; -import { IconChevronDown, IconAdjustments, IconCheck } from "@tabler/icons-react"; +import { useMemo, useRef } from "react"; +import { IconChevronDown, IconAdjustments, IconCheck, IconPlus } from "@tabler/icons-react"; import { DropdownMenu, DropdownMenuContent, DropdownMenuItem, + DropdownMenuSeparator, DropdownMenuTrigger, } from "@kandev/ui/dropdown-menu"; import { useAppStore } from "@/components/state-provider"; import { SidebarFilterPopover } from "@/components/task/sidebar-filter/sidebar-filter-popover"; +import { useSidebarViewPopover } from "@/components/task/sidebar-filter/use-sidebar-view-popover"; import { cn } from "@/lib/utils"; const TRIGGER_BUTTON_CLASS = cn( @@ -22,7 +24,15 @@ export function TasksViewPicker() { const activeViewId = useAppStore((s) => s.sidebarViews.activeViewId); const draft = useAppStore((s) => s.sidebarViews.draft); const setActiveView = useAppStore((s) => s.setSidebarActiveView); - const [filterOpen, setFilterOpen] = useState(false); + const openPopoverAfterPickerCloseRef = useRef(false); + const { + open, + onOpenChange, + startNewView, + renameRequestedViewId, + consumeRenameRequest, + newViewDisabledReason, + } = useSidebarViewPopover(); const activeView = useMemo( () => views.find((v) => v.id === activeViewId) ?? views[0], @@ -45,7 +55,16 @@ export function TasksViewPicker() { - + { + if (!openPopoverAfterPickerCloseRef.current) return; + event.preventDefault(); + openPopoverAfterPickerCloseRef.current = false; + onOpenChange(true); + }} + > {views.map((view) => { const isActive = view.id === activeViewId; return ( @@ -63,11 +82,21 @@ export function TasksViewPicker() { ); })} + + { + openPopoverAfterPickerCloseRef.current = startNewView({ openPopover: false }); + }} + /> ); } + +function NewViewMenuItem({ + disabledReason, + hasDraft, + onSelect, +}: { + disabledReason: string | null; + hasDraft: boolean; + onSelect: () => void; +}) { + return ( + + + New view + {disabledReason && ( + + )} + + ); +} diff --git a/apps/web/components/task/sidebar-filter/sidebar-filter-bar.tsx b/apps/web/components/task/sidebar-filter/sidebar-filter-bar.tsx index 7e8dc02114..d222ddcc22 100644 --- a/apps/web/components/task/sidebar-filter/sidebar-filter-bar.tsx +++ b/apps/web/components/task/sidebar-filter/sidebar-filter-bar.tsx @@ -1,21 +1,32 @@ "use client"; -import { useMemo, useState } from "react"; -import { IconAdjustments } from "@tabler/icons-react"; +import { useMemo, useRef } from "react"; +import { IconAdjustments, IconPlus } from "@tabler/icons-react"; import { Button } from "@kandev/ui/button"; +import { toast } from "sonner"; import { useAppStore } from "@/components/state-provider"; import { useRegisterCommands } from "@/hooks/use-register-commands"; import type { CommandItem } from "@/lib/commands/types"; +import { cn } from "@/lib/utils"; import { SidebarViewChips } from "./sidebar-view-chips"; import { SidebarFilterPopover } from "./sidebar-filter-popover"; +import { useSidebarViewPopover } from "./use-sidebar-view-popover"; export function SidebarFilterBar() { - const [open, setOpen] = useState(false); + const filterTriggerRef = useRef(null); const draft = useAppStore((s) => s.sidebarViews.draft); const activeViewId = useAppStore((s) => s.sidebarViews.activeViewId); const views = useAppStore((s) => s.sidebarViews.views); const setActiveView = useAppStore((s) => s.setSidebarActiveView); const hasDraft = !!draft && draft.baseViewId === activeViewId; + const { + open, + onOpenChange, + startNewView, + renameRequestedViewId, + consumeRenameRequest, + newViewDisabledReason, + } = useSidebarViewPopover(); const commands = useMemo(() => { const list: CommandItem[] = [ @@ -24,7 +35,7 @@ export function SidebarFilterBar() { label: "Open sidebar filters", group: "Sidebar", keywords: ["filter", "sort", "group", "view", "sidebar"], - action: () => setOpen(true), + action: () => onOpenChange(true), }, ]; for (const view of views) { @@ -37,7 +48,7 @@ export function SidebarFilterBar() { }); } return list; - }, [views, setActiveView]); + }, [onOpenChange, views, setActiveView]); useRegisterCommands(commands); return ( @@ -45,27 +56,60 @@ export function SidebarFilterBar() { data-testid="sidebar-filter-bar" // Transparent so the bar inherits whatever surface hosts it — bg-card in // the dockview sidebar, bg-background in the mobile sheet — instead of - // painting a clashing strip. px-3 aligns the chip row's left edge with the - // 12px content inset of the group headers and task rows below. - className="flex h-[30px] shrink-0 items-center gap-1 border-b border-border/60 bg-transparent px-3" + // painting a clashing strip. Mobile px-2 leaves room for fixed touch + // actions; md:px-3 aligns with the 12px content inset below. + className="flex h-11 shrink-0 items-center gap-1 border-b border-border/60 bg-transparent px-2 md:h-[30px] md:px-3" > + {hasDraft && ( )} diff --git a/apps/web/components/task/sidebar-filter/sidebar-filter-popover.tsx b/apps/web/components/task/sidebar-filter/sidebar-filter-popover.tsx index 37e3f2ed74..3e031c65ab 100644 --- a/apps/web/components/task/sidebar-filter/sidebar-filter-popover.tsx +++ b/apps/web/components/task/sidebar-filter/sidebar-filter-popover.tsx @@ -22,6 +22,8 @@ type Props = { trigger: React.ReactNode; open: boolean; onOpenChange: (open: boolean) => void; + renameRequestedViewId?: string | null; + onRenameRequestHandled?: (viewId: string) => void; }; function makeClauseId(): string { @@ -43,7 +45,13 @@ function resolveCurrent( return { filters: activeView.filters, sort: activeView.sort, group: activeView.group }; } -export function SidebarFilterPopover({ trigger, open, onOpenChange }: Props) { +export function SidebarFilterPopover({ + trigger, + open, + onOpenChange, + renameRequestedViewId, + onRenameRequestHandled, +}: Props) { const views = useAppStore((s) => s.sidebarViews.views); const activeViewId = useAppStore((s) => s.sidebarViews.activeViewId); const storedDraft = useAppStore((s) => s.sidebarViews.draft); @@ -85,7 +93,11 @@ export function SidebarFilterPopover({ trigger, open, onOpenChange }: Props) { return ( {trigger} - +
activeView && deleteView(activeView.id)} + renameRequestedViewId={renameRequestedViewId} + onRenameRequestHandled={onRenameRequestHandled} />
diff --git a/apps/web/components/task/sidebar-filter/sidebar-view-chips.tsx b/apps/web/components/task/sidebar-filter/sidebar-view-chips.tsx index af1de696e8..de8be7467d 100644 --- a/apps/web/components/task/sidebar-filter/sidebar-view-chips.tsx +++ b/apps/web/components/task/sidebar-filter/sidebar-view-chips.tsx @@ -54,7 +54,7 @@ export function SidebarViewChips() { strategy={horizontalListSortingStrategy} >
{views.map((view) => ( @@ -144,7 +144,7 @@ function SidebarViewChip({ data-active={active} aria-pressed={active} className={cn( - "flex h-6 shrink-0 cursor-pointer items-center rounded-md border px-2 text-left text-[11px] transition-colors active:cursor-grabbing", + "flex h-10 shrink-0 cursor-pointer items-center rounded-md border px-2 text-left text-[11px] transition-colors active:cursor-grabbing md:h-6", active ? "border-primary/40 bg-primary/10 text-foreground" : "border-transparent text-muted-foreground hover:text-foreground", diff --git a/apps/web/components/task/sidebar-filter/use-sidebar-view-popover.test.tsx b/apps/web/components/task/sidebar-filter/use-sidebar-view-popover.test.tsx new file mode 100644 index 0000000000..cfef29b121 --- /dev/null +++ b/apps/web/components/task/sidebar-filter/use-sidebar-view-popover.test.tsx @@ -0,0 +1,92 @@ +import { act, cleanup, renderHook, waitFor } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { SidebarView } from "@/lib/state/slices/ui/sidebar-view-types"; + +const ALL_VIEW: SidebarView = { + id: "view-all", + name: "All tasks", + filters: [], + sort: { key: "state", direction: "asc" }, + group: "repository", + collapsedGroups: [], +}; + +const mockState = vi.hoisted(() => ({ + sidebarViews: { + views: [] as SidebarView[], + activeViewId: "view-all", + draft: null as { baseViewId: string } | null, + }, + createSidebarView: vi.fn<() => string | null>(), +})); + +vi.mock("@/components/state-provider", () => ({ + useAppStore: (selector: (state: typeof mockState) => unknown) => selector(mockState), +})); + +import { getNewViewDisabledReason, useSidebarViewPopover } from "./use-sidebar-view-popover"; + +beforeEach(() => { + mockState.sidebarViews.views = [ALL_VIEW]; + mockState.sidebarViews.activeViewId = ALL_VIEW.id; + mockState.sidebarViews.draft = null; + mockState.createSidebarView.mockReset(); + mockState.createSidebarView.mockImplementation(() => { + const created = { ...ALL_VIEW, id: "view-new", name: "New view" }; + mockState.sidebarViews.views = [...mockState.sidebarViews.views, created]; + mockState.sidebarViews.activeViewId = created.id; + return created.id; + }); +}); + +afterEach(cleanup); + +describe("useSidebarViewPopover", () => { + it("opens rename for the exact view returned by instant creation", () => { + const { result } = renderHook(() => useSidebarViewPopover()); + + act(() => expect(result.current.startNewView()).toBe(true)); + + expect(result.current.open).toBe(true); + expect(result.current.renameRequestedViewId).toBe("view-new"); + expect(mockState.createSidebarView).toHaveBeenCalledOnce(); + + act(() => result.current.consumeRenameRequest("view-new")); + expect(result.current.renameRequestedViewId).toBeNull(); + }); + + it("clears pending rename when the popover closes or creation rolls back", async () => { + const { result, rerender } = renderHook(() => useSidebarViewPopover()); + act(() => expect(result.current.startNewView()).toBe(true)); + + act(() => result.current.onOpenChange(false)); + expect(result.current.renameRequestedViewId).toBeNull(); + + act(() => expect(result.current.startNewView()).toBe(true)); + mockState.sidebarViews.views = [ALL_VIEW]; + rerender(); + await waitFor(() => expect(result.current.renameRequestedViewId).toBeNull()); + }); + + it("blocks creation for a draft or the 50-view limit", () => { + expect(getNewViewDisabledReason(1, true)).toMatch(/save or discard/i); + expect(getNewViewDisabledReason(50, false)).toMatch(/50/); + + mockState.sidebarViews.draft = { baseViewId: ALL_VIEW.id }; + const { result, rerender } = renderHook(() => useSidebarViewPopover()); + expect(result.current.newViewDisabledReason).toMatch(/save or discard/i); + act(() => expect(result.current.startNewView()).toBe(false)); + expect(mockState.createSidebarView).not.toHaveBeenCalled(); + + mockState.sidebarViews.draft = null; + mockState.sidebarViews.views = Array.from({ length: 50 }, (_, index) => ({ + ...ALL_VIEW, + id: `view-${index}`, + name: `View ${index}`, + })); + rerender(); + expect(result.current.newViewDisabledReason).toMatch(/50/); + act(() => expect(result.current.startNewView()).toBe(false)); + expect(mockState.createSidebarView).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/web/components/task/sidebar-filter/use-sidebar-view-popover.ts b/apps/web/components/task/sidebar-filter/use-sidebar-view-popover.ts new file mode 100644 index 0000000000..5790c690a9 --- /dev/null +++ b/apps/web/components/task/sidebar-filter/use-sidebar-view-popover.ts @@ -0,0 +1,58 @@ +"use client"; + +import { useCallback, useEffect, useState } from "react"; +import { useAppStore } from "@/components/state-provider"; +import { MAX_SIDEBAR_VIEWS } from "@/lib/state/slices/ui/sidebar-view-builtins"; + +export function getNewViewDisabledReason(viewCount: number, hasDraft: boolean): string | null { + if (hasDraft) return "Save or discard changes before creating a new view."; + if (viewCount >= MAX_SIDEBAR_VIEWS) { + return `You can save up to ${MAX_SIDEBAR_VIEWS} views.`; + } + return null; +} + +export function useSidebarViewPopover() { + const views = useAppStore((state) => state.sidebarViews.views); + const draft = useAppStore((state) => state.sidebarViews.draft); + const createSidebarView = useAppStore((state) => state.createSidebarView); + const [open, setOpen] = useState(false); + const [renameRequestedViewId, setRenameRequestedViewId] = useState(null); + const newViewDisabledReason = getNewViewDisabledReason(views.length, draft !== null); + + const startNewView = useCallback( + (options?: { openPopover?: boolean }): boolean => { + if (newViewDisabledReason) return false; + const createdViewId = createSidebarView(); + if (!createdViewId) return false; + setRenameRequestedViewId(createdViewId); + if (options?.openPopover !== false) setOpen(true); + return true; + }, + [createSidebarView, newViewDisabledReason], + ); + + const onOpenChange = useCallback((nextOpen: boolean) => { + setOpen(nextOpen); + if (!nextOpen) setRenameRequestedViewId(null); + }, []); + + const consumeRenameRequest = useCallback((viewId: string) => { + setRenameRequestedViewId((current) => (current === viewId ? null : current)); + }, []); + + useEffect(() => { + if (renameRequestedViewId && !views.some((view) => view.id === renameRequestedViewId)) { + setRenameRequestedViewId(null); + } + }, [renameRequestedViewId, views]); + + return { + open, + onOpenChange, + startNewView, + renameRequestedViewId, + consumeRenameRequest, + newViewDisabledReason, + }; +} diff --git a/apps/web/components/task/sidebar-filter/view-manager.test.tsx b/apps/web/components/task/sidebar-filter/view-manager.test.tsx new file mode 100644 index 0000000000..f0509c2077 --- /dev/null +++ b/apps/web/components/task/sidebar-filter/view-manager.test.tsx @@ -0,0 +1,100 @@ +import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { SidebarView } from "@/lib/state/slices/ui/sidebar-view-types"; +import { ViewHeaderRow } from "./view-manager"; + +const NEW_VIEW: SidebarView = { + id: "view-new", + name: "New view", + filters: [], + sort: { key: "state", direction: "asc" }, + group: "repository", + collapsedGroups: [], +}; +const RENAME_INPUT_TEST_ID = "view-rename-input"; + +function headerProps(activeView: SidebarView | undefined = NEW_VIEW) { + return { + activeView, + hasDraft: false, + canDelete: true, + onSaveOverwrite: vi.fn(), + onSaveAs: vi.fn(), + onRename: vi.fn(), + onDiscard: vi.fn(), + onDelete: vi.fn(), + renameRequestedViewId: null as string | null, + onRenameRequestHandled: vi.fn(), + }; +} + +afterEach(cleanup); + +describe("ViewHeaderRow external rename", () => { + it("focuses and selects the requested active view name", async () => { + const props = headerProps(); + props.renameRequestedViewId = NEW_VIEW.id; + render(); + + const input = await screen.findByTestId(RENAME_INPUT_TEST_ID); + expect((input as HTMLInputElement).value).toBe("New view"); + expect(screen.getByRole("textbox", { name: "View name" })).toBe(input); + expect(document.activeElement).toBe(input); + expect((input as HTMLInputElement).selectionStart).toBe(0); + expect((input as HTMLInputElement).selectionEnd).toBe("New view".length); + expect(props.onRenameRequestHandled).toHaveBeenCalledWith(NEW_VIEW.id); + }); + + it("does not restart rename for an equivalent active-view object", async () => { + const props = headerProps(); + props.renameRequestedViewId = NEW_VIEW.id; + const { rerender } = render(); + await screen.findByTestId(RENAME_INPUT_TEST_ID); + + rerender(); + + expect(props.onRenameRequestHandled).toHaveBeenCalledOnce(); + }); + + it("keeps the created view when rename is cancelled", async () => { + const props = headerProps(); + props.renameRequestedViewId = NEW_VIEW.id; + render(); + + await screen.findByTestId(RENAME_INPUT_TEST_ID); + fireEvent.click(screen.getByRole("button", { name: "Cancel" })); + + expect(screen.queryByTestId(RENAME_INPUT_TEST_ID)).toBeNull(); + expect(screen.getByTestId("sidebar-filter-active-view-name").textContent).toBe("New view"); + expect(props.onRename).not.toHaveBeenCalled(); + expect(props.onDelete).not.toHaveBeenCalled(); + }); + + it("exits stale rename mode if optimistic creation rolls back", async () => { + const props = headerProps(); + props.renameRequestedViewId = NEW_VIEW.id; + const { rerender } = render(); + await screen.findByTestId(RENAME_INPUT_TEST_ID); + + const restored = { ...headerProps({ ...NEW_VIEW, id: "view-all", name: "All tasks" }) }; + rerender(); + + await waitFor(() => { + expect(screen.queryByTestId(RENAME_INPUT_TEST_ID)).toBeNull(); + }); + expect(screen.getByTestId("sidebar-filter-active-view-name").textContent).toBe("All tasks"); + }); + + it("submits the requested rename without closing the surrounding popover", async () => { + const props = headerProps(); + props.renameRequestedViewId = NEW_VIEW.id; + render(); + const input = await screen.findByTestId(RENAME_INPUT_TEST_ID); + + fireEvent.change(input, { target: { value: "Focused work" } }); + fireEvent.keyDown(input, { key: "Enter" }); + + expect(props.onRename).toHaveBeenCalledWith(NEW_VIEW.id, "Focused work"); + expect(screen.queryByTestId(RENAME_INPUT_TEST_ID)).toBeNull(); + }); +}); diff --git a/apps/web/components/task/sidebar-filter/view-manager.tsx b/apps/web/components/task/sidebar-filter/view-manager.tsx index a2faa32965..c837493ca4 100644 --- a/apps/web/components/task/sidebar-filter/view-manager.tsx +++ b/apps/web/components/task/sidebar-filter/view-manager.tsx @@ -1,6 +1,6 @@ "use client"; -import { useState } from "react"; +import { useLayoutEffect, useRef, useState } from "react"; import { Input } from "@kandev/ui/input"; import { Button } from "@kandev/ui/button"; import type { SidebarView } from "@/lib/state/slices/ui/sidebar-view-types"; @@ -16,16 +16,22 @@ type HeaderProps = { onRename: (id: string, name: string) => void; onDiscard: () => void; onDelete: () => void; + renameRequestedViewId?: string | null; + onRenameRequestHandled?: (viewId: string) => void; }; export function ViewHeaderRow(props: HeaderProps) { const [mode, setMode] = useState("view"); const [nameDraft, setNameDraft] = useState(""); + const [editingViewId, setEditingViewId] = useState(null); + const activeViewId = props.activeView?.id; + const activeViewName = props.activeView?.name; const isEditing = mode !== "view"; function enterRename() { if (!props.activeView) return; setNameDraft(props.activeView.name); + setEditingViewId(props.activeView.id); setMode("rename"); } function enterSaveAs() { @@ -35,6 +41,7 @@ export function ViewHeaderRow(props: HeaderProps) { function exit() { setMode("view"); setNameDraft(""); + setEditingViewId(null); } function submit() { const trimmed = nameDraft.trim(); @@ -44,6 +51,23 @@ export function ViewHeaderRow(props: HeaderProps) { exit(); } + useLayoutEffect(() => { + const requestedViewId = props.renameRequestedViewId; + if (!requestedViewId || activeViewId !== requestedViewId || activeViewName === undefined) + return; + setNameDraft(activeViewName); + setEditingViewId(requestedViewId); + setMode("rename"); + props.onRenameRequestHandled?.(requestedViewId); + }, [activeViewId, activeViewName, props.onRenameRequestHandled, props.renameRequestedViewId]); + + useLayoutEffect(() => { + if (mode !== "rename" || !editingViewId || activeViewId === editingViewId) return; + setMode("view"); + setNameDraft(""); + setEditingViewId(null); + }, [activeViewId, editingViewId, mode]); + return (
@@ -112,9 +136,17 @@ function NameInput({ onSubmit: () => void; onCancel: () => void; }) { + const inputRef = useRef(null); + + useLayoutEffect(() => { + inputRef.current?.focus(); + inputRef.current?.select(); + }, []); + return ( onChange(e.target.value)} onKeyDown={(e) => { diff --git a/apps/web/e2e/pages/sidebar-filter-popover.ts b/apps/web/e2e/pages/sidebar-filter-popover.ts index 06b3db5307..f752e61cda 100644 --- a/apps/web/e2e/pages/sidebar-filter-popover.ts +++ b/apps/web/e2e/pages/sidebar-filter-popover.ts @@ -35,6 +35,11 @@ export class SidebarFilterPopoverPage { return this.page.getByTestId("sidebar-filter-popover"); } + /** Direct-create action inside the currently open desktop view menu. */ + get newViewAction(): Locator { + return this.chipMenu.getByTestId("sidebar-new-view"); + } + /** Open radix dropdown menu hosting the view chips. Scoped to the menu that * actually contains the view chips so it can't match another sidebar menu. */ get chipMenu(): Locator { @@ -78,6 +83,19 @@ export class SidebarFilterPopoverPage { await expect(this.chipMenu).toBeHidden(); } + /** Create immediately, then wait for the optional rename handoff. */ + async beginNewView(): Promise { + await this.openViewPicker(); + await this.newViewAction.scrollIntoViewIfNeeded(); + await this.newViewAction.click(); + await expect(this.chipMenu).toBeHidden(); + await expect(this.popover).toBeVisible(); + const input = this.popover.getByTestId("view-rename-input"); + await expect(input).toBeVisible(); + await expect(input).toBeFocused(); + return input; + } + /** * Assert the active view by name. The active view label is rendered on the * picker trigger button, so this works without opening the dropdown. diff --git a/apps/web/e2e/tests/task/mobile-sidebar-views.spec.ts b/apps/web/e2e/tests/task/mobile-sidebar-views.spec.ts index 53b7559a2c..4a6119c65d 100644 --- a/apps/web/e2e/tests/task/mobile-sidebar-views.spec.ts +++ b/apps/web/e2e/tests/task/mobile-sidebar-views.spec.ts @@ -2,12 +2,9 @@ * Mobile parity for the sidebar view system (filters / sort / group + saved * views). * - * The desktop sidebar exposes a `SidebarFilterBar` (saved-view chip row + a - * filter gear that opens the shared `SidebarFilterPopover`). That same bar is - * now folded into the mobile task-switcher sheet so mobile users can switch and - * edit views instead of being stuck on whatever view was last picked on - * desktop. The sheet already applied the active view via `applyView`; these - * tests guard the newly-added switching/editing UI. + * The mobile task-switcher sheet exposes a saved-view chip row, fixed direct + * creation action, and filter gear backed by the shared `SidebarFilterPopover`. + * The desktop AppSidebar uses a separate dropdown picker for the same state. * * Lives in `mobile-*.spec.ts` so the `mobile-chrome` Playwright project applies * the mobile device automatically. @@ -59,6 +56,73 @@ async function addTitleFilter(testPage: Page, sheet: Locator, value: string): Pr } test.describe("Mobile sidebar — view system", () => { + test("direct creation stays touch-reachable, viewport-safe, and persists", async ({ + testPage, + apiClient, + seedData, + }) => { + const sheet = await seedAndOpenSheet(testPage, apiClient, seedData, ["Mobile New View Task"]); + const newView = sheet.getByTestId("sidebar-new-view"); + const gear = sheet.getByTestId("sidebar-filter-gear"); + await expect(newView).toBeVisible(); + await expect(newView).toContainText("New view"); + const newViewBox = await newView.boundingBox(); + const gearBox = await gear.boundingBox(); + const chipRowBox = await sheet.getByTestId("sidebar-view-chip-row").boundingBox(); + expect(newViewBox?.height).toBeGreaterThanOrEqual(40); + expect(gearBox?.height).toBeGreaterThanOrEqual(40); + expect(chipRowBox!.x + chipRowBox!.width).toBeLessThanOrEqual(newViewBox!.x); + expect(newViewBox!.x + newViewBox!.width).toBeLessThanOrEqual(gearBox!.x); + await expect( + sheet.getByTestId("sidebar-view-chip-row").getByTestId("sidebar-new-view"), + ).toHaveCount(0); + + await newView.click(); + const popover = testPage.getByTestId("sidebar-filter-popover"); + const renameInput = popover.getByTestId("view-rename-input"); + await expect(popover).toBeVisible(); + await expect(renameInput).toBeFocused(); + await expect(renameInput).toHaveValue("New view"); + const popoverBox = await popover.boundingBox(); + const viewport = testPage.viewportSize(); + expect(popoverBox).not.toBeNull(); + expect(viewport).not.toBeNull(); + expect(popoverBox!.x).toBeGreaterThanOrEqual(0); + expect(popoverBox!.x + popoverBox!.width).toBeLessThanOrEqual(viewport!.width); + + await popover.getByRole("button", { name: "Cancel" }).click(); + await testPage.keyboard.press("Escape"); + await expect( + sheet.getByTestId("sidebar-view-chip-row").getByTestId("sidebar-view-chip").filter({ + hasText: "New view", + }), + ).toHaveAttribute("data-active", "true"); + expect( + await testPage.evaluate( + () => document.documentElement.scrollWidth <= document.documentElement.clientWidth, + ), + ).toBe(true); + await expect + .poll(async () => { + const { settings } = await apiClient.getUserSettings(); + return (settings.sidebar_views as Array<{ name?: string }> | undefined)?.some( + (view) => view.name === "New view", + ); + }) + .toBe(true); + + await testPage.reload(); + await new SessionPage(testPage).waitForLoad(); + await testPage.getByTestId("mobile-session-menu").click(); + const reloadedSheet = testPage.getByRole("dialog"); + await expect( + reloadedSheet + .getByTestId("sidebar-view-chip-row") + .getByTestId("sidebar-view-chip") + .filter({ hasText: "New view" }), + ).toHaveAttribute("data-active", "true"); + }); + test("editing filters in the mobile sheet narrows the task list live", async ({ testPage, apiClient, @@ -79,6 +143,15 @@ test.describe("Mobile sidebar — view system", () => { // the globally-mounted (hidden on mobile) AppSidebar TasksViewPicker renders // the same testid, so a page-level query is a strict-mode collision. await expect(sheet.getByTestId("sidebar-filter-gear-indicator")).toBeVisible(); + const blockedNewView = sheet.getByTestId("sidebar-new-view"); + await expect(blockedNewView).toHaveAttribute("aria-disabled", "true"); + await expect(blockedNewView).toHaveAttribute("aria-label", /save or discard changes/i); + // aria-disabled communicates the blocked state, but this action remains + // clickable so touch/keyboard users can get the concrete reason toast. + await blockedNewView.click({ force: true }); + await expect( + testPage.getByText("Save or discard changes before creating a new view."), + ).toBeVisible(); await testPage.keyboard.press("Escape"); // The list inside the sheet re-filters live via applyView. @@ -118,6 +191,44 @@ test.describe("Mobile sidebar — view system", () => { await expect(sheet.getByText("Fix auth bug")).toBeVisible(); }); + test("many saved views scroll without covering fixed actions", async ({ + testPage, + apiClient, + seedData, + }) => { + await seedAndOpenSheet(testPage, apiClient, seedData, ["Scrollable Views Task"]); + const views = Array.from({ length: 8 }, (_, index) => ({ + id: `mobile-scroll-${index}`, + name: `Long mobile view ${index + 1}`, + filters: [], + sort: { key: "state", direction: "asc" }, + group: "repository", + collapsed_groups: [], + })); + const response = await apiClient.rawRequest("PATCH", "/api/v1/user/settings", { + sidebar_views: views, + sidebar_active_view_id: views[0].id, + sidebar_draft: null, + }); + expect(response.ok).toBe(true); + + await testPage.reload(); + await new SessionPage(testPage).waitForLoad(); + await testPage.getByTestId("mobile-session-menu").click(); + const sheet = testPage.getByRole("dialog"); + const chipRow = sheet.getByTestId("sidebar-view-chip-row"); + await expect(chipRow).toBeVisible(); + expect(await chipRow.evaluate((element) => element.scrollWidth > element.clientWidth)).toBe( + true, + ); + await chipRow.evaluate((element) => { + element.scrollLeft = element.scrollWidth; + }); + expect(await chipRow.evaluate((element) => element.scrollLeft)).toBeGreaterThan(0); + await expect(sheet.getByTestId("sidebar-new-view")).toBeVisible(); + await expect(sheet.getByTestId("sidebar-filter-gear")).toBeVisible(); + }); + test("tapping a group header collapses and expands the group in the sheet", async ({ testPage, apiClient, diff --git a/apps/web/e2e/tests/task/sidebar-filter.spec.ts b/apps/web/e2e/tests/task/sidebar-filter.spec.ts index 6deef7bfcb..cca8918500 100644 --- a/apps/web/e2e/tests/task/sidebar-filter.spec.ts +++ b/apps/web/e2e/tests/task/sidebar-filter.spec.ts @@ -15,7 +15,7 @@ * - Filter add/remove, negation, live list update * - Sort + direction toggle * - Group-by (repository, state, none) - * - Saved views CRUD (save-as, rename, delete) + * - Direct saved-view creation plus save-as, rename, and delete * - Persistence across reload * - Draft semantics + discard * - Last-view deletion guard @@ -252,6 +252,125 @@ test.describe("Sidebar filter — group + sort", () => { }); test.describe("Sidebar filter — saved views CRUD", () => { + test("direct creation starts from canonical defaults, focuses rename, and persists", async ({ + testPage, + apiClient, + seedData, + }) => { + const { filters } = await openWithSeed(testPage, apiClient, seedData, ["Direct View Task"]); + + // Make the active source visibly non-default before creating the blank view. + await filters.addFilterRow(); + await filters.setClauseDimension(0, "Title"); + await filters.setClauseTextValue(0, "source-only"); + await filters.setSort("Updated", "desc"); + await filters.setGroup("State"); + await filters.saveAs("Custom source"); + await filters.close(); + + const renameInput = await filters.beginNewView(); + await expect(renameInput).toHaveValue("New view"); + await expect(filters.popover.getByTestId("filter-clause-row")).toHaveCount(0); + await expect(filters.popover.getByTestId("sort-key-select")).toContainText("Status"); + await expect(filters.popover.getByTestId("sort-direction-toggle")).toHaveAttribute( + "data-direction", + "asc", + ); + await expect(filters.popover.getByTestId("group-key-select")).toContainText("Repository"); + + await renameInput.fill("Planning view"); + await filters.popover.getByTestId("view-rename-confirm").click(); + await expect(filters.popover).toBeVisible(); + await expect(filters.popover.getByTestId("sidebar-filter-active-view-name")).toContainText( + "Planning view", + ); + await expect + .poll(async () => { + const { settings } = await apiClient.getUserSettings(); + return (settings.sidebar_views as Array<{ name?: string }> | undefined)?.some( + (view) => view.name === "Planning view", + ); + }) + .toBe(true); + + await testPage.reload(); + const session = new SessionPage(testPage); + await session.waitForLoad(); + await new SidebarFilterPopoverPage(testPage).expectActiveViewChip("Planning view"); + }); + + test("cancelling optional rename keeps automatic names and creates the next gap", async ({ + testPage, + apiClient, + seedData, + }) => { + const { filters } = await openWithSeed(testPage, apiClient, seedData, ["Auto Name Task"]); + + await filters.beginNewView(); + await filters.popover.getByRole("button", { name: "Cancel" }).click(); + await expect(filters.popover.getByTestId("sidebar-filter-active-view-name")).toContainText( + "New view", + ); + await filters.close(); + + const secondName = await filters.beginNewView(); + await expect(secondName).toHaveValue("New view 2"); + await filters.popover.getByRole("button", { name: "Cancel" }).click(); + await filters.close(); + await filters.expectChipOrder(["All tasks", "New view", "New view 2"]); + }); + + test("direct creation explains dirty-draft and saved-view-limit blocks", async ({ + testPage, + apiClient, + seedData, + }) => { + const { filters } = await openWithSeed(testPage, apiClient, seedData, ["Blocked View Task"]); + await filters.addFilterRow(); + await filters.close(); + await filters.openViewPicker(); + await expect(filters.newViewAction).toHaveAttribute("aria-disabled", "true"); + await expect(filters.newViewAction).toHaveAttribute("aria-label", /save or discard changes/i); + await filters.closeViewPicker(); + await filters.open(); + await expect(filters.popover.getByTestId("sidebar-filter-dirty-indicator")).toBeVisible(); + await filters.discard(); + await expect + .poll(async () => { + const { settings } = await apiClient.getUserSettings(); + return settings.sidebar_draft ?? null; + }) + .toBeNull(); + + const limitViews = Array.from({ length: 50 }, (_, index) => ({ + id: `limit-view-${index}`, + name: `Limit view ${index + 1}`, + filters: [], + sort: { key: "state", direction: "asc" }, + group: "repository", + collapsed_groups: [], + })); + const response = await apiClient.rawRequest("PATCH", "/api/v1/user/settings", { + sidebar_views: limitViews, + sidebar_active_view_id: limitViews[0].id, + sidebar_draft: null, + }); + expect(response.ok).toBe(true); + await expect + .poll(async () => { + const { settings } = await apiClient.getUserSettings(); + return (settings.sidebar_views as unknown[] | undefined)?.length; + }) + .toBe(50); + await testPage.reload(); + await new SessionPage(testPage).waitForLoad(); + const reloaded = new SidebarFilterPopoverPage(testPage); + await reloaded.openViewPicker(); + await reloaded.newViewAction.scrollIntoViewIfNeeded(); + await expect(reloaded.newViewAction).toHaveAttribute("aria-disabled", "true"); + await expect(reloaded.newViewAction).toHaveAttribute("aria-label", /up to 50 views/i); + }); + test("save-as creates a new chip and selects it", async ({ testPage, apiClient, seedData }) => { const { filters } = await openWithSeed(testPage, apiClient, seedData, ["View CRUD Task"]); await filters.addFilterRow(); diff --git a/apps/web/lib/state/slices/ui/sidebar-view-actions.test.ts b/apps/web/lib/state/slices/ui/sidebar-view-actions.test.ts new file mode 100644 index 0000000000..64a38368a1 --- /dev/null +++ b/apps/web/lib/state/slices/ui/sidebar-view-actions.test.ts @@ -0,0 +1,245 @@ +import { waitFor } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { create, type StoreApi, type UseBoundStore } from "zustand"; +import { immer } from "zustand/middleware/immer"; +import { ApiError } from "@/lib/api/client"; +import { updateUserSettings } from "@/lib/api/domains/settings-api"; +import { DEFAULT_VIEW } from "./sidebar-view-builtins"; +import { createUISlice } from "./ui-slice"; +import type { SidebarView, SidebarViewDraft } from "./sidebar-view-types"; +import type { UISlice } from "./types"; + +vi.mock("@/lib/api/domains/settings-api", () => ({ + updateUserSettings: vi.fn(() => Promise.resolve({ settings: {} })), +})); + +type UIStore = UseBoundStore>; + +const CREATE_FAILED = "create failed"; +const RENAME_FAILED = "rename failed"; + +function makeStore(): UIStore { + return create()( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + immer((...args) => ({ ...(createUISlice as any)(...args) })), + ); +} + +function makeView(id: string, name: string): SidebarView { + return { + id, + name, + filters: [], + sort: { key: "state", direction: "asc" }, + group: "none", + collapsedGroups: [], + }; +} + +function seedSidebar( + store: UIStore, + views: SidebarView[], + draft: SidebarViewDraft | null = null, +): void { + store.setState((state) => ({ + ...state, + sidebarViews: { + ...state.sidebarViews, + views, + activeViewId: views[0].id, + draft, + }, + })); +} + +beforeEach(() => { + window.localStorage.clear(); + vi.mocked(updateUserSettings).mockReset(); + vi.mocked(updateUserSettings).mockResolvedValue({ + settings: {}, + } as Awaited>); +}); + +describe("createSidebarView semantics", () => { + it("appends and activates an independent canonical view with the first available name", () => { + const store = makeStore(); + const customized = makeView("custom", "Custom"); + customized.filters = [ + { id: "custom-filter", dimension: "titleMatch", op: "matches", value: "custom" }, + ]; + customized.sort = { key: "updatedAt", direction: "desc" }; + customized.group = "state"; + customized.collapsedGroups = ["active"]; + seedSidebar(store, [ + customized, + makeView("new-1", "New view"), + makeView("new-3", "New view 3"), + ]); + + const createdId = store.getState().createSidebarView(); + const sidebar = store.getState().sidebarViews; + const created = sidebar.views.at(-1); + + expect(created).toEqual({ + id: expect.stringMatching(/^view-/), + name: "New view 2", + filters: [], + sort: { key: "state", direction: "asc" }, + group: "repository", + collapsedGroups: [], + }); + expect(sidebar.activeViewId).toBe(createdId); + expect(sidebar.draft).toBeNull(); + expect(created?.filters).not.toBe(DEFAULT_VIEW.filters); + expect(created?.sort).not.toBe(DEFAULT_VIEW.sort); + expect(created?.collapsedGroups).not.toBe(DEFAULT_VIEW.collapsedGroups); + expect(updateUserSettings).toHaveBeenCalledWith({ + sidebar_views: [ + expect.objectContaining({ id: "custom", name: "Custom" }), + expect.objectContaining({ id: "new-1", name: "New view" }), + expect.objectContaining({ id: "new-3", name: "New view 3" }), + expect.objectContaining({ id: createdId, name: "New view 2", group: "repository" }), + ], + sidebar_active_view_id: createdId, + sidebar_draft: null, + }); + }); + + it("does nothing while the active view has an unsaved draft", () => { + const store = makeStore(); + const draft: SidebarViewDraft = { + baseViewId: "all", + filters: [], + sort: { key: "title", direction: "asc" }, + group: "workflow", + }; + seedSidebar(store, [makeView("all", "All tasks")], draft); + + expect(store.getState().createSidebarView()).toBeNull(); + expect(store.getState().sidebarViews.draft).toEqual(draft); + expect(store.getState().sidebarViews.views).toHaveLength(1); + expect(updateUserSettings).not.toHaveBeenCalled(); + }); + + it("does nothing at the saved-view limit", () => { + const store = makeStore(); + const views = Array.from({ length: 50 }, (_, index) => + makeView(`view-${index}`, `View ${index}`), + ); + seedSidebar(store, views); + + expect(store.getState().createSidebarView()).toBeNull(); + expect(store.getState().sidebarViews.views).toHaveLength(50); + expect(store.getState().sidebarViews.activeViewId).toBe(views[0].id); + expect(updateUserSettings).not.toHaveBeenCalled(); + }); +}); + +describe("createSidebarView queued rollback", () => { + it("rolls back to the pre-create state when create and immediate rename both fail", async () => { + const store = makeStore(); + const original = makeView("all", "All tasks"); + seedSidebar(store, [original]); + vi.mocked(updateUserSettings) + .mockRejectedValueOnce(new ApiError(CREATE_FAILED, 500, {})) + .mockRejectedValueOnce(new ApiError(RENAME_FAILED, 500, {})); + + const createdId = store.getState().createSidebarView(); + expect(createdId).not.toBeNull(); + store.getState().renameSidebarView(createdId!, "Renamed view"); + + await waitFor(() => expect(store.getState().sidebarViews.syncError).toBe(RENAME_FAILED)); + expect(store.getState().sidebarViews.views).toEqual([original]); + expect(store.getState().sidebarViews.activeViewId).toBe(original.id); + }); + + it("keeps the automatic view when create syncs but its immediate rename fails", async () => { + const store = makeStore(); + const original = makeView("all", "All tasks"); + seedSidebar(store, [original]); + vi.mocked(updateUserSettings) + .mockResolvedValueOnce({ settings: {} } as Awaited>) + .mockRejectedValueOnce(new ApiError(RENAME_FAILED, 500, {})); + + const createdId = store.getState().createSidebarView(); + expect(createdId).not.toBeNull(); + store.getState().renameSidebarView(createdId!, "Renamed view"); + + await waitFor(() => expect(store.getState().sidebarViews.syncError).toBe(RENAME_FAILED)); + expect(store.getState().sidebarViews.views).toEqual([ + original, + expect.objectContaining({ id: createdId, name: "New view" }), + ]); + expect(store.getState().sidebarViews.activeViewId).toBe(createdId); + }); + + it("restores a valid active view when two queued creates fail", async () => { + const store = makeStore(); + const original = makeView("all", "All tasks"); + seedSidebar(store, [original]); + vi.mocked(updateUserSettings) + .mockRejectedValueOnce(new ApiError("first create failed", 500, {})) + .mockRejectedValueOnce(new ApiError("second create failed", 500, {})); + + const firstCreatedId = store.getState().createSidebarView(); + store.getState().createSidebarView(); + store.getState().setSidebarActiveView(firstCreatedId!); + + await waitFor(() => + expect(store.getState().sidebarViews.syncError).toBe("second create failed"), + ); + expect(store.getState().sidebarViews.views).toEqual([original]); + expect(store.getState().sidebarViews.activeViewId).toBe(original.id); + }); +}); + +describe("createSidebarView failure isolation", () => { + it("restores the previous sidebar state when creation fails to sync", async () => { + const store = makeStore(); + const original = makeView("all", "All tasks"); + seedSidebar(store, [original]); + vi.mocked(updateUserSettings).mockRejectedValueOnce(new ApiError(CREATE_FAILED, 500, {})); + + store.getState().createSidebarView(); + expect(store.getState().sidebarViews.views).toHaveLength(2); + + await waitFor(() => expect(store.getState().sidebarViews.syncError).toBe(CREATE_FAILED)); + expect(store.getState().sidebarViews.views).toEqual([original]); + expect(store.getState().sidebarViews.activeViewId).toBe(original.id); + }); + + it("clears a draft based on a created view that fails to sync", async () => { + const store = makeStore(); + const original = makeView("all", "All tasks"); + seedSidebar(store, [original]); + vi.mocked(updateUserSettings).mockRejectedValueOnce(new ApiError(CREATE_FAILED, 500, {})); + + const createdId = store.getState().createSidebarView(); + expect(createdId).not.toBeNull(); + store.getState().updateSidebarDraft({ group: "state" }); + + await waitFor(() => expect(store.getState().sidebarViews.syncError).toBe(CREATE_FAILED)); + expect(store.getState().sidebarViews.views).toEqual([original]); + expect(store.getState().sidebarViews.activeViewId).toBe(original.id); + expect(store.getState().sidebarViews.draft).toBeNull(); + }); + + it("rolls back independently when another store mutates at the same time", async () => { + const failedStore = makeStore(); + const successfulStore = makeStore(); + seedSidebar(failedStore, [makeView("failed-all", "All tasks")]); + seedSidebar(successfulStore, [makeView("successful-all", "All tasks")]); + vi.mocked(updateUserSettings) + .mockRejectedValueOnce(new ApiError("isolated failure", 500, {})) + .mockResolvedValueOnce({ settings: {} } as Awaited>); + + failedStore.getState().createSidebarView(); + successfulStore.getState().createSidebarView(); + + await waitFor(() => + expect(failedStore.getState().sidebarViews.syncError).toBe("isolated failure"), + ); + expect(failedStore.getState().sidebarViews.views).toHaveLength(1); + expect(successfulStore.getState().sidebarViews.views).toHaveLength(2); + }); +}); diff --git a/apps/web/lib/state/slices/ui/sidebar-view-actions.ts b/apps/web/lib/state/slices/ui/sidebar-view-actions.ts index 465c2d547e..af68d15eca 100644 --- a/apps/web/lib/state/slices/ui/sidebar-view-actions.ts +++ b/apps/web/lib/state/slices/ui/sidebar-view-actions.ts @@ -9,6 +9,7 @@ import type { SortSpec, } from "./sidebar-view-types"; import { toApiSidebarDraft, toApiSidebarView } from "./sidebar-view-wire"; +import { createDefaultSidebarView, MAX_SIDEBAR_VIEWS } from "./sidebar-view-builtins"; type ImmerSet = (recipe: (draft: UISlice) => void, shouldReplace?: false | undefined) => void; @@ -31,7 +32,14 @@ function reorderViewsById( return next; } -let viewsSyncRequestId = 0; +function nextNewViewName(views: SidebarView[]): string { + const names = new Set(views.map((view) => view.name)); + if (!names.has("New view")) return "New view"; + let suffix = 2; + while (names.has(`New view ${suffix}`)) suffix += 1; + return `New view ${suffix}`; +} + const sidebarSettingsQueues = new WeakMap>(); type SidebarSnapshot = { @@ -40,6 +48,21 @@ type SidebarSnapshot = { draft: SidebarViewDraft | null; }; +type ViewMutationSyncState = { + latestRequestId: number; + failedRollback?: SidebarSnapshot; +}; + +const viewMutationSyncStates = new WeakMap(); + +function getViewMutationSyncState(set: ImmerSet): ViewMutationSyncState { + const existing = viewMutationSyncStates.get(set); + if (existing) return existing; + const created = { latestRequestId: 0 }; + viewMutationSyncStates.set(set, created); + return created; +} + function snapshotSidebar(s: UISliceState["sidebarViews"]): SidebarSnapshot { return { views: s.views.map(cloneView), @@ -103,22 +126,40 @@ function mutateViews( if (!committed) return; const after = get().sidebarViews; const afterSnapshot = snapshotSidebar(after); - const thisRequestId = ++viewsSyncRequestId; + const syncState = getViewMutationSyncState(set); + const thisRequestId = ++syncState.latestRequestId; const request = enqueueSidebarSettingsSync(set, toSidebarSettingsPayload(after)); - request.catch((err) => { - if (thisRequestId !== viewsSyncRequestId) return; - const message = err instanceof Error ? err.message : "Failed to sync sidebar views"; - set((draft) => { - draft.sidebarViews.views = snapshot.views; - if (draft.sidebarViews.activeViewId === afterSnapshot.activeViewId) { - draft.sidebarViews.activeViewId = snapshot.activeViewId; - } - if (draftsEqual(draft.sidebarViews.draft, afterSnapshot.draft)) { - draft.sidebarViews.draft = snapshot.draft; - } - draft.sidebarViews.syncError = message; - }); - }); + request.then( + () => { + syncState.failedRollback = undefined; + }, + (err) => { + const rollback = syncState.failedRollback ?? snapshot; + syncState.failedRollback = rollback; + if (thisRequestId !== syncState.latestRequestId) return; + const message = err instanceof Error ? err.message : "Failed to sync sidebar views"; + set((draft) => { + draft.sidebarViews.views = rollback.views; + const activeViewStillExists = rollback.views.some( + (view) => view.id === draft.sidebarViews.activeViewId, + ); + if ( + draft.sidebarViews.activeViewId === afterSnapshot.activeViewId || + !activeViewStillExists + ) { + draft.sidebarViews.activeViewId = rollback.activeViewId; + } + const currentDraft = draft.sidebarViews.draft; + const draftBaseStillExists = + !currentDraft || rollback.views.some((view) => view.id === currentDraft.baseViewId); + if (draftsEqual(currentDraft, afterSnapshot.draft) || !draftBaseStillExists) { + draft.sidebarViews.draft = rollback.draft; + } + draft.sidebarViews.syncError = message; + }); + syncState.failedRollback = undefined; + }, + ); } function buildSidebarLocalActions(set: ImmerSet, get: () => UISlice) { @@ -185,6 +226,17 @@ function buildSidebarBackendActions(set: ImmerSet, get: () => UISlice) { const mv = (mutate: (s: UISliceState["sidebarViews"]) => boolean | void) => mutateViews(set, get, mutate); return { + createSidebarView: () => { + let createdViewId: string | null = null; + mv((s) => { + if (s.draft || s.views.length >= MAX_SIDEBAR_VIEWS) return false; + const view = createDefaultSidebarView(makeId("view"), nextNewViewName(s.views)); + s.views.push(view); + s.activeViewId = view.id; + createdViewId = view.id; + }); + return createdViewId; + }, toggleSidebarGroupCollapsed: (viewId: string, groupKey: string) => mv((s) => { const view = s.views.find((v) => v.id === viewId); diff --git a/apps/web/lib/state/slices/ui/sidebar-view-builtins.ts b/apps/web/lib/state/slices/ui/sidebar-view-builtins.ts index 8e6e049329..8196ade42c 100644 --- a/apps/web/lib/state/slices/ui/sidebar-view-builtins.ts +++ b/apps/web/lib/state/slices/ui/sidebar-view-builtins.ts @@ -1,14 +1,19 @@ import type { SidebarView } from "./sidebar-view-types"; export const DEFAULT_VIEW_ID = "view-all-tasks"; +export const MAX_SIDEBAR_VIEWS = 50; -export const DEFAULT_VIEW: SidebarView = { - id: DEFAULT_VIEW_ID, - name: "All tasks", - filters: [], - sort: { key: "state", direction: "asc" }, - group: "repository", - collapsedGroups: [], -}; +export function createDefaultSidebarView(id: string, name: string): SidebarView { + return { + id, + name, + filters: [], + sort: { key: "state", direction: "asc" }, + group: "repository", + collapsedGroups: [], + }; +} + +export const DEFAULT_VIEW: SidebarView = createDefaultSidebarView(DEFAULT_VIEW_ID, "All tasks"); export const DEFAULT_ACTIVE_VIEW_ID = DEFAULT_VIEW_ID; diff --git a/apps/web/lib/state/slices/ui/types.ts b/apps/web/lib/state/slices/ui/types.ts index 1868a0699c..7877895b59 100644 --- a/apps/web/lib/state/slices/ui/types.ts +++ b/apps/web/lib/state/slices/ui/types.ts @@ -217,6 +217,7 @@ export type UISliceActions = { openBottomTerminalWithCommand: (command: string) => void; clearBottomTerminalCommand: () => void; setSidebarActiveView: (viewId: string) => void; + createSidebarView: () => string | null; updateSidebarDraft: ( patch: Partial<{ filters: FilterClause[]; sort: SortSpec; group: GroupKey }>, ) => void; diff --git a/apps/web/lib/state/store.ts b/apps/web/lib/state/store.ts index fd030b180c..b67f0ea470 100644 --- a/apps/web/lib/state/store.ts +++ b/apps/web/lib/state/store.ts @@ -525,6 +525,7 @@ export type AppState = { ) => void; setSessionPollMode: (sessionId: string, mode: SessionPollMode) => void; /* prettier-ignore */ setSidebarActiveView: UIA["setSidebarActiveView"]; + createSidebarView: UIA["createSidebarView"]; updateSidebarDraft: UIA["updateSidebarDraft"]; saveSidebarDraftAs: UIA["saveSidebarDraftAs"]; saveSidebarDraftOverwrite: UIA["saveSidebarDraftOverwrite"]; diff --git a/docs/plans/sidebar-view-creation/plan.md b/docs/plans/sidebar-view-creation/plan.md new file mode 100644 index 0000000000..4336d4ba99 --- /dev/null +++ b/docs/plans/sidebar-view-creation/plan.md @@ -0,0 +1,117 @@ +--- +spec: docs/specs/ui/sidebar-view-creation.md +created: 2026-07-17 +status: completed +--- + +# Implementation Plan: Direct Sidebar View Creation + +## Overview + +Add a guarded optimistic `createSidebarView()` state action first, then connect it to desktop and mobile controls through one shared popover/rename controller. Finish with production-build Playwright journeys proving immediate creation, optional rename, persistence, blocked states, and mobile touch parity. Existing user-settings contracts remain unchanged. + +--- + +## Backend + +No backend changes are planned. `apps/backend/internal/user/service/service.go::applySidebarViews` already validates the 50-view limit, nonblank names and IDs, and unique IDs; `applySidebarViewState` validates active-view membership. Existing settings storage, boot hydration, WebSocket updates, retries, rollback, and error toast remain the persistence path. + +## Frontend + +### Instant-create state contract + +- In `apps/web/lib/state/slices/ui/sidebar-view-builtins.ts`, keep `DEFAULT_VIEW` canonical and add a factory that creates independent default view data plus a frontend 50-view limit aligned with backend `maxSidebarViews`. +- In `apps/web/lib/state/slices/ui/sidebar-view-actions.ts`, add `createSidebarView()` to `buildSidebarBackendActions`. Reuse `makeId`, `mutateViews`, and `toSidebarSettingsPayload`; guard drafts and the limit, choose the lowest available automatic name, append canonical defaults, and activate the new view. +- Add the action signature to `UISliceActions` in `apps/web/lib/state/slices/ui/types.ts` and forward it through `AppState` in `apps/web/lib/state/store.ts`. +- Keep `saveSidebarDraftAs`, `duplicateSidebarView`, and their persistence behavior unchanged. + +### Shared creation and rename flow + +- Create `apps/web/components/task/sidebar-filter/use-sidebar-view-popover.ts` to own popover-open state, a monotonic rename-request token, `startNewView`, and disabled reasons for an active draft or the 50-view limit. +- Update `SidebarFilterPopover` in `sidebar-filter-popover.tsx` to accept the rename request, clear stale requests when the popover closes or the active view rolls back, and constrain its current `22rem` width to the viewport. +- Update `ViewHeaderRow` in `view-manager.tsx` to enter rename mode for an external request, prefill from the newly active view, expose an accessible input name, and retain existing autofocus, Enter-save, Escape/Cancel, manual Rename, and `Save as…` behavior. Saving rename leaves the popover open; cancel never deletes the view. + +### Desktop and mobile entry points + +- In `apps/web/components/app-sidebar/sections/tasks-view-picker.tsx`, follow `WorkspacePickerContent` from `app-sidebar-workspace-picker.tsx`: append `DropdownMenuSeparator` and an `IconPlus` `New view` item with a stable test ID and keyboard-safe `onSelect`. +- In `apps/web/components/task/sidebar-filter/sidebar-filter-bar.tsx`, add a fixed labeled `New view` action outside `SidebarViewChips` so touch users do not need to scroll or hover. Keep interactive hit areas at least 40×40 px without overlapping the chip or gear targets. +- Adjust `apps/web/components/task/sidebar-filter/sidebar-view-chips.tsx` only if needed to preserve swipe scrolling, press-and-hold drag, and compact desktop dimensions after the mobile bar grows. +- Use existing Radix/shadcn surfaces and transitions. Add no animation dependency or broad sidebar restyle. + +## Tests + +- **What:** canonical creation, lowest-available naming, independent nested data, append/activate semantics, complete settings payload, draft/limit no-ops, queued-write rollback, and store isolation. + **Files:** `apps/web/lib/state/slices/ui/sidebar-view-actions.test.ts` and `apps/web/lib/state/slices/ui/ui-slice.test.ts` + **How:** RED/GREEN Zustand unit tests around `createSidebarView()` with mocked `updateUserSettings`. +- **What:** external rename request, automatic-name prefill, focus/accessibility, Enter-save, Escape/Cancel retention, stale-request cleanup, and manual-rename regression. + **Files:** `apps/web/components/task/sidebar-filter/view-manager.test.tsx` and a focused test beside `use-sidebar-view-popover.ts`. + **How:** Testing Library component/hook tests with mocked store actions. +- **What:** separated desktop menu item, one create per selection, event-driven popover handoff, draft/limit reasons, and a fixed mobile action. + **Files:** `apps/web/e2e/tests/task/sidebar-filter.spec.ts`, `apps/web/e2e/tests/task/mobile-sidebar-views.spec.ts`, and existing `apps/web/components/app-sidebar/sections/tasks-section.test.tsx` + **How:** production-build Playwright covers real Radix focus/layering and responsive layout; the existing section test guards the picker mount. + +## E2E Tests + +- **Scenario:** create from desktop with no prior edit, observe immediate active canonical defaults and focused prefilled rename, then rename and reload. + **File:** `apps/web/e2e/tests/task/sidebar-filter.spec.ts` + **What to verify:** selection before rename, no clone of active state, persisted renamed view, and popover remaining open. +- **Scenario:** cancel/close rename, create repeatedly, and attempt creation with a dirty draft or 50 views. + **File:** `apps/web/e2e/tests/task/sidebar-filter.spec.ts` + **What to verify:** automatic-name retention/sequence and disabled reasons without draft loss. +- **Scenario:** create through the mobile task-switcher sheet, optionally rename/customize, reopen after reload, and inspect narrow layout. + **File:** `apps/web/e2e/tests/task/mobile-sidebar-views.spec.ts` + **What to verify:** touch-reachable fixed action, persistence, chip swipe behavior, minimum hit area, and no document-level horizontal overflow. +- Extend `apps/web/e2e/pages/sidebar-filter-popover.ts` with direct-create, automatic-name, and rename helpers; scope mobile locators to the sheet because the hidden desktop sidebar remains mounted. + +## Implementation Waves + +Wave 1: + +- [x] [task-01-instant-create-state](task-01-instant-create-state.md) — done + +Wave 2: + +- [x] [task-02-sidebar-new-view-ui](task-02-sidebar-new-view-ui.md) — done; depends on task 01 + +Wave 3: + +- [x] [task-03-sidebar-view-e2e](task-03-sidebar-view-e2e.md) — done; depends on task 02 + +Frontend files overlap and the E2E surface requires integrated behavior, so waves remain sequential. + +## Verification + +```bash +cd apps +pnpm --filter @kandev/web test -- \ + lib/state/slices/ui/sidebar-view-actions.test.ts \ + components/task/sidebar-filter/use-sidebar-view-popover.test.tsx \ + components/task/sidebar-filter/view-manager.test.tsx + +cd apps/web +pnpm build +pnpm e2e:run --host --no-build --project chromium -- tests/task/sidebar-filter.spec.ts +pnpm e2e:run --host --no-build --project mobile-chrome -- tests/task/mobile-sidebar-views.spec.ts + +cd ../.. +make fmt +make typecheck test lint +``` + +### Result + +- Production build passed. +- Focused desktop Playwright passed 18/18; focused mobile Playwright passed 5/5. +- Web unit suite passed 5,113 tests across 632 files; backend retry, script tests, typecheck, and all linters passed. +- The repository-wide CLI suite retains an unrelated baseline failure: `.github/workflows/notify-docs.yml` pins pnpm `10.29.3`, while `apps/package.json` pins `9.15.9`. Neither file is changed by this plan. + +## Risks and considerations + +- The single sidebar draft slot makes creation while dirty destructive; both controller and store action must guard it. +- Never reuse nested references from `DEFAULT_VIEW` when creating a view. +- Create and immediate rename enqueue two ordered full-array settings writes; reuse the existing queue rather than adding timing delays. +- A create rollback can occur while rename UI is focused; the controller must clear stale intent when the created active ID disappears. +- Frontend limit metadata can drift from backend `maxSidebarViews`; backend stays authoritative and failure rollback remains required. +- Radix dropdown-to-popover focus transfer must be event-driven, not timeout-based. +- Hidden desktop and visible mobile controls share test IDs; mobile Playwright locators must stay sheet-scoped. +- Larger touch targets must not overlap or break `TouchSensor` swipe/hold behavior. diff --git a/docs/plans/sidebar-view-creation/task-01-instant-create-state.md b/docs/plans/sidebar-view-creation/task-01-instant-create-state.md new file mode 100644 index 0000000000..89d324abfd --- /dev/null +++ b/docs/plans/sidebar-view-creation/task-01-instant-create-state.md @@ -0,0 +1,51 @@ +--- +id: "01-instant-create-state" +title: "Instant-create sidebar view state" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/ui/sidebar-view-creation.md" +--- + +# Task 01: Instant-Create Sidebar View State + +Implement the dedicated optimistic state action through RED/GREEN tests before UI wiring. + +## Acceptance + +- `createSidebarView()` appends and activates an independent canonical-default view using the lowest available exact automatic name. +- The action sends the complete saved-view/active-view/draft settings payload and reuses existing retry, rollback, and sync-error behavior. +- An active draft or 50 saved views makes the action a no-op; existing `Save as…` and duplicate semantics do not change. + +## Verification + +```bash +cd apps +pnpm --filter @kandev/web test -- lib/state/slices/ui/sidebar-view-actions.test.ts +cd web +pnpm run typecheck +``` + +## Files likely touched + +- `apps/web/lib/state/slices/ui/sidebar-view-builtins.ts` +- `apps/web/lib/state/slices/ui/sidebar-view-actions.ts` +- `apps/web/lib/state/slices/ui/types.ts` +- `apps/web/lib/state/store.ts` +- `apps/web/lib/state/slices/ui/sidebar-view-actions.test.ts` + +## Dependencies + +None. + +## Inputs + +- Spec: canonical defaults, naming, limit, persistence, and failed-create scenarios. +- Plan: Frontend > Instant-create state contract; Tests. +- Existing patterns: `makeId`, `mutateViews`, `snapshotSidebar`, and `toSidebarSettingsPayload` in `sidebar-view-actions.ts`; `DEFAULT_VIEW` in `sidebar-view-builtins.ts`; backend `maxSidebarViews` in `apps/backend/internal/user/service/service.go`. +- ADR 0041: backend user settings remain the only durable source. + +## Output contract + +Report state behavior, files changed, focused test/typecheck results, blockers, and residual risks. Set this task to `done` and update its checkbox in `plan.md` only after acceptance and verification pass. diff --git a/docs/plans/sidebar-view-creation/task-02-sidebar-new-view-ui.md b/docs/plans/sidebar-view-creation/task-02-sidebar-new-view-ui.md new file mode 100644 index 0000000000..ad0b909513 --- /dev/null +++ b/docs/plans/sidebar-view-creation/task-02-sidebar-new-view-ui.md @@ -0,0 +1,59 @@ +--- +id: "02-sidebar-new-view-ui" +title: "Sidebar new-view UI" +status: done +wave: 2 +depends_on: ["01-instant-create-state"] +plan: "plan.md" +spec: "../../specs/ui/sidebar-view-creation.md" +--- + +# Task 02: Sidebar New-View UI + +Connect one shared create/rename flow to the desktop picker and mobile task-switcher sheet. + +## Acceptance + +- Desktop exposes a separated `New view` menu item; mobile exposes a fixed, labeled, non-hover action with a non-overlapping hit area of at least 40×40 px. +- Creation opens the viewport-safe filter popover with the automatic name focused for rename; save keeps the popover open, while Escape/Cancel/close keeps the created view. +- Dirty-draft and 50-view states disable creation with clear reasons; stale rename requests clear after close or optimistic rollback, and existing manual Rename/`Save as…` behavior still works. + +## Verification + +```bash +cd apps +pnpm --filter @kandev/web test -- \ + components/app-sidebar/sections/tasks-section.test.tsx \ + components/task/sidebar-filter/use-sidebar-view-popover.test.tsx \ + components/task/sidebar-filter/view-manager.test.tsx +cd web +pnpm run typecheck +``` + +## Files likely touched + +- `apps/web/components/task/sidebar-filter/use-sidebar-view-popover.ts` +- `apps/web/components/task/sidebar-filter/use-sidebar-view-popover.test.tsx` +- `apps/web/components/task/sidebar-filter/sidebar-filter-popover.tsx` +- `apps/web/components/task/sidebar-filter/view-manager.tsx` +- `apps/web/components/task/sidebar-filter/view-manager.test.tsx` +- `apps/web/components/app-sidebar/sections/tasks-view-picker.tsx` +- `apps/web/components/app-sidebar/sections/tasks-section.test.tsx` +- `apps/web/components/task/sidebar-filter/sidebar-filter-bar.tsx` +- `apps/web/components/task/sidebar-filter/sidebar-view-chips.tsx` + +## Dependencies + +Task 01 (`createSidebarView()` and shared limit/default helpers). + +## Inputs + +- Spec: desktop/mobile entry points, optional rename, blocked states, and narrow-viewport scenarios. +- Plan: Shared creation and rename flow; Desktop and mobile entry points; Tests. +- Existing patterns: `WorkspacePickerContent` in `app-sidebar-workspace-picker.tsx`, Radix mocks in its test, `ViewHeaderRow` in `view-manager.tsx`, and the current mobile `SidebarFilterBar` mount in `session-task-switcher-sheet.tsx`. +- Mobile parity: keep actions reachable without hover or horizontal page scrolling; preserve chip swipe and touch-drag separation. +- Interface polish: use existing hierarchy/transitions, explicit `New view` wording, and minimum non-overlapping hit areas; add no decorative motion. + +## Output contract + +Report desktop/mobile behavior, accessibility/focus handling, files changed, focused tests/typecheck run, blockers, and residual risks. Set this task to `done` and update its checkbox in `plan.md` only after acceptance and verification pass. diff --git a/docs/plans/sidebar-view-creation/task-03-sidebar-view-e2e.md b/docs/plans/sidebar-view-creation/task-03-sidebar-view-e2e.md new file mode 100644 index 0000000000..5e705a3857 --- /dev/null +++ b/docs/plans/sidebar-view-creation/task-03-sidebar-view-e2e.md @@ -0,0 +1,64 @@ +--- +id: "03-sidebar-view-e2e" +title: "Sidebar view creation E2E" +status: done +wave: 3 +depends_on: ["02-sidebar-new-view-ui"] +plan: "plan.md" +spec: "../../specs/ui/sidebar-view-creation.md" +--- + +# Task 03: Sidebar View Creation E2E + +Prove the integrated direct-create journey against the production Vite build on desktop and mobile, then run repo verification. + +## Acceptance + +- Desktop Playwright proves immediate canonical creation, focused optional rename, rename/cancel persistence, automatic naming, dirty/limit guards, and reload survival. +- Mobile Playwright proves the fixed touch action creates and persists a view without hover, overlapping hit areas, broken chip scrolling, or horizontal page overflow. +- Focused E2E plus `make fmt`, repo typecheck, and lint complete successfully; feature-relevant tests pass, any unrelated baseline failure is recorded, and stale sidebar-view test comments are corrected. + +## Verification + +```bash +cd apps/web +pnpm build +pnpm e2e:run --host --no-build --project chromium -- tests/task/sidebar-filter.spec.ts +pnpm e2e:run --host --no-build --project mobile-chrome -- tests/task/mobile-sidebar-views.spec.ts + +cd ../.. +make fmt +make typecheck test lint +``` + +## Verification result + +- Production build passed. +- Focused desktop Playwright passed 18/18; focused mobile Playwright passed 5/5. +- Web unit suite passed 5,113 tests across 632 files; backend retry, script tests, typecheck, and all linters passed. +- The repository-wide CLI suite retains an unrelated baseline failure: `.github/workflows/notify-docs.yml` pins pnpm `10.29.3`, while `apps/package.json` pins `9.15.9`. Neither file is changed by this plan. + +## Files likely touched + +- `apps/web/e2e/pages/sidebar-filter-popover.ts` +- `apps/web/e2e/tests/task/sidebar-filter.spec.ts` +- `apps/web/e2e/tests/task/mobile-sidebar-views.spec.ts` +- `docs/specs/ui/sidebar-view-creation.md` +- `docs/plans/sidebar-view-creation/plan.md` +- `docs/plans/sidebar-view-creation/task-03-sidebar-view-e2e.md` + +## Dependencies + +Task 02 and its completed desktop/mobile UI. + +## Inputs + +- Spec: all scenarios, especially canonical defaults, optional rename, blocked states, failure-safe persistence, and mobile parity. +- Plan: E2E Tests; Verification; Risks and considerations. +- Existing page object: `SidebarFilterPopoverPage` in `apps/web/e2e/pages/sidebar-filter-popover.ts`. +- Existing journeys: saved-view CRUD in `sidebar-filter.spec.ts` and mobile sheet helpers in `mobile-sidebar-views.spec.ts`. +- Scope mobile selectors to the open sheet because a hidden desktop AppSidebar is mounted concurrently. + +## Output contract + +Report desktop/mobile journeys exercised, exact commands/results, artifacts for failures, files changed, blockers, and residual risks. Mark this task and plan entry `done`; move spec/plan status to `shipped`/`completed` only after integrated verification passes. diff --git a/docs/specs/INDEX.md b/docs/specs/INDEX.md index 42692e952a..0824a9ef55 100644 --- a/docs/specs/INDEX.md +++ b/docs/specs/INDEX.md @@ -108,6 +108,7 @@ Subscription quota tracking and per-agent cheap-model profile routing. | [acp-shell-command-output](ui/acp-shell-command-output.md) | shipped | | [acp-model-configuration-summary](ui/acp-model-configuration-summary.md) | shipped | | [review-file-status](ui/review-file-status.md) | building | +| [sidebar-view-creation](ui/sidebar-view-creation.md) | shipped | | [slash-command-composer](ui/slash-command-composer.md) | shipped | ## system-page/ — operational diagnostics & maintenance UI diff --git a/docs/specs/ui/sidebar-view-creation.md b/docs/specs/ui/sidebar-view-creation.md new file mode 100644 index 0000000000..bac26f7291 --- /dev/null +++ b/docs/specs/ui/sidebar-view-creation.md @@ -0,0 +1,53 @@ +--- +status: shipped +created: 2026-07-17 +owner: kandev +--- + +# Direct Sidebar View Creation + +## Why + +Creating a sidebar view currently requires changing an existing view before `Save as…` becomes available. This makes a basic organizational action feel hidden and risks teaching users that editing an existing view is part of creating a new one. + +## What + +- The desktop Tasks view picker ends with a separated `New view` action. The label has no ellipsis because selection creates the view immediately rather than opening a pre-creation dialog. +- The mobile task-switcher sheet exposes a fixed `New view` action beside the horizontally scrolling saved-view chips and filter control. It remains reachable without hover or horizontal page scrolling and has at least a 40×40 px hit area. +- Selecting `New view` immediately appends and activates a saved view with canonical defaults: no filters, `state` ascending sort, `repository` grouping, and no collapsed groups. +- Canonical defaults do not inherit from the active view, an active draft, or a user-modified `All tasks` view. +- The automatic name is the lowest exact name not already present: `New view`, then `New view 2`, `New view 3`, and so on. Existing manually duplicated names remain valid. +- After creation, the existing filter popover opens with its rename input focused and prefilled with the automatic name. Saving a rename keeps the popover open so the user can configure filters, sort, and grouping next. +- Rename is optional. Canceling rename, pressing Escape, or closing the popover keeps the already-created view under its automatic name. +- `Save as…` remains available for deriving a view from unsaved changes; direct creation does not replace or change that workflow. +- When the active view has unsaved changes, direct creation is disabled and explains that the user must save or discard those changes first. No draft is silently cleared. +- A user may save at most 50 sidebar views. At 50 views, direct creation is disabled with a clear limit reason; backend validation remains authoritative. +- The filter popover fits narrow viewports without document-level horizontal overflow. + +## Persistence and failure behavior + +- Creation persists the new view and active-view selection immediately through existing backend-owned user settings. A successful create survives reload and is available on the user's other clients. +- A failed create restores the previous saved views, active view, and draft, then surfaces the existing sidebar-view sync error. It never leaves a selected view that is absent from persisted settings. +- Each new view owns independent filter, sort, and collapsed-group data; later changes to it cannot mutate the canonical default or another view by shared reference. + +See [ADR 0041](../../decisions/0041-backend-owned-portable-user-settings.md) for backend ownership of portable user settings. + +## Scenarios + +- **GIVEN** fewer than 50 saved views and no unsaved draft, **WHEN** the user selects `New view`, **THEN** a canonical default view is appended, activated immediately, and offered for focused rename. +- **GIVEN** the active saved view has custom filters, sort, grouping, or collapsed groups, **WHEN** the user creates a new view, **THEN** the new view still uses the canonical defaults rather than cloning the active state. +- **GIVEN** saved views named `New view` and `New view 3`, **WHEN** the user creates a new view, **THEN** its automatic name is `New view 2`. +- **GIVEN** a newly created view is in rename mode, **WHEN** the user cancels or closes the popover, **THEN** the view remains saved and active under its automatic name. +- **GIVEN** a newly created view is in rename mode, **WHEN** the user saves a nonblank name, **THEN** the name persists and the filter popover remains open for customization. +- **GIVEN** an unsaved sidebar-view draft, **WHEN** the user inspects direct creation on desktop or mobile, **THEN** `New view` is disabled and communicates that the draft must be saved or discarded first. +- **GIVEN** 50 saved sidebar views, **WHEN** the user inspects direct creation on desktop or mobile, **THEN** `New view` is disabled and communicates the saved-view limit. +- **GIVEN** the create settings write fails, **WHEN** retry handling finishes, **THEN** the prior sidebar-view state is restored and the sidebar-view error is shown. +- **GIVEN** a narrow mobile viewport, **WHEN** the task-switcher sheet opens, **THEN** `New view` is touch-reachable, the chip row still scrolls, and neither the bar nor popover causes horizontal page overflow. + +## Out of scope + +- Changing `Save as…`, duplicate-view, rename, delete, or saved-view ordering semantics. +- A pre-creation naming dialog, cancel-to-delete behavior, or delayed persistence until rename. +- Cloning the active view from the direct-create action. +- Backend schemas, new endpoints, or a new cross-tab conflict model for full-array user-settings writes. +- Broad sidebar visual redesign or animation dependencies.