From 467327835157581c1033ae8c4c16e7bbe7d4f021 Mon Sep 17 00:00:00 2001 From: Mario Meyer Date: Mon, 1 Jun 2026 09:51:23 -0400 Subject: [PATCH 1/3] =?UTF-8?q?feat(ui):=20ProjectTaskPicker=20combined=20?= =?UTF-8?q?picker=20=E2=80=94=20fixes=20missing=20task=20select=20in=20pop?= =?UTF-8?q?over?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Single combobox replacing the ProjectPicker+TaskPicker pair wherever both are used together. Tree layout: "(No project)" sentinel at top, project headers rendered bold with a chevron, tasks indented underneath. Project rows are selectable (selecting one yields project-only, task_id=null). Smart search: typing filters both projects and tasks. Tasks matching the query auto-expand their parent project; if the parent's name doesn't match the parent shows dimmed (still clickable for project-only). All projects collapsed by default; opening the dropdown with a current task selected auto-expands its parent so the value is visible. Keyboard: ↑↓ traverse visible rows, Enter selects, Esc closes, Right expands a collapsed project, Left collapses an expanded one (or jumps from a task back to its parent). Why: the popover had no TaskPicker at all (regression from 6c — the parallel task-assignment work added TaskPicker to TimerCard + EditEntryDialog but missed the popover route). Adding a second combobox there would crowd the layout; the combined picker handles it naturally and brings the same unified shape to TimerCard and the EditEntryDialog. The simpler ProjectPicker stays in use for Settings (default project) and CalendarSection (calendar default project) — those genuinely want project-only and no tasks. Tests: 11 new unit tests covering placeholder/value rendering, click + keyboard selection, auto-expand on selected-task, smart search filter, done-task exclusion, and the empty-state message. Existing tests for TimerCard, EditEntryDialog, EntryRow, Today, and Popover updated to match the new aria-label and combined layout. --- ui/src/components/EditEntryDialog.tsx | 48 +- ui/src/components/TimerCard.tsx | 112 ++--- ui/src/components/ui/ProjectTaskPicker.tsx | 429 ++++++++++++++++++ ui/src/routes/Popover.tsx | 24 +- .../test/components/EditEntryDialog.test.tsx | 32 +- ui/src/test/components/EntryRow.test.tsx | 8 +- .../components/ProjectTaskPicker.test.tsx | 230 ++++++++++ ui/src/test/components/TimerCard.test.tsx | 39 +- ui/src/test/routes/Popover.test.tsx | 1 + ui/src/test/routes/Today.test.tsx | 1 + 10 files changed, 764 insertions(+), 160 deletions(-) create mode 100644 ui/src/components/ui/ProjectTaskPicker.tsx create mode 100644 ui/src/test/components/ProjectTaskPicker.test.tsx diff --git a/ui/src/components/EditEntryDialog.tsx b/ui/src/components/EditEntryDialog.tsx index f1409e2..1fecd75 100644 --- a/ui/src/components/EditEntryDialog.tsx +++ b/ui/src/components/EditEntryDialog.tsx @@ -3,8 +3,7 @@ import { api } from "~/api"; import { fromLocalHHMM, toLocalHHMM } from "~/lib/entryFormat"; import type { Entry } from "~/types"; import Button from "./ui/Button"; -import ProjectPicker from "./ui/ProjectPicker"; -import TaskPicker from "./ui/TaskPicker"; +import ProjectTaskPicker from "./ui/ProjectTaskPicker"; import Toggle from "./ui/Toggle"; export default function EditEntryDialog(props: { @@ -30,14 +29,11 @@ export default function EditEntryDialog(props: { const [projects] = createResource(() => api.listProjects(), { initialValue: [], }); - // Tasks for the currently-selected project. Re-fetches when projectId - // flips. An empty projectId resolves to an empty list and the TaskPicker - // stays disabled — no point hitting the IPC. - const [tasks] = createResource( - () => projectId(), - async (pid) => (pid ? await api.listTasks(pid) : []), - { initialValue: [] }, - ); + // All tasks across all projects, eager-loaded once. The combined picker + // groups by project_id itself. + const [tasks] = createResource(() => api.listTasks(null), { + initialValue: [], + }); const isCompleted = createMemo(() => Boolean(props.entry.end_at)); @@ -116,36 +112,18 @@ export default function EditEntryDialog(props: {
- { - // Tasks scope to projects — changing project must discard - // the staged task selection so Save doesn't send a - // task_id that doesn't belong to the new project. - setTaskId(null); - setProjectId(id); + { + setProjectId(v.projectId); + setTaskId(v.taskId); }} projects={projects() ?? []} - placeholder="No project" - size="sm" - /> -
-
- -
- -
-
diff --git a/ui/src/components/TimerCard.tsx b/ui/src/components/TimerCard.tsx index aca7d35..d3dfea7 100644 --- a/ui/src/components/TimerCard.tsx +++ b/ui/src/components/TimerCard.tsx @@ -3,17 +3,16 @@ import { api } from "~/api"; import Duration from "./Duration"; import StartAtPicker, { type StartAtValue } from "./StartAtPicker"; import Button from "./ui/Button"; -import ProjectPicker from "./ui/ProjectPicker"; +import ProjectTaskPicker from "./ui/ProjectTaskPicker"; import SectionLabel from "./ui/SectionLabel"; import StatusDot from "./ui/StatusDot"; -import TaskPicker from "./ui/TaskPicker"; import Toggle from "./ui/Toggle"; import { useTimerStore } from "~/stores/timer"; export default function TimerCard() { const timer = useTimerStore(); const [description, setDescription] = createSignal(""); - const [projectId, setProjectId] = createSignal(""); + const [projectId, setProjectId] = createSignal(null); const [taskId, setTaskId] = createSignal(null); const [billable, setBillable] = createSignal(false); const [startAt, setStartAt] = createSignal(null); @@ -22,24 +21,30 @@ export default function TimerCard() { }); const projectList = () => projects() ?? []; - // Tasks for the *start form's* selected project. Re-fetched whenever the - // project changes; an empty project resolves to an empty list and the - // TaskPicker stays disabled (no point hitting the IPC). - const [startFormTasks] = createResource( - () => projectId() || null, - async (pid) => (pid ? await api.listTasks(pid) : []), - { initialValue: [] }, - ); + // All tasks across all projects, eager-loaded once. The combined picker + // groups by project_id itself, so we don't have to refetch per selection. + const [tasks] = createResource(() => api.listTasks(null), { + initialValue: [], + }); - // Tasks for the *running entry's* project. Same shape, different source — - // the running entry's project_id might differ from the start form's - // (e.g. when the user is editing the live entry's project inline). - const runningProjectId = () => timer.running()?.project_id ?? null; - const [runningTasks] = createResource( - runningProjectId, - async (pid) => (pid ? await api.listTasks(pid) : []), - { initialValue: [] }, - ); + /// Apply a project+task change to a running entry. Always clears the + /// task first so a queued patch can't carry a task_id from the old + /// project, then sets the new project, then sets the new task. + async function applyLiveChange(args: { + localUuid: string; + hadTask: boolean; + nextProjectId: string | null; + nextTaskId: string | null; + }) { + if (args.hadTask) { + await api.setEntryTask(args.localUuid, null); + } + await api.setEntryProject(args.localUuid, args.nextProjectId); + if (args.nextTaskId) { + await api.setEntryTask(args.localUuid, args.nextTaskId); + } + await timer.refresh(); + } return (
@@ -55,7 +60,7 @@ export default function TimerCard() { timer .start( d, - projectId() || undefined, + projectId() ?? undefined, taskId() ?? undefined, billable(), startAt() ?? undefined, @@ -65,6 +70,7 @@ export default function TimerCard() { setBillable(false); setStartAt(null); setTaskId(null); + setProjectId(null); }); }} > @@ -78,26 +84,15 @@ export default function TimerCard() {
- { - // Tasks scope to projects — changing project must - // discard the old task selection or we'd send a - // task_id that doesn't belong to the new project. - setTaskId(null); - setProjectId(id ?? ""); + { + setProjectId(v.projectId); + setTaskId(v.taskId); }} projects={projectList()} - placeholder="No project" - /> -
-
-
@@ -126,33 +121,22 @@ export default function TimerCard() {
- { - // Switching project on a live entry: clear the task - // first so the queued update doesn't carry a stale - // task_id from the old project. - if (t().task_id) { - await api.setEntryTask(t().local_uuid, null); - } - await api.setEntryProject(t().local_uuid, id); - await timer.refresh(); + + applyLiveChange({ + localUuid: t().local_uuid, + hadTask: Boolean(t().task_id), + nextProjectId: v.projectId, + nextTaskId: v.taskId, + }) + } projects={projectList()} - placeholder="No project" - size="sm" - /> -
-
- { - await api.setEntryTask(t().local_uuid, id); - await timer.refresh(); - }} - tasks={runningTasks() ?? []} - projectSelected={Boolean(t().project_id)} - placeholder="No task" + tasks={tasks() ?? []} + placeholder="Project / task" size="sm" />
diff --git a/ui/src/components/ui/ProjectTaskPicker.tsx b/ui/src/components/ui/ProjectTaskPicker.tsx new file mode 100644 index 0000000..99cffc6 --- /dev/null +++ b/ui/src/components/ui/ProjectTaskPicker.tsx @@ -0,0 +1,429 @@ +import { Popover } from "@kobalte/core/popover"; +import { + For, + Show, + createEffect, + createMemo, + createSignal, + on, +} from "solid-js"; +import { buildPickerOptions, type PickerOption } from "~/lib/projectPickerSort"; +import type { Project, Task } from "~/types"; + +/** + * Single combined picker for project + task selection. Replaces the + * ProjectPicker + TaskPicker pairing wherever both are needed (popover + * start form, TimerCard start form, TimerCard live entry, EditEntryDialog). + * + * The simpler ProjectPicker stays in use for Settings (default project) and + * CalendarSection (calendar default project) — they only need a project, + * no tasks. + * + * Layout: tree. "(No project)" sentinel at top, projects rendered as bold + * rows with a chevron, tasks indented underneath. Project rows are + * selectable (selecting one yields project-only, task_id=null). + * + * Search: smart filter. Typing matches projects AND tasks. Matching tasks + * auto-expand their parent project; if the parent's name doesn't match the + * query the parent is shown dimmed (still clickable for project-only). + * + * Default expansion: all collapsed. The project containing the currently + * selected task auto-expands when the dropdown opens. Any query + * auto-expands all projects with matches. + * + * Keyboard: ↑↓ traverses visible rows; Enter selects; Esc closes; Right + * expands a collapsed project; Left collapses an expanded project (or + * jumps from a task back to its parent). + */ + +export type ProjectTaskValue = { + projectId: string | null; + taskId: string | null; +}; + +type Row = + | { kind: "none"; key: string } + | { kind: "project"; key: string; project: PickerOption; dim: boolean } + | { kind: "task"; key: string; project: PickerOption; task: Task }; + +interface Props { + value: ProjectTaskValue; + onChange: (v: ProjectTaskValue) => void | Promise; + projects: Project[]; + /** All tasks across all projects. The component groups by task.project_id. */ + tasks: Task[]; + placeholder?: string; + size?: "sm" | "md"; + disabled?: boolean; +} + +export default function ProjectTaskPicker(props: Props) { + const [open, setOpen] = createSignal(false); + const [query, setQuery] = createSignal(""); + const [expanded, setExpanded] = createSignal>(new Set()); + const [highlightIdx, setHighlightIdx] = createSignal(0); + + // Group tasks by project_id and produce a stable accessor. + const tasksByProject = createMemo(() => { + const m = new Map(); + for (const t of props.tasks) { + if (t.done) continue; + const list = m.get(t.project_id); + if (list) list.push(t); + else m.set(t.project_id, [t]); + } + for (const list of m.values()) { + list.sort((a, b) => a.name.localeCompare(b.name)); + } + return m; + }); + + const projectOptions = createMemo(() => buildPickerOptions(props.projects)); + + // Visible rows: respects search query AND expansion state. + const rows = createMemo(() => { + const q = query().trim().toLowerCase(); + const exp = expanded(); + const out: Row[] = [{ kind: "none", key: "__none__" }]; + + for (const p of projectOptions()) { + const projMatch = !q || p.name.toLowerCase().includes(q); + const projTasks = tasksByProject().get(p.id) ?? []; + const matchingTasks = q + ? projTasks.filter((t) => t.name.toLowerCase().includes(q)) + : projTasks; + + if (q) { + // Search mode: include project if its name matches OR it has any + // matching task. When the project's own name doesn't match, dim + // it to signal "the children matched, but you can still pick the + // project itself if that's what you want." + if (!projMatch && matchingTasks.length === 0) continue; + out.push({ + kind: "project", + key: `p:${p.id}`, + project: p, + dim: !projMatch, + }); + for (const t of matchingTasks) { + out.push({ + kind: "task", + key: `t:${t.solidtime_id}`, + project: p, + task: t, + }); + } + } else { + // Normal mode: project always visible; tasks only if expanded. + out.push({ kind: "project", key: `p:${p.id}`, project: p, dim: false }); + if (exp.has(p.id)) { + for (const t of projTasks) { + out.push({ + kind: "task", + key: `t:${t.solidtime_id}`, + project: p, + task: t, + }); + } + } + } + } + return out; + }); + + // When the dropdown opens, auto-expand the project that owns the + // currently selected task so the user can see the current value + // without first having to expand by hand. + createEffect( + on(open, (isOpen) => { + if (!isOpen) return; + const v = props.value; + if (v.taskId && v.projectId) { + setExpanded((s) => { + if (s.has(v.projectId!)) return s; + const ns = new Set(s); + ns.add(v.projectId!); + return ns; + }); + } + // Reset query + highlight on every open so the user starts clean. + setQuery(""); + setHighlightIdx(0); + }), + ); + + // Auto-expand any project that has matches whenever the query changes. + // Empty query restores whatever expansion the user had explicitly set; + // we only expand on non-empty queries. + createEffect( + on(query, (q) => { + const trimmed = q.trim().toLowerCase(); + if (!trimmed) return; + const matching = new Set(); + for (const p of projectOptions()) { + const projMatch = p.name.toLowerCase().includes(trimmed); + const projTasks = tasksByProject().get(p.id) ?? []; + const hasMatchingTask = projTasks.some((t) => + t.name.toLowerCase().includes(trimmed), + ); + if (projMatch || hasMatchingTask) matching.add(p.id); + } + setExpanded((s) => { + // Union with whatever the user had open so we don't collapse + // them mid-search. + const ns = new Set(s); + for (const id of matching) ns.add(id); + return ns; + }); + setHighlightIdx(0); + }), + ); + + // Resolve the displayed label for the current value. + const valueLabel = createMemo(() => { + const v = props.value; + if (v.projectId == null) return null; + const p = projectOptions().find((o) => o.id === v.projectId); + if (!p) return null; + if (v.taskId == null) return p.name; + const t = props.tasks.find((x) => x.solidtime_id === v.taskId); + return t ? `${p.name} / ${t.name}` : p.name; + }); + + function commitRow(row: Row) { + switch (row.kind) { + case "none": + void props.onChange({ projectId: null, taskId: null }); + break; + case "project": + void props.onChange({ projectId: row.project.id, taskId: null }); + break; + case "task": + void props.onChange({ + projectId: row.project.id, + taskId: row.task.solidtime_id, + }); + break; + } + setOpen(false); + } + + function toggleExpand(projectId: string) { + setExpanded((s) => { + const ns = new Set(s); + if (ns.has(projectId)) ns.delete(projectId); + else ns.add(projectId); + return ns; + }); + } + + function onKeyDown(e: KeyboardEvent) { + const list = rows(); + const idx = highlightIdx(); + switch (e.key) { + case "ArrowDown": { + e.preventDefault(); + if (list.length > 0) { + setHighlightIdx(Math.min(list.length - 1, idx + 1)); + } + break; + } + case "ArrowUp": { + e.preventDefault(); + setHighlightIdx(Math.max(0, idx - 1)); + break; + } + case "ArrowRight": { + const row = list[idx]; + if (row?.kind === "project" && !expanded().has(row.project.id)) { + e.preventDefault(); + toggleExpand(row.project.id); + } + break; + } + case "ArrowLeft": { + const row = list[idx]; + if (row?.kind === "project" && expanded().has(row.project.id)) { + e.preventDefault(); + toggleExpand(row.project.id); + } else if (row?.kind === "task") { + e.preventDefault(); + const parentIdx = list.findIndex( + (r) => r.kind === "project" && r.project.id === row.project.id, + ); + if (parentIdx >= 0) setHighlightIdx(parentIdx); + } + break; + } + case "Enter": { + e.preventDefault(); + const row = list[idx]; + if (row) commitRow(row); + break; + } + case "Escape": { + e.preventDefault(); + setOpen(false); + break; + } + } + } + + const sizeClass = () => + props.size === "sm" ? "px-2.5 py-1.5 text-[12px]" : "px-3 py-1.5 text-sm"; + + return ( + + + + + {props.placeholder ?? "Select project or task…"} + + } + > + {(label) => {label()}} + + + + ▾ + + + + e.preventDefault()} + > + setQuery(e.currentTarget.value)} + onKeyDown={onKeyDown} + /> +
    + + {(row, i) => ( + commitRow(row)} + onToggle={() => { + if (row.kind === "project") toggleExpand(row.project.id); + }} + onHover={() => setHighlightIdx(i())} + /> + )} + + +
  • + No projects or tasks match "{query()}" +
    • +     +
+ + + + ); +} + +function RowItem(props: { + row: Row; + highlighted: boolean; + isExpanded: boolean; + onSelect: () => void; + onToggle: () => void; + onHover: () => void; +}) { + const baseClass = () => + `flex cursor-pointer items-center gap-1 rounded px-2 py-1.5 text-sm outline-none ${ + props.highlighted ? "bg-zinc-100 dark:bg-zinc-800" : "" + }`; + + return ( +
  • { + // Clicks on the chevron-zone (data-chevron) only toggle expansion, + // not select. Clicks anywhere else in the row select. + const target = e.target as HTMLElement; + if (target.closest("[data-chevron]")) return; + props.onSelect(); + }} + > + {(() => { + switch (props.row.kind) { + case "none": + return ( + + No project + + ); + case "project": + return ( + <> + + + {props.row.project.name} + + + + {props.row.project.clientName} + + + + ); + case "task": + return ( + <> + + + ↳ {props.row.task.name} + + + ); + } + })()} +
    • + ); +} diff --git a/ui/src/routes/Popover.tsx b/ui/src/routes/Popover.tsx index 8114516..43a1622 100644 --- a/ui/src/routes/Popover.tsx +++ b/ui/src/routes/Popover.tsx @@ -5,7 +5,7 @@ import { api } from "~/api"; import Duration from "~/components/Duration"; import StartAtPicker, { type StartAtValue } from "~/components/StartAtPicker"; import Button from "~/components/ui/Button"; -import ProjectPicker from "~/components/ui/ProjectPicker"; +import ProjectTaskPicker from "~/components/ui/ProjectTaskPicker"; import SectionLabel from "~/components/ui/SectionLabel"; import StatusDot from "~/components/ui/StatusDot"; import Toggle from "~/components/ui/Toggle"; @@ -17,7 +17,8 @@ import { useTimerStore } from "~/stores/timer"; export default function Popover() { const timer = useTimerStore(); const [description, setDescription] = createSignal(""); - const [projectId, setProjectId] = createSignal(""); + const [projectId, setProjectId] = createSignal(null); + const [taskId, setTaskId] = createSignal(null); const [billable, setBillable] = createSignal(false); const [startAt, setStartAt] = createSignal(null); const [entries, { refetch: refetchEntries }] = createResource( @@ -27,6 +28,11 @@ export default function Popover() { const [projects] = createResource(() => api.listProjects(), { initialValue: [], }); + // All tasks across projects, eager-loaded once. The combined picker + // groups by project_id itself. + const [tasks] = createResource(() => api.listTasks(null), { + initialValue: [], + }); const unlistenEntries = listen("entries:changed", () => refetchEntries()); onCleanup(() => { unlistenEntries.then((fn) => fn()).catch(() => {}); @@ -88,8 +94,8 @@ export default function Popover() { timer .start( d, - projectId() || undefined, - undefined, + projectId() ?? undefined, + taskId() ?? undefined, billable(), startAt() ?? undefined, ) @@ -110,10 +116,14 @@ export default function Popover() {
    - setProjectId(id ?? "")} + { + setProjectId(v.projectId); + setTaskId(v.taskId); + }} projects={projects() ?? []} + tasks={tasks() ?? []} placeholder="No project" size="sm" /> diff --git a/ui/src/test/components/EditEntryDialog.test.tsx b/ui/src/test/components/EditEntryDialog.test.tsx index d78f046..75c6726 100644 --- a/ui/src/test/components/EditEntryDialog.test.tsx +++ b/ui/src/test/components/EditEntryDialog.test.tsx @@ -47,7 +47,7 @@ beforeEach(() => { }); describe("", () => { - it("renders the description, project picker, billable toggle, and time inputs for a completed entry", () => { + it("renders the description, project/task picker, billable toggle, and time inputs for a completed entry", () => { const { getByText, getByLabelText, container } = render(() => ( ", () => { )); expect(getByText("Edit entry")).toBeDefined(); expect(getByText("Description")).toBeDefined(); - expect(getByText("Project")).toBeDefined(); + expect(getByText("Project / task")).toBeDefined(); expect(getByText("Start")).toBeDefined(); expect(getByText("End")).toBeDefined(); - expect(getByLabelText("Open project list")).toBeDefined(); + expect(getByLabelText("Open project or task list")).toBeDefined(); const times = container.querySelectorAll('input[type="time"]'); expect(times.length).toBe(2); }); @@ -178,31 +178,19 @@ describe("", () => { expect(onClose).toHaveBeenCalled(); }); - it("renders a Task field and picker when the entry has a project", async () => { - const { getByText, getByLabelText } = render(() => ( + it("eagerly loads all tasks for the combined project/task picker", async () => { + render(() => ( )); - expect(getByText("Task")).toBeDefined(); - const trigger = getByLabelText("Open task list"); - const taskInput = trigger.parentElement?.querySelector("input") as HTMLInputElement; - expect(taskInput.disabled).toBe(false); - }); - - it("disables the Task picker when the entry has no project", async () => { - const { getByLabelText } = render(() => ( - - )); - const trigger = getByLabelText("Open task list"); - const taskInput = trigger.parentElement?.querySelector("input") as HTMLInputElement; - expect(taskInput.disabled).toBe(true); + await flush(); + // The combined picker fetches all tasks upfront (project_id=null) so + // it can group them under their parents. The previous separate + // TaskPicker fetched per-project on demand. + expect(api.listTasks).toHaveBeenCalledWith(null); }); it("Save without touching the task leaves setEntryTask uncalled", async () => { diff --git a/ui/src/test/components/EntryRow.test.tsx b/ui/src/test/components/EntryRow.test.tsx index 1439a16..f151d70 100644 --- a/ui/src/test/components/EntryRow.test.tsx +++ b/ui/src/test/components/EntryRow.test.tsx @@ -7,8 +7,10 @@ vi.mock("~/api", () => ({ { id: "p-1", name: "Tet", color: null, client_id: null, client_name: null, archived: 0 }, { id: "p-2", name: "Other", color: null, client_id: null, client_name: null, archived: 0 }, ]), + listTasks: vi.fn().mockResolvedValue([]), updateDescription: vi.fn().mockResolvedValue(undefined), setEntryProject: vi.fn().mockResolvedValue(undefined), + setEntryTask: vi.fn().mockResolvedValue(undefined), setEntryBillable: vi.fn().mockResolvedValue(undefined), updateEntryTimes: vi.fn().mockResolvedValue(undefined), deleteEntry: vi.fn().mockResolvedValue(undefined), @@ -118,14 +120,14 @@ describe("", () => { expect(queryByText("Edit entry")).not.toBeNull(); }); - it("dialog shows the ProjectPicker after the row is clicked", async () => { + it("dialog shows the combined project/task picker after the row is clicked", async () => { const { container, queryByLabelText } = render(() => ( )); - expect(queryByLabelText("Open project list")).toBeNull(); + expect(queryByLabelText("Open project or task list")).toBeNull(); fireEvent.click(container.querySelector("button")!); await flush(); - expect(queryByLabelText("Open project list")).not.toBeNull(); + expect(queryByLabelText("Open project or task list")).not.toBeNull(); }); it("renders a Restart button on completed entries", () => { diff --git a/ui/src/test/components/ProjectTaskPicker.test.tsx b/ui/src/test/components/ProjectTaskPicker.test.tsx new file mode 100644 index 0000000..74f5840 --- /dev/null +++ b/ui/src/test/components/ProjectTaskPicker.test.tsx @@ -0,0 +1,230 @@ +import { describe, expect, it, vi } from "vitest"; +import { fireEvent, render, screen } from "@solidjs/testing-library"; + +import ProjectTaskPicker from "~/components/ui/ProjectTaskPicker"; +import type { Project, Task } from "~/types"; + +const proj = (over: Partial = {}): Project => ({ + id: "p-1", + name: "Tet", + color: null, + client_id: null, + client_name: null, + archived: 0, + ...over, +}); + +const task = (over: Partial = {}): Task => ({ + solidtime_id: "t-1", + project_id: "p-1", + name: "Implement picker", + done: false, + ...over, +}); + +const flush = () => new Promise((r) => setTimeout(r, 0)); + +/// Kobalte's Popover.Portal renders outside the render container into +/// document.body, so queries inside the dropdown use `screen` (which +/// targets document.body) rather than the render-scoped helpers. +describe("", () => { + it("renders the trigger with the placeholder when no value is set", () => { + const { getByLabelText, getByText } = render(() => ( + + )); + expect(getByLabelText("Open project or task list")).toBeDefined(); + expect(getByText("Choose…")).toBeDefined(); + }); + + it("renders the project name when only a project is selected", () => { + const { getByText } = render(() => ( + + )); + expect(getByText("Tet")).toBeDefined(); + }); + + it("renders 'Project / Task' when both are selected", () => { + const { getByText } = render(() => ( + + )); + expect(getByText("Tet / Refactor")).toBeDefined(); + }); + + it("opens the dropdown and lists 'No project' + the project rows", async () => { + const { getByLabelText } = render(() => ( + + )); + fireEvent.click(getByLabelText("Open project or task list")); + await flush(); + expect(screen.queryByText("No project")).not.toBeNull(); + expect(screen.queryByText("Alpha")).not.toBeNull(); + expect(screen.queryByText("Beta")).not.toBeNull(); + }); + + it("clicking 'No project' fires onChange with both ids null and closes the dropdown", async () => { + const onChange = vi.fn(); + const { getByLabelText } = render(() => ( + + )); + fireEvent.click(getByLabelText("Open project or task list")); + await flush(); + fireEvent.click(screen.getByText("No project")); + await flush(); + expect(onChange).toHaveBeenCalledWith({ projectId: null, taskId: null }); + expect(screen.queryByText("No project")).toBeNull(); + }); + + it("clicking a project header selects project-only (task_id stays null)", async () => { + const onChange = vi.fn(); + const { getByLabelText } = render(() => ( + + )); + fireEvent.click(getByLabelText("Open project or task list")); + await flush(); + fireEvent.click(screen.getByText("Alpha")); + await flush(); + expect(onChange).toHaveBeenCalledWith({ projectId: "p-1", taskId: null }); + }); + + it("expanding a project reveals its tasks; clicking a task selects project + task", async () => { + const onChange = vi.fn(); + const { getByLabelText } = render(() => ( + + )); + fireEvent.click(getByLabelText("Open project or task list")); + await flush(); + expect(screen.queryByText("↳ First")).toBeNull(); + fireEvent.click(screen.getByLabelText("Expand")); + await flush(); + expect(screen.queryByText("↳ First")).not.toBeNull(); + expect(screen.queryByText("↳ Second")).not.toBeNull(); + fireEvent.click(screen.getByText("↳ Second")); + await flush(); + expect(onChange).toHaveBeenCalledWith({ + projectId: "p-1", + taskId: "t-2", + }); + }); + + it("auto-expands the project owning the currently selected task on open", async () => { + const { getByLabelText } = render(() => ( + + )); + fireEvent.click(getByLabelText("Open project or task list")); + await flush(); + expect(screen.queryByText("↳ First")).not.toBeNull(); + }); + + it("smart search auto-expands projects with matching tasks", async () => { + const { getByLabelText } = render(() => ( + + )); + fireEvent.click(getByLabelText("Open project or task list")); + await flush(); + const search = screen.getByPlaceholderText( + "Search projects + tasks…", + ) as HTMLInputElement; + search.value = "refactor"; + fireEvent.input(search); + await flush(); + expect(screen.queryByText("↳ Refactor picker")).not.toBeNull(); + expect(screen.queryByText("Beta")).toBeNull(); + }); + + it("filters out tasks marked done", async () => { + const { getByLabelText } = render(() => ( + + )); + fireEvent.click(getByLabelText("Open project or task list")); + await flush(); + // Project auto-expands because the parent has projectId in value. + // Expand explicitly in case auto-expand only triggers when taskId is set. + fireEvent.click(screen.getByLabelText("Expand")); + await flush(); + expect(screen.queryByText("↳ Active")).not.toBeNull(); + expect(screen.queryByText("↳ Finished")).toBeNull(); + }); + + it("shows an empty-state message when search has no matches", async () => { + const { getByLabelText } = render(() => ( + + )); + fireEvent.click(getByLabelText("Open project or task list")); + await flush(); + const search = screen.getByPlaceholderText( + "Search projects + tasks…", + ) as HTMLInputElement; + search.value = "nonexistent"; + fireEvent.input(search); + await flush(); + expect(screen.queryByText(/No projects or tasks match/)).not.toBeNull(); + }); +}); diff --git a/ui/src/test/components/TimerCard.test.tsx b/ui/src/test/components/TimerCard.test.tsx index df9e448..feb0e9b 100644 --- a/ui/src/test/components/TimerCard.test.tsx +++ b/ui/src/test/components/TimerCard.test.tsx @@ -67,12 +67,12 @@ beforeEach(() => { }); describe(" — start form (no timer running)", () => { - it("renders the description input, a project picker, and a Start button", () => { + it("renders the description input, the combined project/task picker, and a Start button", () => { const { getByPlaceholderText, getByText, getByLabelText } = render(() => ( )); expect(getByPlaceholderText("What are you working on?")).toBeDefined(); - expect(getByLabelText("Open project list")).toBeDefined(); + expect(getByLabelText("Open project or task list")).toBeDefined(); expect(getByText("Start")).toBeDefined(); }); @@ -110,14 +110,13 @@ describe(" — start form (no timer running)", () => { ); }); - it("renders a TaskPicker disabled until a project is selected", async () => { - const { getByLabelText } = render(() => ); + it("loads all tasks (cross-project) on mount for the combined picker", async () => { + render(() => ); await flushMicrotasks(); - const trigger = getByLabelText("Open task list") as HTMLButtonElement; - expect(trigger).toBeDefined(); - // The Combobox input is the one that goes disabled. - const taskInput = trigger.parentElement?.querySelector("input") as HTMLInputElement; - expect(taskInput.disabled).toBe(true); + // The combined ProjectTaskPicker fetches all tasks upfront (passing + // null to scope to "everything") so the dropdown can render tasks + // grouped under their parent projects without per-expansion fetches. + expect(api.listTasks).toHaveBeenCalledWith(null); }); it("does not call start when the description is blank", async () => { @@ -153,28 +152,10 @@ describe(" — running timer panel", () => { expect(storeMock.stop).toHaveBeenCalledTimes(1); }); - it("running panel shows the ProjectPicker for live project changes", async () => { + it("running panel shows the combined project/task picker for live changes", async () => { setRunning(runningTimer({ description: "x" })); const { getByLabelText } = render(() => ); await flushMicrotasks(); - expect(getByLabelText("Open project list")).toBeDefined(); - }); - - it("running panel exposes a TaskPicker (disabled when no project)", async () => { - setRunning(runningTimer({ description: "x", project_id: null })); - const { getByLabelText } = render(() => ); - await flushMicrotasks(); - const trigger = getByLabelText("Open task list") as HTMLButtonElement; - const taskInput = trigger.parentElement?.querySelector("input") as HTMLInputElement; - expect(taskInput.disabled).toBe(true); - }); - - it("running panel enables the TaskPicker when the entry has a project", async () => { - setRunning(runningTimer({ description: "x", project_id: "p-1" })); - const { getByLabelText } = render(() => ); - await flushMicrotasks(); - const trigger = getByLabelText("Open task list") as HTMLButtonElement; - const taskInput = trigger.parentElement?.querySelector("input") as HTMLInputElement; - expect(taskInput.disabled).toBe(false); + expect(getByLabelText("Open project or task list")).toBeDefined(); }); }); diff --git a/ui/src/test/routes/Popover.test.tsx b/ui/src/test/routes/Popover.test.tsx index fdb285e..7ba3e8c 100644 --- a/ui/src/test/routes/Popover.test.tsx +++ b/ui/src/test/routes/Popover.test.tsx @@ -34,6 +34,7 @@ vi.mock("~/api", () => ({ listProjects: vi.fn().mockResolvedValue([ { id: "p-1", name: "Tet", color: null, client_id: null, client_name: null, archived: 0 }, ]), + listTasks: vi.fn().mockResolvedValue([]), }, })); diff --git a/ui/src/test/routes/Today.test.tsx b/ui/src/test/routes/Today.test.tsx index c76a215..17b7e65 100644 --- a/ui/src/test/routes/Today.test.tsx +++ b/ui/src/test/routes/Today.test.tsx @@ -25,6 +25,7 @@ vi.mock("~/api", () => ({ api: { listToday: vi.fn().mockResolvedValue([]), listProjects: vi.fn().mockResolvedValue([]), + listTasks: vi.fn().mockResolvedValue([]), syncNow: vi.fn().mockResolvedValue(0), deleteEntry: vi.fn().mockResolvedValue(undefined), listSyncErrors: vi.fn().mockResolvedValue([]), From f2f3abaeb76f46bf22b19821e598146c0e3d1d3a Mon Sep 17 00:00:00 2001 From: Mario Meyer Date: Mon, 1 Jun 2026 10:26:41 -0400 Subject: [PATCH 2/3] fix(picker): visual polish after first review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three iteration tweaks based on local testing: 1. Chevron clarity — bigger button (h-6 w-6 instead of h-4 w-4), SVG caret instead of unicode ▸/▾, only rendered for projects that actually have tasks. Projects without tasks get a bullet • in the same column so the layout aligns and they don't hint at expansion. 2. Search expansion stickiness — split expansion into user-driven and search-driven sets. Clearing the query collapses search-only expansions back; user-driven expansions persist. 3. Task styling — drop the ↳ prefix, indent more (w-6 spacer + w-4 dash column), smaller font (text-xs) + muted color (text-zinc-600). Tasks now get a leading – instead of ↳. 3 new tests for the search-expansion separation + chevron-absence on empty projects. Total 14 picker tests. --- ui/src/components/ui/ProjectTaskPicker.tsx | 199 ++++++++++++------ .../components/ProjectTaskPicker.test.tsx | 96 ++++++++- 2 files changed, 225 insertions(+), 70 deletions(-) diff --git a/ui/src/components/ui/ProjectTaskPicker.tsx b/ui/src/components/ui/ProjectTaskPicker.tsx index 99cffc6..84492fd 100644 --- a/ui/src/components/ui/ProjectTaskPicker.tsx +++ b/ui/src/components/ui/ProjectTaskPicker.tsx @@ -20,16 +20,17 @@ import type { Project, Task } from "~/types"; * no tasks. * * Layout: tree. "(No project)" sentinel at top, projects rendered as bold - * rows with a chevron, tasks indented underneath. Project rows are - * selectable (selecting one yields project-only, task_id=null). + * rows with a chevron when they have tasks (no chevron for childless + * projects). Project rows are selectable (selecting one yields + * project-only, task_id=null). * * Search: smart filter. Typing matches projects AND tasks. Matching tasks * auto-expand their parent project; if the parent's name doesn't match the * query the parent is shown dimmed (still clickable for project-only). + * Search-driven expansions don't persist after the query clears. * * Default expansion: all collapsed. The project containing the currently - * selected task auto-expands when the dropdown opens. Any query - * auto-expands all projects with matches. + * selected task auto-expands when the dropdown opens. * * Keyboard: ↑↓ traverses visible rows; Enter selects; Esc closes; Right * expands a collapsed project; Left collapses an expanded project (or @@ -43,7 +44,13 @@ export type ProjectTaskValue = { type Row = | { kind: "none"; key: string } - | { kind: "project"; key: string; project: PickerOption; dim: boolean } + | { + kind: "project"; + key: string; + project: PickerOption; + dim: boolean; + hasTasks: boolean; + } | { kind: "task"; key: string; project: PickerOption; task: Task }; interface Props { @@ -60,10 +67,12 @@ interface Props { export default function ProjectTaskPicker(props: Props) { const [open, setOpen] = createSignal(false); const [query, setQuery] = createSignal(""); - const [expanded, setExpanded] = createSignal>(new Set()); + // User-driven expansions only — survives query changes. Persists for + // the lifetime of the dropdown instance. + const [userExpanded, setUserExpanded] = createSignal>(new Set()); const [highlightIdx, setHighlightIdx] = createSignal(0); - // Group tasks by project_id and produce a stable accessor. + // Group tasks by project_id (active tasks only) and produce a stable accessor. const tasksByProject = createMemo(() => { const m = new Map(); for (const t of props.tasks) { @@ -80,10 +89,40 @@ export default function ProjectTaskPicker(props: Props) { const projectOptions = createMemo(() => buildPickerOptions(props.projects)); - // Visible rows: respects search query AND expansion state. + // Projects auto-expanded purely because the active search query has + // matching tasks under them. Recomputed from the query; not stored in + // userExpanded, so clearing the query collapses them back automatically. + const searchExpansions = createMemo>(() => { + const q = query().trim().toLowerCase(); + if (!q) return new Set(); + const out = new Set(); + for (const p of projectOptions()) { + const projMatch = p.name.toLowerCase().includes(q); + const projTasks = tasksByProject().get(p.id) ?? []; + const hasMatchingTask = projTasks.some((t) => + t.name.toLowerCase().includes(q), + ); + if (projMatch || hasMatchingTask) out.add(p.id); + } + return out; + }); + + // The effective expanded set is the union of user-driven and search- + // driven expansions. Searching shows more without losing user state; + // clearing search reverts to user state alone. + const effectiveExpanded = createMemo>(() => { + const u = userExpanded(); + const s = searchExpansions(); + if (s.size === 0) return u; + const ns = new Set(u); + for (const id of s) ns.add(id); + return ns; + }); + + // Visible rows: respects search query AND effective expansion state. const rows = createMemo(() => { const q = query().trim().toLowerCase(); - const exp = expanded(); + const exp = effectiveExpanded(); const out: Row[] = [{ kind: "none", key: "__none__" }]; for (const p of projectOptions()) { @@ -92,6 +131,7 @@ export default function ProjectTaskPicker(props: Props) { const matchingTasks = q ? projTasks.filter((t) => t.name.toLowerCase().includes(q)) : projTasks; + const hasTasks = projTasks.length > 0; if (q) { // Search mode: include project if its name matches OR it has any @@ -104,6 +144,7 @@ export default function ProjectTaskPicker(props: Props) { key: `p:${p.id}`, project: p, dim: !projMatch, + hasTasks, }); for (const t of matchingTasks) { out.push({ @@ -115,7 +156,13 @@ export default function ProjectTaskPicker(props: Props) { } } else { // Normal mode: project always visible; tasks only if expanded. - out.push({ kind: "project", key: `p:${p.id}`, project: p, dim: false }); + out.push({ + kind: "project", + key: `p:${p.id}`, + project: p, + dim: false, + hasTasks, + }); if (exp.has(p.id)) { for (const t of projTasks) { out.push({ @@ -133,13 +180,14 @@ export default function ProjectTaskPicker(props: Props) { // When the dropdown opens, auto-expand the project that owns the // currently selected task so the user can see the current value - // without first having to expand by hand. + // without first having to expand by hand. This counts as a user + // expansion since the user effectively selected this previously. createEffect( on(open, (isOpen) => { if (!isOpen) return; const v = props.value; if (v.taskId && v.projectId) { - setExpanded((s) => { + setUserExpanded((s) => { if (s.has(v.projectId!)) return s; const ns = new Set(s); ns.add(v.projectId!); @@ -152,29 +200,9 @@ export default function ProjectTaskPicker(props: Props) { }), ); - // Auto-expand any project that has matches whenever the query changes. - // Empty query restores whatever expansion the user had explicitly set; - // we only expand on non-empty queries. + // Reset highlight on query change to avoid pointing at a filtered-out row. createEffect( - on(query, (q) => { - const trimmed = q.trim().toLowerCase(); - if (!trimmed) return; - const matching = new Set(); - for (const p of projectOptions()) { - const projMatch = p.name.toLowerCase().includes(trimmed); - const projTasks = tasksByProject().get(p.id) ?? []; - const hasMatchingTask = projTasks.some((t) => - t.name.toLowerCase().includes(trimmed), - ); - if (projMatch || hasMatchingTask) matching.add(p.id); - } - setExpanded((s) => { - // Union with whatever the user had open so we don't collapse - // them mid-search. - const ns = new Set(s); - for (const id of matching) ns.add(id); - return ns; - }); + on(query, () => { setHighlightIdx(0); }), ); @@ -208,8 +236,8 @@ export default function ProjectTaskPicker(props: Props) { setOpen(false); } - function toggleExpand(projectId: string) { - setExpanded((s) => { + function toggleUserExpand(projectId: string) { + setUserExpanded((s) => { const ns = new Set(s); if (ns.has(projectId)) ns.delete(projectId); else ns.add(projectId); @@ -235,17 +263,24 @@ export default function ProjectTaskPicker(props: Props) { } case "ArrowRight": { const row = list[idx]; - if (row?.kind === "project" && !expanded().has(row.project.id)) { + if ( + row?.kind === "project" && + row.hasTasks && + !effectiveExpanded().has(row.project.id) + ) { e.preventDefault(); - toggleExpand(row.project.id); + toggleUserExpand(row.project.id); } break; } case "ArrowLeft": { const row = list[idx]; - if (row?.kind === "project" && expanded().has(row.project.id)) { + if ( + row?.kind === "project" && + effectiveExpanded().has(row.project.id) + ) { e.preventDefault(); - toggleExpand(row.project.id); + toggleUserExpand(row.project.id); } else if (row?.kind === "task") { e.preventDefault(); const parentIdx = list.findIndex( @@ -323,11 +358,12 @@ export default function ProjectTaskPicker(props: Props) { row={row} highlighted={i() === highlightIdx()} isExpanded={ - row.kind === "project" && expanded().has(row.project.id) + row.kind === "project" && + effectiveExpanded().has(row.project.id) } onSelect={() => commitRow(row)} onToggle={() => { - if (row.kind === "project") toggleExpand(row.project.id); + if (row.kind === "project") toggleUserExpand(row.project.id); }} onHover={() => setHighlightIdx(i())} /> @@ -380,45 +416,84 @@ function RowItem(props: { No project ); - case "project": + case "project": { + const row = props.row; return ( <> - + + - {props.row.project.name} + {row.project.name} - + - {props.row.project.clientName} + {row.project.clientName} ); + } case "task": return ( <> - - - ↳ {props.row.task.name} + + + – + + + {props.row.task.name} ); diff --git a/ui/src/test/components/ProjectTaskPicker.test.tsx b/ui/src/test/components/ProjectTaskPicker.test.tsx index 74f5840..decd1f7 100644 --- a/ui/src/test/components/ProjectTaskPicker.test.tsx +++ b/ui/src/test/components/ProjectTaskPicker.test.tsx @@ -132,12 +132,12 @@ describe("", () => { )); fireEvent.click(getByLabelText("Open project or task list")); await flush(); - expect(screen.queryByText("↳ First")).toBeNull(); + expect(screen.queryByText("First")).toBeNull(); fireEvent.click(screen.getByLabelText("Expand")); await flush(); - expect(screen.queryByText("↳ First")).not.toBeNull(); - expect(screen.queryByText("↳ Second")).not.toBeNull(); - fireEvent.click(screen.getByText("↳ Second")); + expect(screen.queryByText("First")).not.toBeNull(); + expect(screen.queryByText("Second")).not.toBeNull(); + fireEvent.click(screen.getByText("Second")); await flush(); expect(onChange).toHaveBeenCalledWith({ projectId: "p-1", @@ -156,7 +156,7 @@ describe("", () => { )); fireEvent.click(getByLabelText("Open project or task list")); await flush(); - expect(screen.queryByText("↳ First")).not.toBeNull(); + expect(screen.queryByText("First")).not.toBeNull(); }); it("smart search auto-expands projects with matching tasks", async () => { @@ -182,7 +182,7 @@ describe("", () => { search.value = "refactor"; fireEvent.input(search); await flush(); - expect(screen.queryByText("↳ Refactor picker")).not.toBeNull(); + expect(screen.queryByText("Refactor picker")).not.toBeNull(); expect(screen.queryByText("Beta")).toBeNull(); }); @@ -204,8 +204,88 @@ describe("", () => { // Expand explicitly in case auto-expand only triggers when taskId is set. fireEvent.click(screen.getByLabelText("Expand")); await flush(); - expect(screen.queryByText("↳ Active")).not.toBeNull(); - expect(screen.queryByText("↳ Finished")).toBeNull(); + expect(screen.queryByText("Active")).not.toBeNull(); + expect(screen.queryByText("Finished")).toBeNull(); + }); + + it("does NOT render a chevron for projects with no tasks", async () => { + const { getByLabelText } = render(() => ( + + )); + fireEvent.click(getByLabelText("Open project or task list")); + await flush(); + // Tasks have no chevron when there are no children. + expect(screen.queryByLabelText("Expand")).toBeNull(); + }); + + it("search-driven expansions clear when the query is removed", async () => { + const { getByLabelText } = render(() => ( + + )); + fireEvent.click(getByLabelText("Open project or task list")); + await flush(); + // Beta starts collapsed — its task not visible. + expect(screen.queryByText("Unique")).toBeNull(); + // Search opens it. + const search = screen.getByPlaceholderText( + "Search projects + tasks…", + ) as HTMLInputElement; + search.value = "unique"; + fireEvent.input(search); + await flush(); + expect(screen.queryByText("Unique")).not.toBeNull(); + // Clear the query — Beta should collapse back since the expansion was + // search-driven, not user-driven. + search.value = ""; + fireEvent.input(search); + await flush(); + expect(screen.queryByText("Unique")).toBeNull(); + }); + + it("user-driven expansions persist across search activity", async () => { + const { getByLabelText } = render(() => ( + + )); + fireEvent.click(getByLabelText("Open project or task list")); + await flush(); + // User expands Alpha manually. + fireEvent.click(screen.getByLabelText("Expand")); + await flush(); + expect(screen.queryByText("Visible")).not.toBeNull(); + // Type a query that doesn't match anything. + const search = screen.getByPlaceholderText( + "Search projects + tasks…", + ) as HTMLInputElement; + search.value = "zzz-nope"; + fireEvent.input(search); + await flush(); + // Clear the query — the user's manual expansion should still hold. + search.value = ""; + fireEvent.input(search); + await flush(); + expect(screen.queryByText("Visible")).not.toBeNull(); }); it("shows an empty-state message when search has no matches", async () => { From c2fe481558399607b232bb9796d07a2cf34d2109 Mon Sep 17 00:00:00 2001 From: Mario Meyer Date: Tue, 16 Jun 2026 13:03:30 -0400 Subject: [PATCH 3/3] fix(popover): clear task selection after starting a timer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex flagged on PR #29: the popover's success callback resets description / billable / startAt but leaves taskId in place. Result — after the user stops a timer started with a task, the next popover start silently inherits that task unless they explicitly clear the picker. TimerCard already clears both project and task; the popover was the inconsistent one. Fix only resets taskId. projectId stays — the pre-picker popover preserved project across back-to-back timer starts and users rely on that for working on the same project repeatedly. --- ui/src/routes/Popover.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ui/src/routes/Popover.tsx b/ui/src/routes/Popover.tsx index 43a1622..b7e401f 100644 --- a/ui/src/routes/Popover.tsx +++ b/ui/src/routes/Popover.tsx @@ -103,6 +103,11 @@ export default function Popover() { setDescription(""); setBillable(false); setStartAt(null); + // Clear the task so the next start doesn't silently + // inherit it. Keep the project — the old single-picker + // popover preserved project across starts and users + // rely on that for back-to-back same-project work. + setTaskId(null); }); }} >