docs(plans): object-form field grouping — 2-task plan (#45)
This commit is contained in:
@@ -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: `<fieldset className="space-y-3 border-t pt-3"><legend className="label-caption">{t("form.flexibleHeading")}</legend>{definitions.map((def) => <div key={def.key}><FieldInput definition={def} form={form} />{errors.fields?.[def.key] && <p role="alert" className="text-xs text-destructive">{errors.fields[def.key]?.message ?? t("form.required")}</p>}</div>)}</fieldset>`.
|
||||||
|
- 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 `<fieldset>` body:
|
||||||
|
```tsx
|
||||||
|
{definitions && definitions.length > 0 && (
|
||||||
|
<fieldset className="space-y-3 border-t pt-3">
|
||||||
|
<legend className="sr-only">{t("form.flexibleHeading")}</legend>
|
||||||
|
{groupDefinitions(definitions, t("fields.other")).map((g) => (
|
||||||
|
<div key={g.group} className="space-y-3">
|
||||||
|
<div className="label-caption">{g.group}</div>
|
||||||
|
{g.defs.map((def) => (
|
||||||
|
<div key={def.key}>
|
||||||
|
<FieldInput definition={def} form={form} />
|
||||||
|
{errors.fields?.[def.key] && (
|
||||||
|
<p role="alert" className="text-xs text-destructive">
|
||||||
|
{errors.fields[def.key]?.message ?? t("form.required")}
|
||||||
|
</p>
|
||||||
|
)}
|
||||||
|
</div>
|
||||||
|
))}
|
||||||
|
</div>
|
||||||
|
))}
|
||||||
|
</fieldset>
|
||||||
|
)}
|
||||||
|
```
|
||||||
|
Change ONLY this block: the `<legend>` 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 `<ObjectForm mode="create" …>` and asserts the group subheadings + membership:
|
||||||
|
```tsx
|
||||||
|
test("groups flexible fields by definition group with subheadings", async () => {
|
||||||
|
renderApp(<ObjectForm mode="create" onSubmit={() => {}} 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).
|
||||||
Reference in New Issue
Block a user