From c4381397e68ec40fb00c6d254074927f6f42c72f Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Wed, 26 Aug 2026 02:25:52 -0700 Subject: [PATCH 1/4] feat(ui): annotation progress bars fill with success on a bordered track The Progress primitive gains a success variant beside its primary default and takes its ref as a prop. BatchProgressBar draws the batch's annotation as an 8px success fill on a muted track with a border hairline, in the list and the gallery header alike, and a draft row says it is not approved yet rather than counting work it has no jobs for. The job header's bar takes the same variant; ingest and model-download bars keep primary. --- frontend/app/e2e/navigation.spec.ts | 74 +++++++++++++++---- frontend/app/src/styleguide/Styleguide.tsx | 11 +++ frontend/ui-core/src/primitives/Feedback.tsx | 33 ++++++--- .../src/primitives/primitives.test.tsx | 24 ++++++ .../ui-core/src/screens/BatchLifecycle.tsx | 14 +++- .../ui-core/src/screens/BatchesScreen.tsx | 2 +- frontend/ui-core/src/screens/JobPanels.tsx | 1 + .../src/screens/batchLifecycle.test.tsx | 36 ++++++++- 8 files changed, 164 insertions(+), 31 deletions(-) diff --git a/frontend/app/e2e/navigation.spec.ts b/frontend/app/e2e/navigation.spec.ts index 576ee4ea..9797fd74 100644 --- a/frontend/app/e2e/navigation.spec.ts +++ b/frontend/app/e2e/navigation.spec.ts @@ -59,6 +59,20 @@ const PIXEL = Buffer.from( * Everything is static: this suite asks one question — where does the way out * go? — and nothing about it depends on state moving. */ +const BATCH_OUT = { + id: BATCH, + project_id: PROJECT, + name: "drive-01", + state: "in_annotation", + allowed_actions: batchActions("in_annotation"), + promoted_asset_count: 0, + parent_batch_id: null, + pre_label_run: null, + schema_version: 1, + asset_count: 1, + progress: { ...NO_PROGRESS, unannotated: 1, total: 1 }, +} satisfies Wire["BatchOut"]; + async function serveApi(page: Page): Promise { await page.route("**/api/**", (route) => { const request = route.request(); @@ -119,23 +133,10 @@ async function serveApi(page: Page): Promise { json: { ...NO_PROGRESS, unannotated: 1, total: 1 } satisfies Wire["ProgressCounts"], }); } - if (path === `/batches/${BATCH}`) { - return route.fulfill({ - json: { - id: BATCH, - project_id: PROJECT, - name: "drive-01", - state: "in_annotation", - allowed_actions: batchActions("in_annotation"), - promoted_asset_count: 0, - parent_batch_id: null, - pre_label_run: null, - schema_version: 1, - asset_count: 1, - progress: { ...NO_PROGRESS, unannotated: 1, total: 1 }, - } satisfies Wire["BatchOut"], - }); + if (path === `/projects/${PROJECT}/batches`) { + return route.fulfill({ json: { items: [BATCH_OUT], total: 1 } satisfies Wire["BatchPage"] }); } + if (path === `/batches/${BATCH}`) return route.fulfill({ json: BATCH_OUT }); if (path === `/batches/${BATCH}/assets`) { return route.fulfill({ json: { @@ -373,3 +374,44 @@ test("the dataset is one press from every other section", async ({ page }) => { await page.getByTestId("nav-dataset").click(); await expect(page.getByTestId("dataset-screen")).toBeVisible(); }); + +/** A CSS colour as the sRGB bytes it paints, so two spellings of one token compare equal. */ +async function channelsOf(page: Page, colour: string): Promise { + return page.evaluate((value) => { + const canvas = document.createElement("canvas"); + canvas.width = 1; + canvas.height = 1; + const ctx = canvas.getContext("2d")!; + ctx.fillStyle = value; + ctx.fillRect(0, 0, 1, 1); + return Array.from(ctx.getImageData(0, 0, 1, 1).data); + }, colour); +} + +test("a batch row's progress fills with the success colour on a bordered muted track", async ({ + page, +}) => { + await openCold(page, `/projects/${PROJECT}/batches`); + await expect(page.getByTestId("batches-screen")).toBeVisible(); + + const bar = page.getByRole("progressbar", { name: "Annotation progress" }).first(); + await expect(bar).toBeVisible(); + const tokens = await page.evaluate(() => { + const style = getComputedStyle(document.documentElement); + return { + success: style.getPropertyValue("--success"), + muted: style.getPropertyValue("--muted"), + border: style.getPropertyValue("--border"), + }; + }); + const painted = async (locator: typeof bar, prop: string): Promise => + channelsOf(page, await locator.evaluate((node, p) => getComputedStyle(node).getPropertyValue(p), prop)); + + expect(await painted(bar.locator("> *").first(), "background-color")).toEqual( + await channelsOf(page, tokens.success), + ); + expect(await painted(bar, "background-color")).toEqual(await channelsOf(page, tokens.muted)); + expect(await painted(bar, "border-top-color")).toEqual(await channelsOf(page, tokens.border)); + // 8px, so an empty track is still a visible shape and not a hairline. + expect(await bar.evaluate((node) => node.getBoundingClientRect().height)).toBe(8); +}); diff --git a/frontend/app/src/styleguide/Styleguide.tsx b/frontend/app/src/styleguide/Styleguide.tsx index 81ad4045..0f9fed5d 100644 --- a/frontend/app/src/styleguide/Styleguide.tsx +++ b/frontend/app/src/styleguide/Styleguide.tsx @@ -351,6 +351,17 @@ export function Styleguide(): JSX.Element {

Ingest — 240 of 412

+
+

+ Annotation — 7 of 11 annotated (64%); the batch surfaces' variant +

+ +
, - ComponentPropsWithoutRef ->(function Progress({ className, value, ...props }, ref) { +// `primary`, not `brand`: the fill is a functional control — the thing a person +// watches to know work is happening — and brand is identity, never a functional +// colour. `success` is the batch-state family's settled colour, for a bar that +// measures annotation rather than a transfer. +const progressFill = cva("h-full w-full flex-1 transition-transform", { + variants: { + variant: { + primary: "bg-primary", + success: "bg-success", + }, + }, + defaultVariants: { variant: "primary" }, +}); + +export type ProgressProps = ComponentProps & + VariantProps; + +export function Progress({ className, value, variant, ...props }: ProgressProps): JSX.Element { return ( ); -}); +} export function Skeleton({ className, ...props }: HTMLAttributes): JSX.Element { return ( diff --git a/frontend/ui-core/src/primitives/primitives.test.tsx b/frontend/ui-core/src/primitives/primitives.test.tsx index 7af86a32..17f83054 100644 --- a/frontend/ui-core/src/primitives/primitives.test.tsx +++ b/frontend/ui-core/src/primitives/primitives.test.tsx @@ -299,6 +299,30 @@ describe("Progress", () => { "42", ); }); + + it("fills with the functional colour by default and with success when asked", () => { + const { rerender } = render(); + const fill = (): Element => screen.getByRole("progressbar").firstElementChild as Element; + expect(fill().className).toContain("bg-primary"); + expect(fill().className).not.toContain("bg-success"); + rerender(); + expect(fill().className).toContain("bg-success"); + expect(fill().className).not.toContain("bg-primary"); + }); + + it("hands its ref to the track element", () => { + let track: HTMLDivElement | null = null; + render( + { + track = node; + }} + />, + ); + expect(track).toBe(screen.getByRole("progressbar")); + }); }); describe("Dialog", () => { diff --git a/frontend/ui-core/src/screens/BatchLifecycle.tsx b/frontend/ui-core/src/screens/BatchLifecycle.tsx index ff03f71b..8dbcc50c 100644 --- a/frontend/ui-core/src/screens/BatchLifecycle.tsx +++ b/frontend/ui-core/src/screens/BatchLifecycle.tsx @@ -67,17 +67,27 @@ import { export function BatchProgressBar({ counts, detailed = true, + draft = false, ...rest }: { readonly counts: ProgressCounts; /** The gallery header states one sentence; the batch table lists every state. */ readonly detailed?: boolean; + /** A draft has no jobs, so a count of work "to do" would name work that does not exist yet. */ + readonly draft?: boolean; } & { readonly "data-testid"?: string }): JSX.Element { const share = annotatedShare(counts); return (
- - {detailed ? ( + + {draft ? ( + Not approved yet + ) : detailed ? ( {counts.annotated} annotated · {counts.skipped} skipped · {counts.accepted} accepted ·{" "} {counts.unannotated} to do diff --git a/frontend/ui-core/src/screens/BatchesScreen.tsx b/frontend/ui-core/src/screens/BatchesScreen.tsx index 8e1a67af..541b6dbd 100644 --- a/frontend/ui-core/src/screens/BatchesScreen.tsx +++ b/frontend/ui-core/src/screens/BatchesScreen.tsx @@ -179,7 +179,7 @@ export function BatchesScreen({ : `v${batch.schema_version}`} - + {/* The forward action, then `⋯`. Deleting is the one thing a diff --git a/frontend/ui-core/src/screens/JobPanels.tsx b/frontend/ui-core/src/screens/JobPanels.tsx index 19d45a6d..05c9c9e8 100644 --- a/frontend/ui-core/src/screens/JobPanels.tsx +++ b/frontend/ui-core/src/screens/JobPanels.tsx @@ -227,6 +227,7 @@ function JobHeader({ )} diff --git a/frontend/ui-core/src/screens/batchLifecycle.test.tsx b/frontend/ui-core/src/screens/batchLifecycle.test.tsx index e7e552b4..0a763ff0 100644 --- a/frontend/ui-core/src/screens/batchLifecycle.test.tsx +++ b/frontend/ui-core/src/screens/batchLifecycle.test.tsx @@ -21,7 +21,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { JSX, ReactNode } from "react"; import { ApiProvider } from "../data/ApiProvider"; -import { ApproveDialog } from "./BatchLifecycle"; +import { ApproveDialog, BatchProgressBar } from "./BatchLifecycle"; import type { Batch } from "./queries"; import { batchActions } from "../testing/wire.fixtures.js"; @@ -182,3 +182,37 @@ describe("the approve dialog's refusals", () => { expect(screen.queryByTestId("approve-schema-missing")).toBeNull(); }); }); + +describe("BatchProgressBar", () => { + const COUNTS = { + total: 11, + unannotated: 4, + pre_labeled: 0, + annotated: 7, + skipped: 0, + review_pending: 0, + accepted: 0, + }; + + it("draws annotation as a success fill on a bordered track, above the counts", () => { + render(); + const bar = screen.getByRole("progressbar", { name: "Annotation progress" }); + expect(bar.getAttribute("aria-valuenow")).toBe("64"); + expect(bar.className).toContain("h-2"); + expect(bar.className).toContain("border"); + expect((bar.firstElementChild as Element).className).toContain("bg-success"); + expect(screen.getByText(/7 annotated · 0 skipped · 0 accepted · 4 to do/)).toBeTruthy(); + }); + + it("says a draft is not approved yet rather than counting work it has no jobs for", () => { + render( + , + ); + expect(screen.getByRole("progressbar").getAttribute("aria-valuenow")).toBe("0"); + expect(screen.getByText("Not approved yet")).toBeTruthy(); + expect(screen.queryByText(/to do/)).toBeNull(); + }); +}); From 74506fbceaec894555225c6540b3d2e1572b7fea Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Wed, 26 Aug 2026 02:31:04 -0700 Subject: [PATCH 2/4] fix(server): BatchOut.of reads the pre-label run on every path The constructor took None for pre_label_run by default, so a route that forgot the read published a batch nobody had pre-labeled. The parameter is now required, as JobOut.of already has it, and every projection does the read. --- src/visionset/server/models.py | 5 +++-- src/visionset/server/routes/assets.py | 9 ++++++++- src/visionset/server/routes/batches.py | 14 +++++++++++--- tests/server/test_wire_models.py | 12 ++++++++++-- 4 files changed, 32 insertions(+), 8 deletions(-) diff --git a/src/visionset/server/models.py b/src/visionset/server/models.py index 52679c4e..cbd7cb6a 100644 --- a/src/visionset/server/models.py +++ b/src/visionset/server/models.py @@ -1227,7 +1227,7 @@ def of( counts: dict[AssetProgress, int], *, promoted: AbstractSet[UUID], - pre_label_run: PreLabelRun | None = None, + pre_label_run: PreLabelRun | None, ) -> Self: return cls( id=batch.id, @@ -1343,9 +1343,10 @@ def of( counts: dict[AssetProgress, int], *, promoted: AbstractSet[UUID], + pre_label_run: PreLabelRun | None, ) -> Self: return cls( - batch=BatchOut.of(change.batch, counts, promoted=promoted), + batch=BatchOut.of(change.batch, counts, promoted=promoted, pre_label_run=pre_label_run), changed=list(change.changed), ) diff --git a/src/visionset/server/routes/assets.py b/src/visionset/server/routes/assets.py index e99458db..e7b6044d 100644 --- a/src/visionset/server/routes/assets.py +++ b/src/visionset/server/routes/assets.py @@ -179,9 +179,16 @@ def list_asset_batches(workspace: WorkspaceDep, project_id: UUID, asset_id: UUID jobs = JobService(workspace) promoted = _promoted(workspace, project_id) found = batches.holding(asset.id) + pre_label_runs = batches.pre_label_runs() return BatchPage( items=[ - BatchOut.of(batch, jobs.batch_progress(batch.id), promoted=promoted) for batch in found + BatchOut.of( + batch, + jobs.batch_progress(batch.id), + promoted=promoted, + pre_label_run=pre_label_runs.get(batch.id), + ) + for batch in found ], total=len(found), ) diff --git a/src/visionset/server/routes/batches.py b/src/visionset/server/routes/batches.py index 23a4fce9..762a454b 100644 --- a/src/visionset/server/routes/batches.py +++ b/src/visionset/server/routes/batches.py @@ -119,6 +119,7 @@ def create_batch(workspace: WorkspaceDep, project_id: UUID, body: BatchCreate) - created, JobService(workspace).batch_progress(created.id), promoted=_promoted(workspace, project_id), + pre_label_run=batches.latest_pre_label_job(created.id), ) @@ -188,22 +189,26 @@ def approve_batch( pin, and an unknown batch is 404 `BATCH_NOT_FOUND`. """ partition = None if body is None else body.to_domain() - batch = BatchService(workspace).approve(batch_id, partition) + batches = BatchService(workspace) + batch = batches.approve(batch_id, partition) return BatchOut.of( batch, JobService(workspace).batch_progress(batch.id), promoted=_promoted(workspace, batch.project_id), + pre_label_run=batches.latest_pre_label_job(batch_id), ) @router.post("/{batch_id}/start", responses=documented(404, 409)) def start_batch(workspace: WorkspaceDep, batch_id: UUID) -> BatchOut: """Open the batch for annotation. Nothing may be written into it before this.""" - batch = BatchService(workspace).start(batch_id) + batches = BatchService(workspace) + batch = batches.start(batch_id) return BatchOut.of( batch, JobService(workspace).batch_progress(batch.id), promoted=_promoted(workspace, batch.project_id), + pre_label_run=batches.latest_pre_label_job(batch_id), ) @@ -289,11 +294,13 @@ def create_correction_batch( parent's pin. That is the point of correcting under a contract that has moved on, and it is the ordinary approval mechanism rather than anything new. """ - created = BatchService(workspace).create_correction(batch_id, body.name, body.asset_ids) + batches = BatchService(workspace) + created = batches.create_correction(batch_id, body.name, body.asset_ids) return BatchOut.of( created, JobService(workspace).batch_progress(created.id), promoted=_promoted(workspace, created.project_id), + pre_label_run=batches.latest_pre_label_job(created.id), ) @@ -643,6 +650,7 @@ def _membership(workspace: WorkspaceDep, change: MembershipChange) -> BatchMembe change, JobService(workspace).batch_progress(change.batch.id), promoted=_promoted(workspace, change.batch.project_id), + pre_label_run=BatchService(workspace).latest_pre_label_job(change.batch.id), ) diff --git a/tests/server/test_wire_models.py b/tests/server/test_wire_models.py index 5eca486d..61a32039 100644 --- a/tests/server/test_wire_models.py +++ b/tests/server/test_wire_models.py @@ -398,8 +398,16 @@ def test_a_batch_publishes_its_lineage() -> None: child = samples.BATCH.model_copy(update={"parent_batch_id": parent}) orphan = samples.BATCH.model_copy(update={"parent_batch_id": None}) - assert BatchOut.of(child, samples.COUNTS, promoted=frozenset()).parent_batch_id == parent - assert BatchOut.of(orphan, samples.COUNTS, promoted=frozenset()).parent_batch_id is None + assert ( + BatchOut.of(child, samples.COUNTS, promoted=frozenset(), pre_label_run=None).parent_batch_id + == parent + ) + assert ( + BatchOut.of( + orphan, samples.COUNTS, promoted=frozenset(), pre_label_run=None + ).parent_batch_id + is None + ) def test_an_annotation_publishes_the_round_that_produced_it() -> None: From f5fdba4baf672b744e0111ca7f1ae11bfc14b5ad Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Wed, 26 Aug 2026 02:38:39 -0700 Subject: [PATCH 3/4] feat(ui): a one-job batch draws its job flat under the header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With exactly one job the gallery renders no accordion and no job-level bar: the job's door, Pre-label and assignee sit under the batch header, followed by the filter, the order, the strip and the grid, and the batch bar is the page's one bar. The assignee reads as a line — Assigned to , or Unassigned — with the name as the control. From two jobs the accordion stays, each header naming its assignee. The pieces both the screen and the accordion mount move to GalleryControls, so JobPanels no longer imports GalleryScreen while GalleryScreen imports it. A closed panel's header carries no aria-controls, since its panel is unmounted, and each job's filter, order and selection are kept by job id so reopening a panel restores them. The view is patched per field at write time: the grid reports its selection from an effect that can run after a filter change, and a whole-view write from it put the old filter back. --- frontend/ui-core/src/screens/FrameGrid.tsx | 11 +- .../ui-core/src/screens/GalleryControls.tsx | 610 ++++++++++++++++++ .../ui-core/src/screens/GalleryScreen.tsx | 319 +-------- frontend/ui-core/src/screens/JobPanels.tsx | 372 ++++------- frontend/ui-core/src/screens/gallery.test.tsx | 110 +++- .../ui-core/src/screens/jobPanels.test.tsx | 43 +- 6 files changed, 891 insertions(+), 574 deletions(-) create mode 100644 frontend/ui-core/src/screens/GalleryControls.tsx diff --git a/frontend/ui-core/src/screens/FrameGrid.tsx b/frontend/ui-core/src/screens/FrameGrid.tsx index 40441ed5..9243a17c 100644 --- a/frontend/ui-core/src/screens/FrameGrid.tsx +++ b/frontend/ui-core/src/screens/FrameGrid.tsx @@ -139,6 +139,12 @@ export interface FrameGridProps { readonly onLoaded?: (assets: readonly BatchAsset[]) => void; /** The selection, for a host whose own controls act on it. */ readonly onSelectionChange?: (selected: ReadonlySet) => void; + /** + * What is selected when the grid mounts. A host that remembers a job's + * selection while its panel is closed hands it back here on reopening; read + * once, so the host's copy never fights the grid's own. + */ + readonly initialSelection?: ReadonlySet; /** Lets a timeline pick scroll the grid: filled with a function that scrolls to an asset id. */ readonly scrollRef?: RefObject<((assetId: string) => void) | null>; /** What to say when the whole batch is empty — only an unfiltered view can. */ @@ -157,11 +163,14 @@ export function FrameGrid({ onCorrect, onLoaded, onSelectionChange, + initialSelection, scrollRef, emptyBatch, }: FrameGridProps): JSX.Element { const assets = useBatchAssets(batchId, view); - const [selected, setSelected] = useState>(new Set()); + const [selected, setSelected] = useState>( + () => initialSelection ?? new Set(), + ); const [highlighted, setHighlighted] = useState(null); const anchor = useRef(null); diff --git a/frontend/ui-core/src/screens/GalleryControls.tsx b/frontend/ui-core/src/screens/GalleryControls.tsx new file mode 100644 index 00000000..521942b7 --- /dev/null +++ b/frontend/ui-core/src/screens/GalleryControls.tsx @@ -0,0 +1,610 @@ +/** + * The controls a batch's frames are worked through, whichever screen mounts them. + * + * `GalleryScreen` composes the page and `JobPanels` the accordion, and both put + * the same things around a job's frames: the door into the annotator, the + * pre-label trigger, the assignee, the segment filter, the order, the timeline + * and the grid. They live here so neither of those two modules has to import the + * other for them — `JobWorkspace` is that set as one component, mounted flat + * when a batch has one job and inside an accordion panel when it has several. + */ + +import { useCallback, useRef, useState, type JSX } from "react"; +import { Play, User, X } from "lucide-react"; + +import type { AssetProgress } from "../annotator/jobQueries"; +import { Button } from "../primitives/Button"; +import { FieldError, Input } from "../primitives/Input"; +import { JOB_ACTION, declares } from "../data/capabilities"; +import { refusalProse } from "../data/refusals"; +import { DEFAULT_DENSITY, DENSITY_STEPS, FrameGrid } from "./FrameGrid"; +import { PreLabelButton } from "./PreLabelDialog"; +import { + progressCellClass, + progressLabel, + SEGMENT_LABEL, + SEGMENTS, + segmentCounts, + segmentProgress, + type Segment, +} from "./batchState"; +import { + useAssignJob, + useStartJob, + type AssetSort, + type Batch, + type BatchAsset, + type Job, + type ProgressCounts, +} from "./queries"; + +// --- one job's frames and everything that acts on them ------------------------ + +/** What a person chose about how to look at one job's frames. */ +export interface JobView { + readonly segment: Segment; + readonly sort: AssetSort; + readonly selected: ReadonlySet; +} + +export const DEFAULT_JOB_VIEW: JobView = { + segment: "all", + sort: "membership", + selected: new Set(), +}; + +/** + * `view` after `patch`, or `view` itself when nothing would change. A host keeps + * its state's identity when the answer is the same, which is what stops the + * grid's selection report — fired on every identity change of its callback — + * from becoming a loop. + */ +export function patchView(view: JobView, patch: Partial): JobView { + const next = { ...view, ...patch }; + const same = + next.segment === view.segment && + next.sort === view.sort && + next.selected.size === view.selected.size && + [...next.selected].every((id) => view.selected.has(id)); + return same ? view : next; +} + +export interface JobWorkspaceProps { + readonly projectId: string; + readonly batch: Batch; + readonly job: Job; + /** Which job of the batch this is, as the accordion counts them. */ + readonly ordinal: number; + /** The shared thumbnail size, chosen once for the screen. */ + readonly minColumn: number; + /** Absent while the job's counts are in flight, or when their read was refused. */ + readonly counts: ProgressCounts | undefined; + /** + * The filter, the order and the selection are the host's, keyed by job, so a + * panel that closes and reopens comes back the way it was left. Each control + * reports the one field it owns as a patch: the grid's selection report can + * arrive after a filter change, and a whole view from it would put the old + * filter back. + */ + readonly view: JobView; + readonly onView: (patch: Partial) => void; + /** + * With one job the name is a line of the action row; with several it is on + * the accordion header and the panel keeps a button. + */ + readonly assignee: "line" | "button"; + readonly onOpenAsset?: (asset: BatchAsset) => void; + readonly onOpenJob?: (jobId: string) => void; + /** Open the correction dialog the header owns, with this selection. */ + readonly onCorrect?: () => void; + /** The loaded window, for the header's provenance line. */ + readonly onLoaded?: (assets: readonly BatchAsset[]) => void; +} + +/** + * In order: the door into the annotator and the two things that can be done to + * the job as a whole, then that job's filters, its timeline and its grid. The + * counts behind the segments are the **job's** `ProgressCounts`, never the + * batch's — a chip reading `All (48)` over a job holding twelve is the filter + * lying about what it filters. + */ +export function JobWorkspace({ + projectId, + batch, + job, + ordinal, + minColumn, + counts, + view, + onView, + assignee, + onOpenAsset, + onOpenJob, + onCorrect, + onLoaded, +}: JobWorkspaceProps): JSX.Element { + const [loaded, setLoaded] = useState([]); + const [highlighted, setHighlighted] = useState(null); + const scrollToAsset = useRef<((assetId: string) => void) | null>(null); + + // Both the timeline and the screen's header read the same window, and the + // identity has to be stable: `FrameGrid` re-runs its report whenever this + // changes. + const report = useCallback( + (assets: readonly BatchAsset[]) => { + setLoaded(assets); + onLoaded?.(assets); + }, + [onLoaded], + ); + const setSegment = (segment: Segment): void => onView({ segment }); + const setSort = (sort: AssetSort): void => onView({ sort }); + const setSelected = (selected: ReadonlySet): void => onView({ selected }); + + return ( + <> +
+ {onOpenJob !== undefined && ( + + )} + + +
+ + + + { + scrollToAsset.current?.(assetId); + setHighlighted(assetId); + }} + /> + + + + ); +} + +// --- the way into a job ------------------------------------------------------- + +/** + * The way into one job. A `pending` job is taken (`start`) and then opened, so + * `in_progress` means somebody has it open; anything else only opens. The label + * says which: Annotate, Continue, View. Never the page's filled control — the + * batch's own step in the header is, while it has one, and the navigation + * column's Annotate once the batch is open. + */ +export function StartJobButton({ + batchId, + job, + onOpenJob, +}: { + readonly batchId: string; + readonly job: Job; + readonly onOpenJob: (jobId: string) => void; +}): JSX.Element { + const start = useStartJob(batchId, job.id); + const starts = declares(job, JOB_ACTION.start); + // `Continue` is only ever the word for a job somebody is inside. A `pending` + // job that does not declare `start` — every job of an `approved` batch — is + // not continuable and not startable from here, so it reads as what it is. + const label = starts ? "Annotate" : job.state === "in_progress" ? "Continue" : "View"; + return ( +
+ + {start.isError && {refusalProse(start.error)}} +
+ ); +} + +// --- who has the job ---------------------------------------------------------- + +/** + * Who is working this job. A name, not an account — `JobService.assign` takes a + * plain string and there is no annotator identity to enforce anything against — + * so the control is always live; there is nothing to gate it on. + */ +export function AssigneeEditor({ + batchId, + job, + ordinal, + presentation, +}: { + readonly batchId: string; + readonly job: Job; + readonly ordinal: number; + readonly presentation: "line" | "button"; +}): JSX.Element { + const assign = useAssignJob(batchId, job.id); + const [editing, setEditing] = useState(false); + const [draft, setDraft] = useState(""); + // Escape closes the editor WITHOUT committing, and it does so by unmounting + // the input — which is what fires the blur a naive `onBlur={commit}` would + // then read as "the user tabbed away, save it". This flag is how Escape's own + // blur is told apart from every other one: set immediately before the state + // change that causes it, read (and cleared) by the blur that follows. + const discarding = useRef(false); + + function commit(): void { + const name = draft.trim(); + if (name.length === 0 || name === (job.assignee ?? "")) { + setEditing(false); + return; + } + assign.mutate(name, { onSuccess: () => setEditing(false) }); + } + + function edit(): void { + setDraft(job.assignee ?? ""); + setEditing(true); + } + + const input = ( + setDraft(event.target.value)} + onKeyDown={(event) => { + if (event.key === "Enter") commit(); + if (event.key === "Escape") { + discarding.current = true; + setEditing(false); + } + }} + onBlur={() => { + if (discarding.current) { + discarding.current = false; + return; + } + commit(); + }} + className="w-40" + /> + ); + const clear = job.assignee !== null && !editing && ( + + ); + const error = assign.isError && {refusalProse(assign.error)}; + + if (presentation === "button") { + return ( + <> + {editing ? ( + input + ) : ( + + )} + {clear} + {error} + + ); + } + + return ( +
+
+ ); +} + +// --- toolbar ----------------------------------------------------------------- + +/** + * The five segments, the order, and the density ladder — and it is mounted twice + * with a different half of itself each time. + * + * The segments and the order belong to **one job**; the density belongs to the + * **screen** and is rendered once above the frames. So each mount says which + * half it is, and the props for the other half are the ones it does not pass. + * + * The counts come off a `ProgressCounts` — the job's, or the batch's for a draft + * — and never off the loaded pages: the pages are a window onto a collection that + * can hold fifty thousand, and a filter whose counts described the hundred in + * memory would be a filter that lies about what it filters. `segmentCounts` owns + * the grouping and the argument for it. + */ +export function Toolbar({ + segment = "all", + counts, + onSegment, + sort = "membership", + onSort, + density = DEFAULT_DENSITY, + onDensity, + showSegments, + showDensity = true, +}: { + readonly segment?: Segment; + /** + * Absent while the counts are in flight, and for a job whose progress read was + * refused. The chips and the order select are drawn without their numbers + * rather than withheld — the *numbers* are what is missing, and a panel with no + * way to filter or order is a panel one failed read has made unusable. + */ + readonly counts?: Record; + readonly onSegment?: (next: Segment) => void; + readonly sort?: AssetSort; + readonly onSort?: (next: AssetSort) => void; + readonly density?: number; + readonly onDensity?: (step: number) => void; + /** + * False for a draft, and for the shared size control above the frames. Every + * frame in a draft is in the same state — there is nothing to filter *between* — + * and the counts behind the segments are the documented zeros a batch with no + * jobs reports, so five segments reading `(0)` over a full grid is the screen + * contradicting itself. + */ + readonly showSegments: boolean; + /** + * False beside a job's frames. How big the thumbnails are is one setting for + * the screen, so a copy of it per job would be several answers to one question. + */ + readonly showDensity?: boolean; +}): JSX.Element { + return ( +
+ {showSegments && ( + <> + {/* The row above already wraps, but the control is one joined pill and + is wider than the narrowest viewport on its own. It scrolls within + its own row rather than widening the page — the project navigation's + answer to the same shape — because squashing five state filters + costs more than a scroll does. The padding pair keeps the focus ring + off the scroller's clip. */} +
+
+ {SEGMENTS.map((one) => ( + + ))} +
+
+ + + + )} + + {showDensity && onDensity !== undefined && ( + + )} +
+ ); +} + +/** + * How big the thumbnails are — one setting for the screen, wherever it is drawn. + * + * A native range input, not a Radix slider: `@radix-ui/react-slider` is not a + * dependency and this task adds none. The native control is also keyboard + * operable and announced correctly for free, which a div with a drag handler + * would have had to earn back. + * + * It is its own component because the two paths mount it in different places: a + * draft has it in the toolbar over its flat grid, and a batch with jobs has it on + * the header's progress row, where it is the only thing above the frames that + * is not about one job. + */ +export function DensityControl({ + density, + onDensity, +}: { + readonly density: number; + readonly onDensity: (step: number) => void; +}): JSX.Element { + return ( + + ); +} + +// --- timeline ---------------------------------------------------------------- + +/** + * One cell per loaded frame, coloured by its **exact** state. + * + * Deliberately not the segmented grouping: the toolbar groups because "is there + * work left" is the right thing to filter by, and this strip is the one place you + * can see a whole job's states side by side. Clicking scrolls the grid to that + * frame and marks it, so the eye can find it after the jump. + * + * The time labels read the frames' own `frame_timestamp`, which is the locator + * that survives a re-decomposition, and render nothing rather than deriving + * seconds from a sampling rate that may not exist — a bunch of stills has no fps + * and no timestamps, and a timeline of "0s → 0s" over it would be a fabrication. + */ +export function Timeline({ + assets, + onPick, + highlighted, +}: { + readonly assets: readonly BatchAsset[]; + readonly onPick: (assetId: string) => void; + readonly highlighted: string | null; +}): JSX.Element | null { + if (assets.length === 0) return null; + const start = assets[0]?.frame_timestamp; + const end = assets[assets.length - 1]?.frame_timestamp; + + return ( +
+ + {start === null || start === undefined ? "" : `${Math.round(start)}s`} + +
+ {assets.map((asset) => ( +
+ + {end === null || end === undefined ? "" : `${Math.round(end)}s`} + +
+ ); +} + +/** + * A timeline cell, from the same vocabulary the cards use. + * + * One vocabulary for both, so a colour on the strip and a dot on a card cannot + * come to mean different things — and that vocabulary is *semantic* + * rather than a monochrome ramp off `primary`. A ramp is a quantity: it says + * how far along a frame is and cannot say what kind of state it is in, so + * `accepted` and `annotated` come out the same near-black and + * `review_pending` is that near-black at 40%, which reads as "less annotated" + * rather than as "waiting on somebody". + * + * The colour lives in `batchState.ts`; what stays here is the geometry and the + * highlight ring, which are the strip's own. + */ +function cellClass(progress: AssetProgress | null | undefined, isHighlighted: boolean): string { + const ring = isHighlighted ? " ring-2 ring-ring" : ""; + return `h-full min-w-0 flex-1 ${progressCellClass(progress)}${ring}`; +} + diff --git a/frontend/ui-core/src/screens/GalleryScreen.tsx b/frontend/ui-core/src/screens/GalleryScreen.tsx index 8940ac4f..d5148c99 100644 --- a/frontend/ui-core/src/screens/GalleryScreen.tsx +++ b/frontend/ui-core/src/screens/GalleryScreen.tsx @@ -3,16 +3,15 @@ * * What it holds is the chrome around the frames: the header's provenance line * and the one setting that is about looking rather than working, the thumbnail - * size. **Once the batch has jobs, the frames themselves are inside a job** — see - * `JobPanels`, which owns the accordion, its filters, its timeline and its grid. - * A draft has no jobs, so it keeps the flat grid it always had. + * size. **Once the batch has jobs, the frames themselves belong to a job** — + * flat under the header when the batch has one job, inside an accordion from + * two; see `JobPanels`. A draft has no jobs, so it keeps the flat grid it + * always had. */ import { useState, type JSX, type ReactNode } from "react"; -import { Play } from "lucide-react"; import { readStep, writePref } from "../data/prefs"; -import type { AssetProgress } from "../annotator/jobQueries"; import { Badge } from "../primitives/Badge"; import { Button } from "../primitives/Button"; import { FieldError } from "../primitives/Input"; @@ -32,21 +31,17 @@ import { } from "./BatchLifecycle"; import { CorrectionButton, CorrectionOf } from "./CorrectionBatch"; import { BatchOverflowMenu } from "./DeleteBatch"; -import { JobPanels } from "./JobPanels"; +import { DensityControl, Toolbar } from "./GalleryControls"; +import { JobPanels, SingleJobWorkspace, type JobPanelsProps } from "./JobPanels"; import { PromoteButton } from "./PromoteButton"; -import { BATCH_ACTION, JOB_ACTION, declares } from "../data/capabilities"; +import { BATCH_ACTION, declares } from "../data/capabilities"; import { refusalProse } from "../data/refusals"; import { BATCH_STATE_VARIANT, batchStateLabel, earliestArrival, hasJobs, - progressCellClass, - progressLabel, relativeAge, - SEGMENT_LABEL, - SEGMENTS, - type Segment, } from "./batchState"; import { GALLERY_PAGE_SIZE, @@ -54,11 +49,8 @@ import { useBatchJobs, useBatches, useSource, - useStartJob, - type AssetSort, type Batch, type BatchAsset, - type Job, } from "./queries"; export interface GalleryScreenProps { @@ -222,8 +214,11 @@ export function GalleryScreen({ {showsProgress && jobs.isError && {refusalProse(jobs.error)}} + {/* One job is the batch: its controls and frames sit right under the + header, and the batch bar above is the page's one bar. The accordion + exists from two jobs, where choosing between them is the point. */} {showsProgress && batch.data !== undefined && roster.length > 0 && ( - + ) : ( + + ); +} + // --- header ------------------------------------------------------------------ /** @@ -391,9 +394,9 @@ function BatchHeader({ )} {/* The batch's own next step, answered from `allowed_actions`. The way - *into* the annotator is not here: a batch's frames are partitioned - into jobs, so which frames to open is a question only a job can - answer — see `JobPanels`. + *into* the annotator is not among these: a batch's frames are + partitioned into jobs, so the door is the job's, drawn under this + header with the rest of the job's controls — see `JobPanels`. */} {startsAnnotation && batch !== undefined && } {/* @@ -485,283 +488,5 @@ function BatchHeader({ ); } -// --- the way into a job ------------------------------------------------------- - -/** - * The way into one job. A `pending` job is taken (`start`) and then opened, so - * `in_progress` means somebody has it open; anything else only opens. The label - * says which: Annotate, Continue, View. Never the page's filled control — the - * batch's own step in the header is, and a row of them would be several. - */ -export function StartJobButton({ - batchId, - job, - onOpenJob, -}: { - readonly batchId: string; - readonly job: Job; - readonly onOpenJob: (jobId: string) => void; -}): JSX.Element { - const start = useStartJob(batchId, job.id); - const starts = declares(job, JOB_ACTION.start); - // `Continue` is only ever the word for a job somebody is inside. A `pending` - // job that does not declare `start` — every job of an `approved` batch — is - // not continuable and not startable from here, so it reads as what it is. - const label = starts ? "Annotate" : job.state === "in_progress" ? "Continue" : "View"; - return ( -
- - {start.isError && {refusalProse(start.error)}} -
- ); -} - -// --- toolbar ----------------------------------------------------------------- - -/** - * The five segments, the order, and the density ladder — and it is mounted twice - * with a different half of itself each time. - * - * The segments and the order belong to **one job**, inside its panel; the density - * belongs to the **screen** and is rendered once above the accordion. So each - * mount says which half it is, and the props for the other half are the ones it - * does not pass. - * - * The counts come off a `ProgressCounts` — the job's, or the batch's for a draft - * — and never off the loaded pages: the pages are a window onto a collection that - * can hold fifty thousand, and a filter whose counts described the hundred in - * memory would be a filter that lies about what it filters. `segmentCounts` owns - * the grouping and the argument for it. - */ -export function Toolbar({ - segment = "all", - counts, - onSegment, - sort = "membership", - onSort, - density = DEFAULT_DENSITY, - onDensity, - showSegments, - showDensity = true, -}: { - readonly segment?: Segment; - /** - * Absent while the counts are in flight, and for a job whose progress read was - * refused. The chips and the order select are drawn without their numbers - * rather than withheld — the *numbers* are what is missing, and a panel with no - * way to filter or order is a panel one failed read has made unusable. - */ - readonly counts?: Record; - readonly onSegment?: (next: Segment) => void; - readonly sort?: AssetSort; - readonly onSort?: (next: AssetSort) => void; - readonly density?: number; - readonly onDensity?: (step: number) => void; - /** - * False for a draft, and for the shared size control above the accordion. Every - * frame in a draft is in the same state — there is nothing to filter *between* — - * and the counts behind the segments are the documented zeros a batch with no - * jobs reports, so five segments reading `(0)` over a full grid is the screen - * contradicting itself. - */ - readonly showSegments: boolean; - /** - * False inside a job panel. How big the thumbnails are is one setting for the - * screen, so a copy of it in each panel would be four answers to one question. - */ - readonly showDensity?: boolean; -}): JSX.Element { - return ( -
- {showSegments && ( - <> - {/* The row above already wraps, but the control is one joined pill and - is wider than the narrowest viewport on its own. It scrolls within - its own row rather than widening the page — the project navigation's - answer to the same shape — because squashing five state filters - costs more than a scroll does. The padding pair keeps the focus ring - off the scroller's clip. */} -
-
- {SEGMENTS.map((one) => ( - - ))} -
-
- - - - )} - - {showDensity && onDensity !== undefined && ( - - )} -
- ); -} - -/** - * How big the thumbnails are — one setting for the screen, wherever it is drawn. - * - * A native range input, not a Radix slider: `@radix-ui/react-slider` is not a - * dependency and this task adds none. The native control is also keyboard - * operable and announced correctly for free, which a div with a drag handler - * would have had to earn back. - * - * It is its own component because the two paths mount it in different places: a - * draft has it in the toolbar over its flat grid, and a batch with jobs has it on - * the header's progress row, where it is the only thing above the accordion that - * is not about one job. - */ -function DensityControl({ - density, - onDensity, -}: { - readonly density: number; - readonly onDensity: (step: number) => void; -}): JSX.Element { - return ( - - ); -} - -// --- timeline ---------------------------------------------------------------- - -/** - * One cell per loaded frame, coloured by its **exact** state. - * - * Deliberately not the segmented grouping: the toolbar groups because "is there - * work left" is the right thing to filter by, and this strip is the one place you - * can see a whole batch's states side by side. Clicking scrolls the grid to that - * frame and marks it, so the eye can find it after the jump. - * - * The time labels read the frames' own `frame_timestamp`, which is the locator - * that survives a re-decomposition, and render nothing rather than deriving - * seconds from a sampling rate that may not exist — a bunch of stills has no fps - * and no timestamps, and a timeline of "0s → 0s" over it would be a fabrication. - */ -export function Timeline({ - assets, - onPick, - highlighted, -}: { - readonly assets: readonly BatchAsset[]; - readonly onPick: (assetId: string) => void; - readonly highlighted: string | null; -}): JSX.Element | null { - if (assets.length === 0) return null; - const start = assets[0]?.frame_timestamp; - const end = assets[assets.length - 1]?.frame_timestamp; - - return ( -
- - {start === null || start === undefined ? "" : `${Math.round(start)}s`} - -
- {assets.map((asset) => ( -
- - {end === null || end === undefined ? "" : `${Math.round(end)}s`} - -
- ); -} - -/** - * A timeline cell, from the same vocabulary the cards use. - * - * One vocabulary for both, so a colour on the strip and a dot on a card cannot - * come to mean different things — and that vocabulary is *semantic* - * rather than a monochrome ramp off `primary`. A ramp is a quantity: it says - * how far along a frame is and cannot say what kind of state it is in, so - * `accepted` and `annotated` come out the same near-black and - * `review_pending` is that near-black at 40%, which reads as "less annotated" - * rather than as "waiting on somebody". - * - * The colour lives in `batchState.ts`; what stays here is the geometry and the - * highlight ring, which are the strip's own. - */ -function cellClass(progress: AssetProgress | null | undefined, isHighlighted: boolean): string { - const ring = isHighlighted ? " ring-2 ring-ring" : ""; - return `h-full min-w-0 flex-1 ${progressCellClass(progress)}${ring}`; -} - export { columnsFor } from "./FrameGrid"; export { GALLERY_PAGE_SIZE }; diff --git a/frontend/ui-core/src/screens/JobPanels.tsx b/frontend/ui-core/src/screens/JobPanels.tsx index 05c9c9e8..c686dd15 100644 --- a/frontend/ui-core/src/screens/JobPanels.tsx +++ b/frontend/ui-core/src/screens/JobPanels.tsx @@ -1,7 +1,7 @@ /** - * A batch's jobs, as an accordion — at most one of them open, and every one closable. + * A batch's jobs, on the gallery: flat when there is one, an accordion from two. * - * ## Why the frames moved inside a job + * ## Why the frames live inside a job * * A batch's assets are partitioned into jobs at approval, so "which frames am I * working" is a question only a job can answer. While the gallery drew one @@ -10,9 +10,17 @@ * person working job 2 scrolled past job 1's frames to reach their own, and the * strip could not say which job needed them without opening each one. * - * So the grid, the segment counts and the timeline all live in the open panel and - * all read that job. What stays outside is the one setting that is about looking - * rather than about working: the thumbnail size. + * So the grid, the segment counts and the timeline all read one job. What stays + * outside is the one setting that is about looking rather than about working: + * the thumbnail size. + * + * ## One job is the batch + * + * The common batch has one job, and a one-row accordion is a header nobody can + * choose between, a bar repeating the batch's own and a sentence naming a job + * nobody else has. So with exactly one job there is no accordion: the job's + * controls sit under the batch header and its frames follow, and the batch bar + * is the page's one bar. The accordion exists from two jobs. * * ## At most one open, and every panel may be closed * @@ -21,33 +29,20 @@ * accordion read as an index. Which one opens on arrival is `defaultOpenJob`, and * it deliberately waits for **every** job's counts: opening off a half-read map * means opening the wrong job and jumping when the rest land. + * + * A closed panel is unmounted, and what a person chose inside it — the filter, + * the order, the selection — is kept here by job id, so reopening restores it. */ -import { useCallback, useEffect, useRef, useState, type JSX, type KeyboardEvent } from "react"; -import { ChevronDown, ChevronRight, X } from "lucide-react"; +import { useEffect, useState, type JSX, type KeyboardEvent } from "react"; +import { ChevronDown, ChevronRight, User } from "lucide-react"; -import { Button } from "../primitives/Button"; import { Progress } from "../primitives/Feedback"; -import { FieldError, Input } from "../primitives/Input"; +import { FieldError } from "../primitives/Input"; import { refusalProse } from "../data/refusals"; -import { FrameGrid } from "./FrameGrid"; -import { StartJobButton, Timeline, Toolbar } from "./GalleryScreen"; -import { PreLabelButton } from "./PreLabelDialog"; -import { - annotatedShare, - segmentCounts, - segmentProgress, - type Segment, -} from "./batchState"; -import { - useAssignJob, - useJobsProgress, - type AssetSort, - type Batch, - type BatchAsset, - type Job, - type ProgressCounts, -} from "./queries"; +import { DEFAULT_JOB_VIEW, JobWorkspace, patchView, type JobView } from "./GalleryControls"; +import { annotatedShare } from "./batchState"; +import { useJobsProgress, type Batch, type BatchAsset, type Job, type ProgressCounts } from "./queries"; export interface JobPanelsProps { readonly projectId: string; @@ -89,6 +84,77 @@ export function defaultOpenJob( return unfinished?.id ?? jobs[0]?.id ?? null; } +/** + * What each job's frames are being looked at through, remembered across a close. + * + * A patch is applied to the state as it is *then*, not as the caller saw it: the + * grid reports its selection from an effect that can run after a filter change, + * and merging at write time is what keeps that report from undoing the filter. + */ +function useJobViews(): readonly [ + (jobId: string) => JobView, + (jobId: string, patch: Partial) => void, +] { + const [views, setViews] = useState>(new Map()); + const viewOf = (jobId: string): JobView => views.get(jobId) ?? DEFAULT_JOB_VIEW; + const setView = (jobId: string, patch: Partial): void => + setViews((current) => { + const before = current.get(jobId) ?? DEFAULT_JOB_VIEW; + const after = patchView(before, patch); + return after === before ? current : new Map(current).set(jobId, after); + }); + return [viewOf, setView]; +} + +/** + * The one job's controls and frames, with nothing between them and the batch + * header. Its counts are read here, the same way the accordion reads every + * job's, so the segment chips have their numbers. + */ +export function SingleJobWorkspace({ + projectId, + batch, + jobs, + minColumn, + onOpenAsset, + onOpenJob, + onCorrect, + onLoaded, + onSelectionChange, +}: JobPanelsProps): JSX.Element { + const job = jobs[0] as Job; + const { counts: progress, error } = useJobsProgress([job.id]); + const [viewOf, setView] = useJobViews(); + + return ( +
+ {error !== null && {refusalProse(error)}} + { + setView(job.id, patch); + if (patch.selected !== undefined) onSelectionChange?.(patch.selected); + }} + assignee="line" + {...(onOpenAsset === undefined ? {} : { onOpenAsset })} + {...(onOpenJob === undefined ? {} : { onOpenJob })} + {...(onCorrect === undefined ? {} : { onCorrect })} + {...(onLoaded === undefined ? {} : { onLoaded })} + /> +
+ ); +} + export function JobPanels({ projectId, batch, @@ -101,6 +167,7 @@ export function JobPanels({ onSelectionChange, }: JobPanelsProps): JSX.Element { const { counts: progress, error } = useJobsProgress(jobs.map((one) => one.id)); + const [viewOf, setView] = useJobViews(); // `undefined` is "the default has not been applied yet"; `null` is "the person // closed the last panel". The distinction is what stops the default from // reasserting itself: derived every render, it would move the open panel out @@ -120,6 +187,8 @@ export function JobPanels({ function toggle(jobId: string): void { if (jobId !== open) { setOpen(jobId); + // The header reads the open panel's selection; the one opening has its own. + onSelectionChange?.(viewOf(jobId).selected); return; } setOpen(null); @@ -146,22 +215,35 @@ export function JobPanels({ onOpen={() => toggle(job.id)} /> {job.id === open && ( - // Keyed on the job, so the segment, the order and the selection are - // the open job's own rather than the previous one's carried over. - + role="region" + id={`job-panel-${job.id}`} + aria-labelledby={`job-header-${job.id}`} + data-testid={`job-panel-${job.id}`} + className="flex flex-col gap-3 border-t border-border p-3" + > + { + setView(job.id, patch); + if (patch.selected !== undefined) onSelectionChange?.(patch.selected); + }} + assignee="button" + {...(onOpenAsset === undefined ? {} : { onOpenAsset })} + {...(onOpenJob === undefined ? {} : { onOpenJob })} + {...(onCorrect === undefined ? {} : { onCorrect })} + {...(onLoaded === undefined ? {} : { onLoaded })} + /> +
)} ))} @@ -202,7 +284,9 @@ function JobHeader({ id={`job-header-${job.id}`} data-testid={`job-header-${job.id}`} aria-expanded={expanded} - aria-controls={`job-panel-${job.id}`} + // Only while the panel exists: a closed panel is unmounted, and an id + // pointing at nothing is a broken reference rather than a closed one. + {...(expanded ? { "aria-controls": `job-panel-${job.id}` } : {})} onClick={onOpen} onKeyDown={moveBetweenHeaders} className={ @@ -231,8 +315,9 @@ function JobHeader({ className="min-w-0 flex-1" /> )} - - {job.assignee ?? "—"} + + ); @@ -268,200 +353,3 @@ function moveBetweenHeaders(event: KeyboardEvent): void { event.preventDefault(); target.focus(); } - -/** - * One job's frames and everything that acts on them. - * - * In order: the door into the annotator and the two things that can be done to - * the job as a whole, then that job's filters, its timeline and its grid. The - * counts behind the segments are the **job's** `ProgressCounts`, never the - * batch's — a chip reading `All (48)` over a job holding twelve is the filter - * lying about what it filters. - */ -function JobPanel({ - projectId, - batch, - job, - ordinal, - minColumn, - counts, - onOpenAsset, - onOpenJob, - onCorrect, - onLoaded, - onSelectionChange, -}: { - readonly projectId: string; - readonly batch: Batch; - readonly job: Job; - readonly ordinal: number; - readonly minColumn: number; - readonly counts: ProgressCounts | undefined; - readonly onOpenAsset?: (asset: BatchAsset) => void; - readonly onOpenJob?: (jobId: string) => void; - readonly onCorrect?: () => void; - readonly onLoaded?: (assets: readonly BatchAsset[]) => void; - readonly onSelectionChange?: (selected: ReadonlySet) => void; -}): JSX.Element { - const [segment, setSegment] = useState("all"); - const [sort, setSort] = useState("membership"); - const [loaded, setLoaded] = useState([]); - const [highlighted, setHighlighted] = useState(null); - const scrollToAsset = useRef<((assetId: string) => void) | null>(null); - - // Both this panel's timeline and the screen's header read the same window, and - // the identity has to be stable: `FrameGrid` re-runs its report whenever this - // changes. - const report = useCallback( - (assets: readonly BatchAsset[]) => { - setLoaded(assets); - onLoaded?.(assets); - }, - [onLoaded], - ); - - return ( -
-
- {onOpenJob !== undefined && ( - - )} - - -
- - - - { - scrollToAsset.current?.(assetId); - setHighlighted(assetId); - }} - /> - - -
- ); -} - -/** - * Who is working this job. A name, not an account — `JobService.assign` takes a - * plain string and there is no annotator identity to enforce anything against — - * so the control is always live; there is nothing to gate it on. - */ -function AssigneeEditor({ - batchId, - job, - ordinal, -}: { - readonly batchId: string; - readonly job: Job; - readonly ordinal: number; -}): JSX.Element { - const assign = useAssignJob(batchId, job.id); - const [editing, setEditing] = useState(false); - const [draft, setDraft] = useState(""); - // Escape closes the editor WITHOUT committing, and it does so by unmounting - // the input — which is what fires the blur a naive `onBlur={commit}` would - // then read as "the user tabbed away, save it". This flag is how Escape's own - // blur is told apart from every other one: set immediately before the state - // change that causes it, read (and cleared) by the blur that follows. - const discarding = useRef(false); - - function commit(): void { - const name = draft.trim(); - if (name.length === 0 || name === (job.assignee ?? "")) { - setEditing(false); - return; - } - assign.mutate(name, { onSuccess: () => setEditing(false) }); - } - - return ( - <> - {editing ? ( - setDraft(event.target.value)} - onKeyDown={(event) => { - if (event.key === "Enter") commit(); - if (event.key === "Escape") { - discarding.current = true; - setEditing(false); - } - }} - onBlur={() => { - if (discarding.current) { - discarding.current = false; - return; - } - commit(); - }} - className="w-40" - /> - ) : ( - - )} - {job.assignee !== null && !editing && ( - - )} - {assign.isError && {refusalProse(assign.error)}} - - ); -} diff --git a/frontend/ui-core/src/screens/gallery.test.tsx b/frontend/ui-core/src/screens/gallery.test.tsx index 8acdff7f..14726396 100644 --- a/frontend/ui-core/src/screens/gallery.test.tsx +++ b/frontend/ui-core/src/screens/gallery.test.tsx @@ -2074,9 +2074,10 @@ describe("the job panel's way into the annotator", () => { />, ), ); - // The accordion first: the frames are *inside* a panel now, so there are no - // tiles at all until the job roster and its counts have both landed. - await screen.findByTestId("job-panels"); + // The job's workspace first: the frames belong to the job, so there are no + // tiles at all until the roster and its counts have both landed. One job + // means no accordion — the workspace sits flat under the header. + await screen.findByTestId("job-workspace"); await screen.findByTestId("tile-asset-0"); return openedJob; } @@ -2084,13 +2085,48 @@ describe("the job panel's way into the annotator", () => { it("offers no door in the header once the batch is open", async () => { await openWith("in_annotation", "pending", "unannotated"); expect(screen.queryByTestId("start-annotating")).toBeNull(); - // Pre-label is a job's action now, so every trigger on the page is inside the - // accordion. Asserting a batch-keyed testid is absent would pass on a page - // that still had a header mount, because the testid is keyed by job. - const panels = screen.getByTestId("job-panels"); - const triggers = [...document.querySelectorAll('[data-testid^="pre-label-"]')]; - expect(triggers.length).toBeGreaterThan(0); - expect(triggers.every((node) => panels.contains(node))).toBe(true); + // Pre-label and the door are the job's, so every trigger on the page is in + // the job's workspace and none in the header. Asserting a batch-keyed testid + // is absent would pass on a page that still had a header mount, because the + // testid is keyed by job. + const workspace = screen.getByTestId("job-workspace"); + const header = screen.getByTestId("batch-title").closest("header") as HTMLElement; + const triggers = [ + ...document.querySelectorAll('[data-testid^="pre-label-"], [data-testid^="start-job-"]'), + ]; + expect(triggers.length).toBe(2); + expect(triggers.every((node) => workspace.contains(node))).toBe(true); + expect(triggers.some((node) => header.contains(node))).toBe(false); + }); + + it("draws one job flat: no accordion, no job header, one bar on the page", async () => { + await openWith("in_annotation", "in_progress", "unannotated", "annotated"); + expect(screen.queryByTestId("job-panels")).toBeNull(); + expect(screen.queryByTestId(`job-header-${JOB}`)).toBeNull(); + expect(screen.queryByTestId(`job-row-${JOB}`)).toBeNull(); + expect(screen.getAllByRole("progressbar")).toHaveLength(1); + // The workspace holds, in order, the action row, the filter, the strip and + // the grid — everything the accordion's open panel held, with nothing to + // choose between above it. + const workspace = within(screen.getByTestId("job-workspace")); + expect(workspace.getByTestId(`start-job-${JOB}`)).toBeTruthy(); + expect(workspace.getByTestId("segments")).toBeTruthy(); + expect(workspace.getByTestId("sort-order")).toBeTruthy(); + expect(workspace.getByTestId("timeline")).toBeTruthy(); + expect(workspace.getByTestId("tile-asset-0")).toBeTruthy(); + expect(workspace.getByTestId(`assignee-${JOB}`).textContent).toContain("Unassigned"); + }); + + it("keeps the header's filled control to the batch's own step, and to at most one", async () => { + // Two filled controls on a page are two answers to "what now?"; none on a + // page whose step is the column's Annotate is correct. The job's door is + // never the filled one — the batch's own step is, while it has one. + const filled = (): Element[] => [ + ...document.querySelectorAll('[data-testid="gallery"] [data-variant="primary"]'), + ]; + await openWith("in_annotation", "in_progress", "unannotated"); + expect(filled()).toHaveLength(0); + expect(screen.getByTestId(`start-job-${JOB}`).dataset.variant).toBe("secondary"); }); it("starts a pending job, then opens it", async () => { @@ -2255,12 +2291,21 @@ describe("the gallery header's own next step", () => { expect(screen.queryByTestId("start-annotating")).toBeNull(); }); - it("offers no Start on an in_annotation batch, whose door is in the job's panel", async () => { + it("offers no Start on an in_annotation batch, whose door is the job's", async () => { await openIn("in_annotation", "annotated", "unannotated", "unannotated"); expect(screen.queryByTestId("start-batch")).toBeNull(); expect(screen.queryByTestId("start-annotating")).toBeNull(); }); + it("fills exactly one control while the batch has a step of its own", async () => { + const filled = (): Element[] => [ + ...document.querySelectorAll('[data-testid="gallery"] [data-variant="primary"]'), + ]; + await openIn("approved", "unannotated", "unannotated"); + expect(filled()).toHaveLength(1); + expect(filled()[0]).toBe(screen.getByTestId("start-batch")); + }); + it("performs the batch's own start rather than navigating into the annotator", async () => { let started = false; handlers.push((request) => { @@ -2379,12 +2424,14 @@ describe("the jobs accordion", () => { } /** - * The assignee editor is in the **open panel**, not on every row: a collapsed - * header is an overview and names who has the job, and the control that changes - * that is one of the things opening a panel is for. + * With one job the editor is a line of the action row — "Assigned to Dana" or + * "Unassigned", the name itself the control. With several, the name is on the + * collapsed header and the editor is in the **open panel**, because a header + * is an overview and the control that changes it is one of the things opening + * a panel is for. */ - async function openPanel(): Promise> { - return within(await screen.findByTestId(`job-panel-${JOB}`)); + async function workspace(): Promise> { + return within(await screen.findByTestId("job-workspace")); } it("names each job's assignee on its own header, open or not", async () => { @@ -2405,10 +2452,10 @@ describe("the jobs accordion", () => { // The overview half of the claim: who has a job is legible without opening // it, and an unassigned one says so rather than saying nothing. expect((await screen.findByTestId(`job-row-${JOB}`)).textContent).toContain("Dana Reyes"); - expect(screen.getByTestId(`job-row-${OTHER_JOB}`).textContent).toContain("—"); + expect(screen.getByTestId(`job-row-${OTHER_JOB}`).textContent).toContain("Unassigned"); // ...and the editor is in whichever panel is open, once. - const panel = await openPanel(); + const panel = within(await screen.findByTestId(`job-panel-${JOB}`)); expect(panel.getByRole("button", { name: "Dana Reyes" })).toBeTruthy(); expect(screen.queryAllByLabelText(/Assignee for job/)).toHaveLength(0); }); @@ -2446,8 +2493,8 @@ describe("the jobs accordion", () => { return undefined; }); renderGallery(); - const panel = await openPanel(); - await userEvent.click(panel.getByRole("button", { name: /assign/i })); + const panel = await workspace(); + await userEvent.click(panel.getByRole("button", { name: "Unassigned" })); await userEvent.keyboard("Dana Reyes{Enter}"); const said = await panel.findByRole("alert"); expect(said.textContent).toContain("That job is no longer on record."); @@ -2471,13 +2518,14 @@ describe("the jobs accordion", () => { return undefined; }); renderGallery(); - const panel = await openPanel(); - await userEvent.click(panel.getByRole("button", { name: /assign/i })); + const panel = await workspace(); + await userEvent.click(panel.getByRole("button", { name: "Unassigned" })); await userEvent.keyboard("Dana Reyes{Enter}"); const put = sent.find((request) => request.method === "PUT"); expect(put).toBeTruthy(); expect(JSON.parse(bodies.get(put!) ?? "")).toEqual({ assignee: "Dana Reyes" }); - expect(await panel.findByText("Dana Reyes")).toBeTruthy(); + expect(await panel.findByRole("button", { name: "Dana Reyes" })).toBeTruthy(); + expect(panel.getByTestId(`assignee-${JOB}`).textContent).toContain("Assigned to"); }); it("commits the typed name on blur, not only on Enter", async () => { @@ -2490,8 +2538,8 @@ describe("the jobs accordion", () => { return undefined; }); renderGallery(); - const panel = await openPanel(); - await userEvent.click(panel.getByRole("button", { name: /assign/i })); + const panel = await workspace(); + await userEvent.click(panel.getByRole("button", { name: "Unassigned" })); await userEvent.type(panel.getByLabelText(/Assignee for job/), "Dana Reyes"); await userEvent.tab(); const put = sent.find((request) => request.method === "PUT"); @@ -2507,11 +2555,11 @@ describe("the jobs accordion", () => { return undefined; }); renderGallery(); - const panel = await openPanel(); - await userEvent.click(panel.getByRole("button", { name: /assign/i })); + const panel = await workspace(); + await userEvent.click(panel.getByRole("button", { name: "Unassigned" })); await userEvent.type(panel.getByLabelText(/Assignee for job/), "Dana Reyes"); await userEvent.keyboard("{Escape}"); - expect(await panel.findByRole("button", { name: "Assign" })).toBeTruthy(); + expect(await panel.findByRole("button", { name: "Unassigned" })).toBeTruthy(); expect(sent.some((request) => request.method === "PUT")).toBe(false); }); @@ -2523,10 +2571,10 @@ describe("the jobs accordion", () => { return undefined; }); renderGallery(); - const panel = await openPanel(); - await userEvent.click(panel.getByRole("button", { name: /assign/i })); + const panel = await workspace(); + await userEvent.click(panel.getByRole("button", { name: "Unassigned" })); await userEvent.tab(); - expect(await panel.findByRole("button", { name: "Assign" })).toBeTruthy(); + expect(await panel.findByRole("button", { name: "Unassigned" })).toBeTruthy(); expect(sent.some((request) => request.method === "PUT")).toBe(false); }); diff --git a/frontend/ui-core/src/screens/jobPanels.test.tsx b/frontend/ui-core/src/screens/jobPanels.test.tsx index a33c7cb5..7149d91c 100644 --- a/frontend/ui-core/src/screens/jobPanels.test.tsx +++ b/frontend/ui-core/src/screens/jobPanels.test.tsx @@ -334,11 +334,13 @@ describe("which panel opens", () => { await screen.findByTestId(`job-panel-${JOB_B}`); // A job that has stopped existing cannot stay open, and holding its id would - // leave the accordion closed over a batch that has jobs. + // leave the screen closed over a batch that has a job. Down to one, there is + // no accordion to hold open at all: the remaining job's frames sit flat. roster = [job(JOB_A, 2)]; await refetch(["batches"]); - expect(await screen.findByTestId(`job-panel-${JOB_A}`)).toBeTruthy(); + expect(await screen.findByTestId("job-workspace")).toBeTruthy(); + expect(screen.queryByTestId("job-panels")).toBeNull(); expect(screen.queryByTestId(`job-header-${JOB_B}`)).toBeNull(); }); @@ -437,7 +439,42 @@ describe("the collapsed header is the overview", () => { stubs(); renderGallery(); const row = await screen.findByTestId(`job-row-${JOB_B}`); - expect(row.textContent).toContain("—"); + expect(row.textContent).toContain("Unassigned"); + }); + + it("points aria-controls at a panel only while that panel exists", async () => { + // A closed panel is unmounted, so an id pointing at it would point at + // nothing; `aria-expanded="false"` with no `aria-controls` is the honest + // shape, and the open header names the region it controls. + stubs(); + renderGallery(); + await screen.findByTestId(`job-panel-${JOB_A}`); + const open = screen.getByTestId(`job-header-${JOB_A}`); + const closed = screen.getByTestId(`job-header-${JOB_B}`); + expect(open.getAttribute("aria-controls")).toBe(`job-panel-${JOB_A}`); + expect(document.getElementById(`job-panel-${JOB_A}`)).not.toBeNull(); + expect(closed.getAttribute("aria-expanded")).toBe("false"); + expect(closed.hasAttribute("aria-controls")).toBe(false); + }); + + it("restores a job's filter and order when its panel reopens", async () => { + stubs(); + renderGallery(); + await screen.findByTestId(`job-panel-${JOB_A}`); + await userEvent.click(screen.getByTestId("segment-done")); + await userEvent.selectOptions(screen.getByTestId("sort-order"), "confidence"); + expect(screen.getByTestId("segment-done").getAttribute("aria-pressed")).toBe("true"); + + await userEvent.click(screen.getByTestId(`job-header-${JOB_B}`)); + await screen.findByTestId(`job-panel-${JOB_B}`); + // The other job starts from the default: what was chosen was chosen for A. + expect(screen.getByTestId("segment-all").getAttribute("aria-pressed")).toBe("true"); + expect((screen.getByTestId("sort-order") as HTMLSelectElement).value).toBe("membership"); + + await userEvent.click(screen.getByTestId(`job-header-${JOB_A}`)); + await screen.findByTestId(`job-panel-${JOB_A}`); + expect(screen.getByTestId("segment-done").getAttribute("aria-pressed")).toBe("true"); + expect((screen.getByTestId("sort-order") as HTMLSelectElement).value).toBe("confidence"); }); it("moves between headers with the arrow keys", async () => { From c27615067d550c211a956c7b4f2d91a09353c005 Mon Sep 17 00:00:00 2001 From: Jesus Armando Anaya <1445792+JArmandoAnaya@users.noreply.github.com> Date: Wed, 26 Aug 2026 02:41:28 -0700 Subject: [PATCH 4/4] test(ui): the gallery's browser suites and docs follow the one-job shape The cycle scenario and the gallery specs walk a one-job batch as the flat workspace it now is, and a single-job scenario sits beside the two-job one. The information-architecture rule and the UI reference describe both shapes: one job flat under the header, an accordion from two. --- .../information-architecture/SKILL.md | 4 +- docs/content/ui.md | 131 ++++++++++-------- frontend/app/cycle/cycle.spec.ts | 37 ++--- frontend/app/e2e/gallery.spec.ts | 25 ++++ 4 files changed, 118 insertions(+), 79 deletions(-) diff --git a/.agents/skills/frontend/information-architecture/SKILL.md b/.agents/skills/frontend/information-architecture/SKILL.md index 44211a2b..0b9932b1 100644 --- a/.agents/skills/frontend/information-architecture/SKILL.md +++ b/.agents/skills/frontend/information-architecture/SKILL.md @@ -54,8 +54,8 @@ column carries are in `docs/content/ui/navigation.md`, *Inside a project*. Rules: -- **Annotate enters by job.** The project's `Annotate` control opens the chosen `in_annotation` batch's one job directly (`/jobs/:jobId`) when the batch has exactly one, and the batch gallery otherwise — the gallery's job panel is where a job is chosen, and it is the only door: the gallery header carries the `approved → in_annotation` transition and nothing that opens the editor. With several open batches the dropdown stays batch-level and the same rule applies to the pick. A panel's `Annotate` takes a `pending` job to `in_progress` before opening it; `Continue` and `View` only open. Panel controls are secondary — the header's transition is the page's one filled control. -- **A batch with jobs shows one job's frames at a time.** The gallery is an accordion of jobs with at most one panel open and every panel closable; the open panel's counts, timeline and frames are that job's, and the panel open on arrival is the first job with work left. A draft batch, having no jobs, keeps the flat grid. The rule is observability: a batch-wide grid beneath a per-job control puts two scopes for the same frames on one screen. Nothing about it changes an address — the gallery route is unchanged and the open panel is not in the URL. +- **Annotate enters by job.** The project's `Annotate` control opens the chosen `in_annotation` batch's one job directly (`/jobs/:jobId`) when the batch has exactly one, and the batch gallery otherwise — the gallery is where a job is chosen when there are several, and the job's own door is the only door: the gallery header carries the `approved → in_annotation` transition and nothing that opens the editor. With several open batches the dropdown stays batch-level and the same rule applies to the pick. A job's `Annotate` takes a `pending` job to `in_progress` before opening it; `Continue` and `View` only open. A job's controls are secondary — the header's transition is the page's one filled control while the batch has one, and the navigation column's Annotate once it is open. +- **A batch shows one job's frames at a time, and one job is the batch.** With exactly one job the gallery draws no accordion and no job-level bar: the job's controls — its door, Pre-label, and the assignee as an editable line — sit under the batch header, followed by that job's filter, order, strip and frames, and the batch bar is the page's one bar. From two jobs the gallery is an accordion with at most one panel open and every panel closable; the open panel's counts, timeline and frames are that job's, each header names its assignee, and the panel open on arrival is the first job with work left. A draft batch, having no jobs, keeps the flat grid. The rule is observability: a batch-wide grid beneath a per-job control puts two scopes for the same frames on one screen, and a one-row accordion is a choice with nothing to choose between. Nothing about it changes an address — the gallery route is unchanged and the open panel is not in the URL. - **A correction batch is reached from the batch that needs correcting**, never from a "new batch" form: the gallery header and the Batches row both offer it on a `completed` batch, capability-gated on `create_correction`. The annotator's read-only banner and the gallery's bulk bar *link* to it rather than duplicating it — creating a batch is a curation act, curation lives on the batch view, and a second place batches are made is a second place the rules can drift. - **Dataset is first-class.** It is the product's central object and must be reachable in ≤1 click from any project section. It is never gated behind, or discoverable only through, onboarding UI. Promotion success links onward to it; the gallery links to it once a batch is `completed`. - **The frames blocking a narrowing are a subsection of Schema, not a screen.** They are a *view of* the draft on the editor above them, the same relation version history has to the schema. A row links to **every** batch holding its frame rather than to one: an annotation carries an `asset_id` and no batch, so there is no single annotator address to prefer. The section is omitted entirely when the host wires no batch route, on the rule the Batches section already follows. It shows a window of the frames and states the total as text rather than a "see all": the destination that control would need is a project-wide asset view, and there is none — the count is a property of the proposal, not the length of a list somebody can open. diff --git a/docs/content/ui.md b/docs/content/ui.md index 34a65e30..c68b6710 100644 --- a/docs/content/ui.md +++ b/docs/content/ui.md @@ -726,58 +726,70 @@ virtualizes **rows** (a row is what the browser lays out; virtualizing tiles ins CSS grid means reimplementing the grid). The column count is measured with a `ResizeObserver` rather than guessed from a second breakpoint list. -#### The jobs accordion - -**Once a batch has jobs, its frames are shown per job, and at most one job is open at a -time.** A batch is partitioned, and a person works one part of it: a batch-wide grid beneath a -per-job control would put two scopes for the same frames on one screen, where the control -names a job and the grid, the counts and the timeline answer for everybody. One open panel is -one scope - its chips, its timeline and its tiles all count the same job - which is why -opening a panel closes the one that was open. **Every panel may be closed**: clicking the open -header collapses it, and an accordion with nothing open is the batch read as an index of its -jobs. - -The panel open on arrival is **the first job with work left** - frames still unannotated -or only pre-labeled - and the first job otherwise, so landing on a batch lands on -something to do rather than on a job somebody has finished. Nothing is remembered across -reloads: the rule recomputes from counts that are read anyway, and a remembered panel is -stale the moment somebody else works the job. - -**A collapsed header is the overview**, so a job is picked without opening it: ordinal, -frame count, state, `A of F annotated`, who is working it, and a thin progress bar. The -accordion is rendered only once jobs exist, the same `showsProgress` gate the progress bar -above it uses, so a draft needs no empty state of its own. The assignee is a plain editable -name, not an account: `JobService.assign` gates on nothing, so the control is always -live, and clearing it is the same operation with `null`. A failed read shows its -error instead of the accordion silently vanishing - an empty list and a failed one look -identical to the naive `undefined`-or-zero-items check, and only one of them means -there is nothing to assign. - -**The open panel holds, in order:** the way into the annotator, `Pre-label` and the -assignee; the segment chips, counting *that job* from `GET /jobs/{id}/progress` rather -than the batch; the order select; that job's timeline; and only that job's frames, from -`GET /batches/{id}/assets?job=`. Frame numbers stay batch-wide, because a frame's number -is its place in the batch and renumbering per job would give one picture two names. - -**Thumbnail size is one setting and is rendered outside the panels** - it is a property -of how a grid is read rather than of a job, and it is the same persisted preference -either way. It sits on the batch's own progress row in the header, right of the progress bar -and on the line of its `A of F annotated` readout: the last row above the accordion that is -about the batch rather than about one job. A draft, which has no progress row, keeps it in the -toolbar over its flat grid. The segment filter is the opposite: it belongs to the panel and **resets to -`All` when the open job changes**, because a filter carried across shows an empty panel -for a job with nothing in that state, which reads as a job with no frames. - -**A draft batch has no accordion** - one flat grid, with the membership tools and -selection over it - because it has no jobs, and there is nothing to partition its frames -by. - -Each header is a `