diff --git a/docs/superpowers/plans/2026-06-08-form-field-grouping.md b/docs/superpowers/plans/2026-06-08-form-field-grouping.md new file mode 100644 index 0000000..4178865 --- /dev/null +++ b/docs/superpowers/plans/2026-06-08-form-field-grouping.md @@ -0,0 +1,191 @@ +# Object-Form Flexible-Field Grouping — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Group the object form's flexible-field inputs by `def.group` (definition order, "Other" last) with subheadings, reusing one shared helper so the form and the detail view group identically. + +**Architecture:** Extract the detail view's defs-grouping into `lib/group-fields.ts` (`groupDefinitions`), unit-test it, refactor `object-detail.tsx` to use it (output-preserving), then render the form's flexible block grouped via the same helper. + +**Tech Stack:** React 19 + TS + pnpm, react-hook-form, react-i18next, Vitest + RTL. Test runner: `pnpm test` (single pass). + +**Conventions:** pnpm; **no `any`/`eslint-disable`/`@ts-ignore`**; no codename; en/sv parity (no new keys); app source double-quote+semicolon; token classes only; no behavior change to the form (inputs/validation/submission). + +**Spec:** `docs/superpowers/specs/2026-06-08-form-field-grouping-design.md` + +**Key facts:** +- `object-detail.tsx` builds groups inline (`const other = t("fields.other"); const present = (definitions ?? []).filter(d => object.fields[d.key] != null); const groups: { group, defs }[] = []; for (const def of present) { … } ` + orphan push + `groups.sort((a,b) => Number(a.group===other) - Number(b.group===other));`). Type alias `FieldDefinitionView = components["schemas"]["FieldDefinitionView"]` already imported. +- `object-form.tsx` flexible block: `
{t("form.flexibleHeading")}{definitions.map((def) =>
{errors.fields?.[def.key] &&

{errors.fields[def.key]?.message ?? t("form.required")}

}
)}
`. +- Field-def fixtures have `group: "Description"` (inscription) and `group: null` (the rest). +- `sr-only` is a valid utility (used in the #52 skip link). `fields.other` + `form.flexibleHeading` are existing i18n keys. + +--- + +# Task 1: Shared `groupDefinitions` helper + unit test + detail refactor + +**Files:** `web/src/lib/group-fields.ts` (new), `web/src/lib/group-fields.test.ts` (new), `web/src/objects/object-detail.tsx`. + +- [ ] **Step 1: `web/src/lib/group-fields.ts`:** +```ts +import type { components } from "../api/schema"; + +type FieldDefinitionView = components["schemas"]["FieldDefinitionView"]; + +export type FieldGroup = { group: string; defs: FieldDefinitionView[] }; + +/** Group field definitions by `def.group` (trimmed), preserving definition order + * within and across groups; ungrouped defs fall into a trailing `otherLabel` bucket. */ +export function groupDefinitions( + definitions: FieldDefinitionView[], + otherLabel: string, +): FieldGroup[] { + const groups: FieldGroup[] = []; + for (const def of definitions) { + const group = def.group?.trim() ? def.group : otherLabel; + let bucket = groups.find((g) => g.group === group); + if (!bucket) { + bucket = { group, defs: [] }; + groups.push(bucket); + } + bucket.defs.push(def); + } + groups.sort((a, b) => Number(a.group === otherLabel) - Number(b.group === otherLabel)); + return groups; +} +``` + +- [ ] **Step 2: `web/src/lib/group-fields.test.ts`** (write + run, must pass): +```ts +import { expect, test } from "vitest"; +import { groupDefinitions } from "./group-fields"; + +type Def = { key: string; group?: string | null }; +const def = (key: string, group: string | null): Def => ({ key, group }); + +function keysByGroup(defs: Def[]) { + // cast through unknown — the helper only reads key/group + return groupDefinitions(defs as never, "Other").map((g) => ({ + group: g.group, + keys: g.defs.map((d) => (d as unknown as Def).key), + })); +} + +test("preserves definition order within and across groups; Other is last", () => { + const result = keysByGroup([ + def("a", "Description"), + def("b", null), + def("c", "Description"), + def("d", "Provenance"), + def("e", " "), + ]); + expect(result).toEqual([ + { group: "Description", keys: ["a", "c"] }, + { group: "Provenance", keys: ["d"] }, + { group: "Other", keys: ["b", "e"] }, + ]); +}); + +test("all-ungrouped → a single trailing Other group", () => { + expect(keysByGroup([def("x", null), def("y", null)])).toEqual([ + { group: "Other", keys: ["x", "y"] }, + ]); +}); +``` +Run: `cd web && pnpm vitest run src/lib/group-fields.test.ts` → PASS (2 tests). (If the `as never`/`as unknown` casts trip lint, type the test `def` as the real `FieldDefinitionView` partial via `Partial<…> as …` — keep it lint-clean and `any`-free; the helper only reads `key`/`group`.) + +- [ ] **Step 3: Refactor `object-detail.tsx`** to use the helper. Add `import { groupDefinitions } from "../lib/group-fields";`. Replace the inline group-building loop + the final `groups.sort(...)` with: +```tsx +const other = t("fields.other"); +const present = (definitions ?? []).filter((d) => object.fields[d.key] != null); +const groups = groupDefinitions(present, other); +``` +Keep the orphan handling exactly as-is AFTER this (`const definedKeys = …; const orphans = …; if (orphans.length > 0 && !groups.some((g) => g.group === other)) groups.push({ group: other, defs: [] });`). The appended Other bucket remains last (the helper already put any Other last, and appending when absent adds it at the end). Do NOT re-add a `groups.sort(...)` — appending keeps Other last. The render (`groups.map(...)`) is unchanged. + +- [ ] **Step 4: Verify (vitest ONCE):** `cd web && pnpm vitest run src/lib/group-fields.test.ts src/objects/object-detail.test.tsx && pnpm typecheck && pnpm lint`. PASS — the object-detail tests must stay green (output-preserving refactor). + +- [ ] **Step 5: Commit** +```bash +git add web/src/lib/group-fields.ts web/src/lib/group-fields.test.ts web/src/objects/object-detail.tsx +git commit -m "refactor(web): extract groupDefinitions helper; object-detail uses it (#45)" +``` + +--- + +# Task 2: Group the object-form flexible inputs + test + gate + +**Files:** `web/src/objects/object-form.tsx`, `web/src/objects/object-form.test.tsx`. + +- [ ] **Step 1: Group the flexible block** in `object-form.tsx`. Add `import { groupDefinitions } from "../lib/group-fields";`. Replace the flexible `
` body: +```tsx +{definitions && definitions.length > 0 && ( +
+ {t("form.flexibleHeading")} + {groupDefinitions(definitions, t("fields.other")).map((g) => ( +
+
{g.group}
+ {g.defs.map((def) => ( +
+ + {errors.fields?.[def.key] && ( +

+ {errors.fields[def.key]?.message ?? t("form.required")} +

+ )} +
+ ))} +
+ ))} +
+)} +``` + Change ONLY this block: the `` goes from visible `label-caption` to `sr-only`; the flat `definitions.map` becomes grouped. Field inputs + error markup are identical, just nested under group wrappers. No change anywhere else (core fields, visibility, footer, submit logic). + +- [ ] **Step 2: Test** — extend `object-form.test.tsx`. The field-def fixtures have `inscription` in group `"Description"` and the rest ungrouped → "Other". Add a test that renders `` and asserts the group subheadings + membership: +```tsx +test("groups flexible fields by definition group with subheadings", async () => { + renderApp( {}} onCancel={() => {}} />); + // the "Description" group heading and the "Other" group heading both render + expect(await screen.findByText("Description")).toBeInTheDocument(); + expect(screen.getByText(/^Other$/)).toBeInTheDocument(); + // the Description-grouped field input is present (Inscription) and appears before an ungrouped one (Material) + const inscription = screen.getByLabelText(/inscription/i); + const material = screen.getByLabelText(/material/i); + expect(inscription.compareDocumentPosition(material) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); +}); +``` + (Adjust field labels to the fixtures' actual rendered labels — read `web/src/test/fixtures.ts` `fieldDefinitions` for the exact `labels`/keys and `web/src/objects/field-input.tsx` for how each renders its label, so the `getByLabelText`/`findByText` queries match. The key assertion: a named group heading + the "Other" heading both appear, and a grouped field precedes an ungrouped one in the DOM.) Keep the existing object-form tests green. + +- [ ] **Step 3: FULL GATE (run tests EXACTLY ONCE):** +```bash +cd web && pnpm typecheck && pnpm lint && pnpm test && pnpm build && pnpm check:size && pnpm check:colors +``` +All green. Report test totals, largest chunk, check:colors line. + +- [ ] **Step 4: Codename + status:** +```bash +cd /Users/olsson/Laboratory/biggus-dickus +git grep -in 'biggus\|dickus' -- web/src; echo "codename-exit=$?" +git status --short +``` + +- [ ] **Step 5: Manual smoke (recommended).** `pnpm dev`: open New object / edit — the flexible fields now appear under group subheadings (e.g. "Description", "Other") matching the detail view's grouping; inputs/validation/submit still work. + +- [ ] **Step 6: Commit** +```bash +git add web/src/objects/object-form.tsx web/src/objects/object-form.test.tsx +git commit -m "feat(web): group object-form flexible fields by definition group (#45)" +``` + +--- + +## Self-Review (completed) + +**Spec coverage:** shared `groupDefinitions` + unit test (T1 S1–S2); detail refactor output-preserving (T1 S3–S4); form grouped via the helper with `sr-only` legend + visible subheadings (T2 S1); form grouping test (T2 S2); gate (T2 S3). Acceptance criteria 1–5 mapped. ✓ + +**Placeholder scan:** the form test says "adjust labels to the fixtures' actual rendered labels" with the files named (fixtures.ts, field-input.tsx) — a concrete match-the-data step, not a TODO; the core assertion (named + Other headings, order) is explicit. The helper-test cast note keeps it `any`-free. No vague steps. ✓ + +**Type/consistency:** `groupDefinitions(defs, otherLabel): FieldGroup[]` defined in T1, consumed by detail (`present`) and form (all `definitions`); detail's orphan-Other append stays last; the form reuses the existing `FieldInput`/error markup unchanged. ✓ + +## Notes +- No new dependency; no new i18n keys (`fields.other` + `form.flexibleHeading` exist). +- The refactor of object-detail is output-preserving — its tests are the guard. +- Field-list's A–Z grouping is intentionally NOT unified (different purpose). diff --git a/docs/superpowers/specs/2026-06-08-form-field-grouping-design.md b/docs/superpowers/specs/2026-06-08-form-field-grouping-design.md new file mode 100644 index 0000000..d31be7e --- /dev/null +++ b/docs/superpowers/specs/2026-06-08-form-field-grouping-design.md @@ -0,0 +1,136 @@ +# Object-Form Flexible-Field Grouping — Design + +**Date:** 2026-06-08 +**Status:** Approved (brainstorming) — ready for implementation planning. +**Issue:** #45 (deferred half — the object **form**'s flexible-field grouping; the detail view shipped in `e2ae093`). + +## Context + +#45 fixed the object **detail** view: flexible fields are now grouped by `FieldDefinitionView.group` +in definition order with a trailing "Other" bucket (`object-detail.tsx`). The issue's problem #2 +explicitly says "detail **and** form", but the **form** still renders flexible-field inputs flat — one +`
` with a single "Catalogue fields" legend and `definitions.map(...)` (definition order, no +group subheadings). This milestone brings the form to parity by reusing the *same* grouping the detail +view uses (extracted into a shared helper so they can't drift). + +**Facts:** `object-detail.tsx:71-94` builds `{ group, defs }[]` inline: iterate the definitions +(filtered to those with a value), bucket by `def.group?.trim()` else `t("fields.other")`, sort so the +"Other" group is last, plus orphan-key handling. `object-form.tsx` renders the flexible block as +`
{form.flexibleHeading}{definitions.map((def) =>
{error}
)}
`. +Field-definition fixtures carry `group` (e.g. `"Description"` / `null`). `field-list.tsx` also groups, +but A–Z (a management list, from #50) — out of scope to unify with it. + +### Decisions (from brainstorming) +1. Extract the definition-grouping into a shared `lib/group-fields.ts` and use it in **both** detail + and form (one source of truth). +2. The form keeps its `
` for a11y but with an `sr-only` legend; visible per-group + `label-caption` subheadings (matching the detail view) — no redundant double-heading. +3. Definition order within groups (no re-sort); ungrouped → trailing "Other". + +## Components + +### `web/src/lib/group-fields.ts` (new) +```ts +import type { components } from "../api/schema"; + +type FieldDefinitionView = components["schemas"]["FieldDefinitionView"]; + +export type FieldGroup = { group: string; defs: FieldDefinitionView[] }; + +/** Group field definitions by `def.group` (trimmed), preserving definition order + * within and across groups; ungrouped defs fall into a trailing `otherLabel` bucket. */ +export function groupDefinitions( + definitions: FieldDefinitionView[], + otherLabel: string, +): FieldGroup[] { + const groups: FieldGroup[] = []; + for (const def of definitions) { + const group = def.group?.trim() ? def.group : otherLabel; + let bucket = groups.find((g) => g.group === group); + if (!bucket) { + bucket = { group, defs: [] }; + groups.push(bucket); + } + bucket.defs.push(def); + } + // Keep definition order among real groups; the "Other" bucket sorts last. + groups.sort((a, b) => Number(a.group === otherLabel) - Number(b.group === otherLabel)); + return groups; +} +``` +A unit test asserts: definition order preserved; two defs in the same group bucket together; ungrouped +defs land in the `otherLabel` bucket which is **last** even when defined before a grouped def. + +### `object-detail.tsx` (refactor to the shared helper) +Replace the inline `for (const def of present) { … }` group-building + final `groups.sort(...)` with +`const groups = groupDefinitions(present, other);`. Keep the existing orphan handling appended after +(`if (orphans.length > 0 && !groups.some(g => g.group === other)) groups.push({ group: other, defs: [] });` +— appending the Other bucket still leaves it last). Render is unchanged. Net: identical output, now via +the shared helper. (Verify the existing object-detail tests stay green.) + +### `object-form.tsx` (group the flexible inputs) +Replace the flat flexible block with grouped rendering, reusing the FieldInput + per-field error markup +inside each group: +```tsx +{definitions && definitions.length > 0 && ( +
+ {t("form.flexibleHeading")} + {groupDefinitions(definitions, t("fields.other")).map((g) => ( +
+
{g.group}
+ {g.defs.map((def) => ( +
+ + {errors.fields?.[def.key] && ( +

+ {errors.fields[def.key]?.message ?? t("form.required")} +

+ )} +
+ ))} +
+ ))} +
+)} +``` +- The `` becomes `sr-only` (a11y-correct fieldset name) while the visible structure is the + per-group `label-caption` subheadings — mirroring detail, no double-heading. +- Field inputs and their error rendering are unchanged (just moved under group wrappers); the submit + payload / validation / pruning are untouched (the same `definitions` drive the same inputs). +- `import { groupDefinitions } from "../lib/group-fields";` (and `fields.other` is an existing key). + +## Data flow +`useFieldDefinitions()` → `groupDefinitions(defs, t("fields.other"))` → grouped subheadings + the same +`FieldInput`s. No change to form state, RHF registration, pruning, or submission. + +## Error handling / edges +- Definition order preserved; "Other" last (even if an ungrouped def precedes a grouped one). +- All-ungrouped case → a single "Other" group with a visible "Other" subheading (consistent with the + detail view's behavior). +- The shared helper takes the caller's def list: detail passes `present` (defs with values); the form + passes all `definitions`. Orphan handling stays in detail only. +- `sr-only` legend keeps the fieldset accessible without a visible redundant heading. + +## Testing +- **`group-fields.test.ts`** (unit): order preserved; same-group bucketing; ungrouped → trailing Other. +- **object-form test:** with the field-definition fixtures (which include `group` values + ungrouped), + rendering the form shows the grouped subheadings (e.g. "Description" then "Other") and the fields + under them in definition order. (Extend `object-form.test.tsx`; reuse the existing MSW field-def + handler / fixtures — add a `group` to one fixture def if needed to exercise a real named group.) +- **object-detail tests** stay green (the refactor is output-preserving). +- Gate: `typecheck`/`lint`/`test`/`build`/`check:size`/`check:colors`; no new i18n keys (reuse + `fields.other`, `form.flexibleHeading`); en/sv parity unaffected; no codename; no new dependency. + +## Acceptance criteria +1. `groupDefinitions` exists (shared, unit-tested) and is used by **both** object-detail and object-form. +2. The object form renders flexible-field inputs grouped by `def.group` (definition order, "Other" last) + with `label-caption` subheadings; the fieldset keeps an `sr-only` legend. +3. The detail view's grouping is unchanged in output (now via the shared helper). +4. Form behavior is otherwise unchanged (inputs, validation, errors, submission, pruning). +5. `typecheck`/`lint`/`test`/`build`/`check:colors` green; `check:size` reported; en/sv parity; no + codename; no new dependency. + +## Out of scope → follow-ups +- Export/PDF (#39); backend label resolution. +- Reordering fields within a group beyond definition order; a field-definition "position"/sort concept. +- Unifying `field-list.tsx`'s (A–Z) grouping with this (different purpose — a management list). diff --git a/web/src/lib/group-fields.test.ts b/web/src/lib/group-fields.test.ts new file mode 100644 index 0000000..e0c59f6 --- /dev/null +++ b/web/src/lib/group-fields.test.ts @@ -0,0 +1,40 @@ +import { expect, test } from "vitest"; +import { groupDefinitions } from "./group-fields"; +import type { components } from "../api/schema"; + +type FieldDefinitionView = components["schemas"]["FieldDefinitionView"]; +type MinDef = Pick; + +const def = (key: string, group: string | null): MinDef => ({ + key, + group: group as FieldDefinitionView["group"], +}); + +function keysByGroup(defs: MinDef[]) { + return groupDefinitions(defs as FieldDefinitionView[], "Other").map((g) => ({ + group: g.group, + keys: g.defs.map((d) => d.key), + })); +} + +test("preserves definition order within and across groups; Other is last", () => { + const result = keysByGroup([ + def("a", "Description"), + def("b", null), + def("c", "Description"), + def("d", "Provenance"), + def("e", " "), + ]); + + expect(result).toEqual([ + { group: "Description", keys: ["a", "c"] }, + { group: "Provenance", keys: ["d"] }, + { group: "Other", keys: ["b", "e"] }, + ]); +}); + +test("all-ungrouped → a single trailing Other group", () => { + expect(keysByGroup([def("x", null), def("y", null)])).toEqual([ + { group: "Other", keys: ["x", "y"] }, + ]); +}); diff --git a/web/src/lib/group-fields.ts b/web/src/lib/group-fields.ts new file mode 100644 index 0000000..d7fb9f7 --- /dev/null +++ b/web/src/lib/group-fields.ts @@ -0,0 +1,30 @@ +import type { components } from "../api/schema"; + +type FieldDefinitionView = components["schemas"]["FieldDefinitionView"]; + +export type FieldGroup = { group: string; defs: FieldDefinitionView[] }; + +/** Group field definitions by `def.group` (trimmed), preserving definition order + * within and across groups; ungrouped defs fall into a trailing `otherLabel` bucket. */ +export function groupDefinitions( + definitions: FieldDefinitionView[], + otherLabel: string, +): FieldGroup[] { + const groups: FieldGroup[] = []; + + for (const def of definitions) { + const group = def.group?.trim() ? def.group : otherLabel; + let bucket = groups.find((g) => g.group === group); + + if (!bucket) { + bucket = { group, defs: [] }; + groups.push(bucket); + } + + bucket.defs.push(def); + } + + groups.sort((a, b) => Number(a.group === otherLabel) - Number(b.group === otherLabel)); + + return groups; +} diff --git a/web/src/objects/object-detail.tsx b/web/src/objects/object-detail.tsx index 326d48f..de1aef6 100644 --- a/web/src/objects/object-detail.tsx +++ b/web/src/objects/object-detail.tsx @@ -4,6 +4,7 @@ import { useTranslation } from "react-i18next"; import type { components } from "../api/schema"; import { useObject, useFieldDefinitions } from "../api/queries"; +import { groupDefinitions } from "../lib/group-fields"; import { formatDate } from "../lib/format-date"; import { useDocumentTitle } from "../lib/use-document-title"; import { useBreadcrumb } from "../shell/use-breadcrumb"; @@ -14,7 +15,6 @@ import { VisibilityBadge } from "./visibility-badge"; import { buttonVariants } from "@/components/ui/button"; import { Skeleton } from "@/components/ui/skeleton"; -type FieldDefinitionView = components["schemas"]["FieldDefinitionView"]; type AdminObjectView = components["schemas"]["AdminObjectView"]; function Field({ label, value }: { label: string; value: ReactNode }) { @@ -70,17 +70,8 @@ function ObjectDetailLoaded({ object }: { object: AdminObjectView }) { // into a trailing "Other" group. const other = t("fields.other"); const present = (definitions ?? []).filter((d) => object.fields[d.key] != null); - const groups: { group: string; defs: FieldDefinitionView[] }[] = []; - for (const def of present) { - const isOther = !def.group?.trim(); - const group = isOther ? other : def.group!; - let bucket = groups.find((x) => x.group === group); - if (!bucket) { - bucket = { group, defs: [] }; - groups.push(bucket); - } - bucket.defs.push(def); - } + const groups = groupDefinitions(present, other); + // Defensive: a key present in object.fields with no matching definition (e.g. a // definition removed after the value was set). Render it muted under "Other" // rather than silently dropping data; the raw key is the label. @@ -88,10 +79,10 @@ function ObjectDetailLoaded({ object }: { object: AdminObjectView }) { const orphans = Object.entries(object.fields).filter( ([key, value]) => !definedKeys.has(key) && value != null, ); + if (orphans.length > 0 && !groups.some((g) => g.group === other)) { groups.push({ group: other, defs: [] }); } - groups.sort((a, b) => Number(a.group === other) - Number(b.group === other)); return (
diff --git a/web/src/objects/object-form.test.tsx b/web/src/objects/object-form.test.tsx index d12a4c1..b2fad57 100644 --- a/web/src/objects/object-form.test.tsx +++ b/web/src/objects/object-form.test.tsx @@ -82,6 +82,18 @@ test("edit mode: no visibility control, save button, prefilled values", async () expect(screen.getByRole("button", { name: /save/i })).toBeInTheDocument(); }); +test("groups flexible fields by definition group with subheadings", async () => { + renderApp( {}} />); + + expect(await screen.findByText("Description")).toBeInTheDocument(); + expect(screen.getByText(/^Other$/)).toBeInTheDocument(); + + const inscription = screen.getByLabelText(/inscription/i); + const material = screen.getByLabelText(/material/i); + + expect(inscription.compareDocumentPosition(material) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy(); +}); + test("pruneFields: localized_text keeps only the default-language key, other object fields unaffected", () => { const localizedTextKeys = new Set(["title_ml"]); diff --git a/web/src/objects/object-form.tsx b/web/src/objects/object-form.tsx index 0e72c0b..418c11e 100644 --- a/web/src/objects/object-form.tsx +++ b/web/src/objects/object-form.tsx @@ -6,6 +6,7 @@ import { useFieldDefinitions } from "../api/queries"; import { useConfig } from "../config/config-context"; import { FieldInput } from "./field-input"; import { pruneFields } from "./prune-fields"; +import { groupDefinitions } from "../lib/group-fields"; import { UnsavedChangesDialog } from "../lib/unsaved-changes-dialog"; import { useUnsavedChanges } from "../lib/use-unsaved-changes"; import { Button } from "@/components/ui/button"; @@ -197,19 +198,23 @@ export function ObjectForm({ {definitions && definitions.length > 0 && (
- - {t("form.flexibleHeading")} - + {t("form.flexibleHeading")} - {definitions.map((def) => ( -
- + {groupDefinitions(definitions, t("fields.other")).map((g) => ( +
+
{g.group}
- {errors.fields?.[def.key] && ( -

- {errors.fields[def.key]?.message ?? t("form.required")} -

- )} + {g.defs.map((def) => ( +
+ + + {errors.fields?.[def.key] && ( +

+ {errors.fields[def.key]?.message ?? t("form.required")} +

+ )} +
+ ))}
))}