feat(editor): manage a container's children in the owner's form, saved with it #261

Merged

dominik.polakovics merged 3 commits from afk/259 into main 2026-10-13 13:04:53 +02:00

Conversation 3 Commits 3 Files changed 38 +2980 -254

dominik.polakovics commented 2026-10-13 12:06:27 +02:00

Owner

Copy link

Closes #259

What

A container: true element's form (e.g. accordion → accordion-item) gets a children section where an items collection sits, using the same row shape. Each row has: grip, icon, served summary, #uid, Disabled chip, then up/down, eye, pencil and trash.

• Pending until Save: reorder (drag or arrows), enable/disable and remove. Rows show "Moved", and a removed row is struck through with a Keep (undo) button. Dirty tracking, Discard and the unsaved-changes guard all see these changes.
• Pencil: a real <a href> to the child's edit page, with return set to this form. A plain click goes through the router, so the guard runs first.
• Add is immediate: it runs the guard, then opens the ElementPicker filtered for this parent the way the overview's nested insert row filters it. It then calls editor.insert at the end of the container and navigates to the new child, returning here.
• Connected translation: the rows are read-only (no grip, arrows, eye, trash or Add) and show a sync note. The pencil still works. A free translation edits like a source.
• Items owners: behaviour unchanged. Their rows now share the row pieces in FormRows.tsx.
• The "no content" General-tab note is no longer shown for a container.

Server

• POST /api/page/save takes an optional children: {order: [{uid, enabled}], removed: [uid]}. If absent, the children are untouched.
  • All validation runs before any mutation. Sending it for a non-container is a 400 children.notContainer. On a connected translation it is a 409 trans.connectedStructure.
  • If the uids don't match the current element-role children exactly (unknown, duplicate or missing), the save gets a 422 field error on children (children.stale) in the same body as any param errors. Nothing is written, params included.
  • The pure logic is in internal/elemops/children.go, applied after ApplyEdit on the same document. Removal merges text with content.MergeText, and reordering uses the new content.PermuteShortcodes. Enabled state goes through SetShortcodeEnabled.
  • Then: one serialize, one write, syncTranslations (its repair propagates reorders and removals to connected translations; enabled state stays per language), and one snapshot, so a save is one undo step.
• POST /api/form describes a container with children: {label}. The label belongs to the one element placement allows inside, or is empty, in which case the client shows a generic heading.

One owner per rule

• content.PermuteShortcodes is now the single in-place sibling reorder; l10n's orderList and orderSlots call it.
• elemops.RoleAt replaces the hand walk in itemAt.
• elemops.InsertRefusal combines the hidden and insertable checks (insert and the children label both use it).
• routing.elementEditRoute and routing.isPlainClick replace hand-built copies. ElementCard's copy was also missing the altKey/defaultPrevented check.
• Rows read the served role/summary, the same fields the overview sub-list reads.

Docs and skills

• scaffold/site/AGENTS.md: the Containers paragraph describes the section, the Translations paragraph covers the connected read-only rule, and the "no content" note now excludes containers.
• Skills: add-element (Children bullet and verify step), edit-content, verify-site.
• preview-site, redo-theme, add-page-type, create-feature and remove-element were checked and need no change.

Verification

• go vet ./... and go test ./... pass, including new tests in internal/server/children_test.go, internal/elemops/children_test.go and TestPermuteShortcodes.
  • The server tests cover: reorder, disable and remove together with a param change; canonical round-trip; byte-identical refusals; the non-container 400; the connected 409; a free translation; propagation to a connected translation with one undo step restoring both files; and the form description.
• cd web && npx tsc --noEmit -p . && npm test: 3148/3148 pass, 23 of them new (ElementFormChildren.test.tsx, FormScreenChildren.test.tsx). npm run build passes.
• On a fresh harness init site, harness check is clean. Smoke test with harness serve on a hand-written accordion:
  • /api/form returns children.label = "Accordion item".
  • A stale save with a param change gets a 422 on children, and the file is byte-identical.
  • A real save reorders, disables and removes in one "save" commit.
• Not done: a manual check in a browser.

Notes

• A removal leaves the same double blank line between the remaining children that /api/element/delete already leaves. That is existing engine behaviour, unchanged here.
• FormScreen now fetches /api/pagetypes on mount, for the picker's page-type rule. It could be made lazy if the extra request matters.

🤖 Generated with Claude Code

Closes #259 ## What A `container: true` element's form (e.g. `accordion` → `accordion-item`) gets a **children section** where an items collection sits, using the same row shape. Each row has: grip, icon, served summary, `#uid`, Disabled chip, then up/down, eye, pencil and trash. - **Pending until Save:** reorder (drag or arrows), enable/disable and remove. Rows show "Moved", and a removed row is struck through with a Keep (undo) button. Dirty tracking, Discard and the unsaved-changes guard all see these changes. - **Pencil:** a real `<a href>` to the child's edit page, with `return` set to this form. A plain click goes through the router, so the guard runs first. - **Add is immediate:** it runs the guard, then opens the `ElementPicker` filtered for this parent the way the overview's nested insert row filters it. It then calls `editor.insert` at the end of the container and navigates to the new child, returning here. - **Connected translation:** the rows are read-only (no grip, arrows, eye, trash or Add) and show a sync note. The pencil still works. A free translation edits like a source. - **Items owners:** behaviour unchanged. Their rows now share the row pieces in `FormRows.tsx`. - The "no content" General-tab note is no longer shown for a container. ## Server - **`POST /api/page/save`** takes an optional `children: {order: [{uid, enabled}], removed: [uid]}`. If absent, the children are untouched. - All validation runs before any mutation. Sending it for a non-container is a 400 `children.notContainer`. On a connected translation it is a 409 `trans.connectedStructure`. - If the uids don't match the current element-role children exactly (unknown, duplicate or missing), the save gets a 422 field error on `children` (`children.stale`) in the same body as any param errors. Nothing is written, params included. - The pure logic is in `internal/elemops/children.go`, applied after `ApplyEdit` on the same document. Removal merges text with `content.MergeText`, and reordering uses the new `content.PermuteShortcodes`. Enabled state goes through `SetShortcodeEnabled`. - Then: one serialize, one write, `syncTranslations` (its repair propagates reorders and removals to connected translations; enabled state stays per language), and one snapshot, so a save is one undo step. - **`POST /api/form`** describes a container with `children: {label}`. The label belongs to the one element placement allows inside, or is empty, in which case the client shows a generic heading. ## One owner per rule - `content.PermuteShortcodes` is now the single in-place sibling reorder; `l10n`'s `orderList` and `orderSlots` call it. - `elemops.RoleAt` replaces the hand walk in `itemAt`. - `elemops.InsertRefusal` combines the hidden and insertable checks (insert and the children label both use it). - `routing.elementEditRoute` and `routing.isPlainClick` replace hand-built copies. ElementCard's copy was also missing the `altKey`/`defaultPrevented` check. - Rows read the served `role`/`summary`, the same fields the overview sub-list reads. ## Docs and skills - `scaffold/site/AGENTS.md`: the Containers paragraph describes the section, the Translations paragraph covers the connected read-only rule, and the "no content" note now excludes containers. - Skills: `add-element` (Children bullet and verify step), `edit-content`, `verify-site`. - `preview-site`, `redo-theme`, `add-page-type`, `create-feature` and `remove-element` were checked and need no change. ## Verification - `go vet ./...` and `go test ./...` pass, including new tests in `internal/server/children_test.go`, `internal/elemops/children_test.go` and `TestPermuteShortcodes`. - The server tests cover: reorder, disable and remove together with a param change; canonical round-trip; byte-identical refusals; the non-container 400; the connected 409; a free translation; propagation to a connected translation with one undo step restoring both files; and the form description. - `cd web && npx tsc --noEmit -p . && npm test`: 3148/3148 pass, 23 of them new (`ElementFormChildren.test.tsx`, `FormScreenChildren.test.tsx`). `npm run build` passes. - On a fresh `harness init` site, `harness check` is clean. Smoke test with `harness serve` on a hand-written accordion: - `/api/form` returns `children.label = "Accordion item"`. - A stale save with a param change gets a 422 on `children`, and the file is byte-identical. - A real save reorders, disables and removes in one "save" commit. - Not done: a manual check in a browser. ## Notes - A removal leaves the same double blank line between the remaining children that `/api/element/delete` already leaves. That is existing engine behaviour, unchanged here. - `FormScreen` now fetches `/api/pagetypes` on mount, for the picker's page-type rule. It could be made lazy if the extra request matters. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

dominik.polakovics added 1 commit 2026-10-13 12:06:27 +02:00

feat(editor): manage a container's children in the owner's form, saved with it ... ee96b1cc4e

A container element's form (accordion → accordion-item) gains a children
section with the items collection's row shape: grip, icon, served summary,
#uid, Disabled chip, up/down, eye, pencil (a real link to the child's form
with a return here) and trash can. Reorder, enable/disable and remove are
pending until Save; Add is immediate and opens the placement-filtered insert
picker, then the new child's form. In a connected translation the rows are
read-only apart from the pencil.

POST /api/page/save takes an optional `children: {order, removed}` applied
as the element operations it stands for (delete, move within the
container, enable/disable) on the same document as the params: validated
before anything is applied (a stale set is a 422 field error on
"children", nothing written), one write, translation repair, one snapshot.
POST /api/form describes a container with `children: {label}`.

One owner per rule: content.PermuteShortcodes is the single in-place
sibling reorder (l10n's two copies call it), elemops.RoleAt replaces
itemAt's walk, elemops.InsertRefusal combines the hidden/insertable
checks, routing.elementEditRoute and isPlainClick replace hand-built
copies, and the items rows share their row pieces with the new section
(FormRows).

AGENTS.md and the add-element, edit-content and verify-site skills
describe the children section.

Closes #259

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

dominik.polakovics added 1 commit 2026-10-13 12:13:31 +02:00

Merge origin/main into afk/259 ... 56a85780f3

Resolve the conflicts with #258 (sidecar label translations) as unions:
README's /api/form shape gains both `children?` and the field `i18n?`,
forms.go imports both pagetype and sidecardir, and the add-element
skill's verification keeps both the container children-section check and
the per-language label check.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

dominik.polakovics commented 2026-10-13 12:16:51 +02:00

Author

Owner

Copy link

[autoland] verdict: pass

Verdict: CONCERNS (non-blocking), approved, not auto-merged

What I relied on: CI ci / build-and-test came back green on both the original head ee96b1c (run 262) and the new head 56a8578 (run 263). On the merged tree I also ran go vet ./... and go test ./... (all packages pass), npx tsc --noEmit -p ., npm test (3166/3166 in 123 files), npm run build, and harness check on a fresh harness init site, which was clean.

Conventions: the title is a Conventional Commit, feat(editor): …. The body has Closes #259. The diff stays within the issue's scope. It touches scaffold/site/AGENTS.md (Containers, Translations, the "no content" note) and the add-element, edit-content and verify-site skills, as CLAUDE.md requires. The one-owner refactors also follow CLAUDE.md "Every rule has exactly one owner": content.PermuteShortcodes, elemops.RoleAt, elemops.InsertRefusal, routing.elementEditRoute and routing.isPlainClick.

Merge conflict: main had moved on (#258, sidecar label translations). Three files conflicted, and I merged origin/main into afk/259 as 56a8578 (merge, no force-push). All three were unions of separate additions:

• README.md: the /api/form shape now has both children? and the field's i18n?.
• internal/server/forms.go: imports both pagetype and sidecardir; both are used.
• scaffold/site/.claude/skills/add-element/SKILL.md: the verify step keeps both the container children-section check and the per-language label check.

Concern: the children-section heading skips #258's label translation

This comes from #258 and this PR landing at about the same time. Since #258, every sidecar label the client shows goes through web/src/api/labels.ts (the i18n map from <name>.<lang>.yaml). The items description got i18n and itemI18n for this. The new children description did not:

• internal/server/forms.go:149 (childrenDescription.Label) and :690 (return only.Label) send the raw sidecar label with no i18n.
• web/src/api/forms.ts:158 (the decoder) resolves items labels but never children.
• web/src/components/ChildrenSection.tsx:272 (heading) and :470 (Add button) therefore show the untranslated label.

What a user sees: on the shipped scaffold with the editor in German, the accordion form's children heading reads "Accordion item" and the Add button "Accordion item hinzufügen", where both should say "Akkordeon-Eintrag" (scaffold/editor/elements/accordion-item.de.yaml). The rows and the insert picker are translated, because they go through elementDisplay. The suggested fix is small:

• add I18n sidecardir.I18n \json:"i18n,omitempty"`tochildrenDescription, set from only.I18n.Labels()`;
• call resolveLabel(desc.children) in the forms.ts decoder;
• add a test for both.

The feature itself works and is well covered. I'm leaving the merge to a human because of this concern.

[autoland] verdict: pass ## Verdict: CONCERNS (non-blocking), approved, not auto-merged **What I relied on:** CI `ci / build-and-test` came back green on both the original head ee96b1c (run 262) and the new head 56a8578 (run 263). On the merged tree I also ran `go vet ./...` and `go test ./...` (all packages pass), `npx tsc --noEmit -p .`, `npm test` (3166/3166 in 123 files), `npm run build`, and `harness check` on a fresh `harness init` site, which was clean. **Conventions:** the title is a Conventional Commit, `feat(editor): …`. The body has `Closes #259`. The diff stays within the issue's scope. It touches `scaffold/site/AGENTS.md` (Containers, Translations, the "no content" note) and the `add-element`, `edit-content` and `verify-site` skills, as CLAUDE.md requires. The one-owner refactors also follow CLAUDE.md "Every rule has exactly one owner": `content.PermuteShortcodes`, `elemops.RoleAt`, `elemops.InsertRefusal`, `routing.elementEditRoute` and `routing.isPlainClick`. **Merge conflict:** main had moved on (#258, sidecar label translations). Three files conflicted, and I merged origin/main into afk/259 as 56a8578 (merge, no force-push). All three were unions of separate additions: - `README.md`: the `/api/form` shape now has both `children?` and the field's `i18n?`. - `internal/server/forms.go`: imports both `pagetype` and `sidecardir`; both are used. - `scaffold/site/.claude/skills/add-element/SKILL.md`: the verify step keeps both the container children-section check and the per-language label check. ### Concern: the children-section heading skips #258's label translation This comes from #258 and this PR landing at about the same time. Since #258, every sidecar label the client shows goes through `web/src/api/labels.ts` (the `i18n` map from `<name>.<lang>.yaml`). The items description got `i18n` and `itemI18n` for this. The new children description did not: - `internal/server/forms.go:149` (`childrenDescription.Label`) and `:690` (`return only.Label`) send the raw sidecar label with no `i18n`. - `web/src/api/forms.ts:158` (the decoder) resolves `items` labels but never `children`. - `web/src/components/ChildrenSection.tsx:272` (heading) and `:470` (Add button) therefore show the untranslated label. What a user sees: on the shipped scaffold with the editor in German, the accordion form's children heading reads "Accordion item" and the Add button "Accordion item hinzufügen", where both should say "Akkordeon-Eintrag" (`scaffold/editor/elements/accordion-item.de.yaml`). The rows and the insert picker are translated, because they go through `elementDisplay`. The suggested fix is small: - add `I18n sidecardir.I18n \`json:"i18n,omitempty"\`` to `childrenDescription`, set from `only.I18n.Labels()`; - call `resolveLabel(desc.children)` in the `forms.ts` decoder; - add a test for both. The feature itself works and is well covered. I'm leaving the merge to a human because of this concern.

dominik.polakovics added 1 commit 2026-10-13 12:36:11 +02:00

fix(editor): translate the children-section heading through the child's language file ... 4c8ebae51c

The children description (#259) sent the one insertable child element's
raw sidecar label with no i18n, so with the editor in German the accordion
form's heading and Add button read "Accordion item" while the rows and the
picker, which go through elementDisplay, read "Akkordeon-Eintrag" (#258).

childrenDescription carries the child element's label translations as
i18n (its I18n.Labels(), like itemI18n), and the forms.ts decoder resolves
the children heading with resolveLabel exactly as it resolves the items
collection's labels. childrenLabel becomes childElement, returning the
schema, so label and translations come from the one place. README's
/api/form shape names the new key; tests on both sides.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

dominik.polakovics commented 2026-10-13 12:36:19 +02:00

Author

Owner

Copy link

Fixed the label-translation concern in 4c8ebae.

• childrenDescription now carries I18n sidecardir.I18n \json:"i18n,omitempty"`, set from the child element's I18n.Labels(). childrenLabelbecamechildElement` and returns the schema, so label and translations come from one place.
• The forms.ts decoder calls resolveLabel(desc.children), and ChildrenDescription declares i18n?.
• Tests: TestFormDescribeChildren adds an accordion-item.de.yaml and expects {"label":"Accordion item","i18n":{"de":{"label":"Akkordeon-Eintrag"}}}; labels.test.ts checks the heading resolves to the German label, keeps the written one without a translation, and keeps "" for the generic heading.
• README's /api/form shape now reads children: {label, i18n?}.

Verified: go vet ./... and go test ./... pass; npx tsc --noEmit -p ., npm test (3167/3167) and npm run build pass.

Fixed the label-translation concern in 4c8ebae. - `childrenDescription` now carries `I18n sidecardir.I18n \`json:"i18n,omitempty"\``, set from the child element's `I18n.Labels()`. `childrenLabel` became `childElement` and returns the schema, so label and translations come from one place. - The `forms.ts` decoder calls `resolveLabel(desc.children)`, and `ChildrenDescription` declares `i18n?`. - Tests: `TestFormDescribeChildren` adds an `accordion-item.de.yaml` and expects `{"label":"Accordion item","i18n":{"de":{"label":"Akkordeon-Eintrag"}}}`; `labels.test.ts` checks the heading resolves to the German label, keeps the written one without a translation, and keeps `""` for the generic heading. - README's `/api/form` shape now reads `children: {label, i18n?}`. Verified: `go vet ./...` and `go test ./...` pass; `npx tsc --noEmit -p .`, `npm test` (3167/3167) and `npm run build` pass.

dominik.polakovics commented 2026-10-13 12:58:48 +02:00

Author

Owner

Copy link

[autoland] verdict: pass

This was generated by AI while landing a PR.

PASS. Signal relied on: CI ci / build-and-test green on run 264, the run for the current head 4c8ebae (runs 262 and 263 covered the earlier heads, as the autoland comment records). Nothing re-run.

• Conventions: title is a Conventional Commit; body carries Closes #259; AGENTS.md and the add-element, edit-content and verify-site skills are updated as CLAUDE.md requires.
• The one autoland concern (children heading skipping the #258 label translation) is fixed in 4c8ebae: childrenDescription carries i18n from the child schema's I18n.Labels(), the forms.ts decoder resolves it with resolveLabel, README names the key, tests on both sides. Reviewed the fix diff in full.
• Spot-checked internal/elemops/children.go: CheckChildren runs entirely before ApplyChildren, so a stale uid set writes nothing, params included; the enabled comparison and the slot move/toggle refusal are correct.
• origin/main is an ancestor of the head: no conflict to resolve.

[autoland] verdict: pass > *This was generated by AI while landing a PR.* **PASS.** Signal relied on: CI `ci / build-and-test` green on run 264, the run for the current head 4c8ebae (runs 262 and 263 covered the earlier heads, as the autoland comment records). Nothing re-run. - Conventions: title is a Conventional Commit; body carries `Closes #259`; AGENTS.md and the add-element, edit-content and verify-site skills are updated as CLAUDE.md requires. - The one autoland concern (children heading skipping the #258 label translation) is fixed in 4c8ebae: `childrenDescription` carries `i18n` from the child schema's `I18n.Labels()`, the forms.ts decoder resolves it with `resolveLabel`, README names the key, tests on both sides. Reviewed the fix diff in full. - Spot-checked `internal/elemops/children.go`: `CheckChildren` runs entirely before `ApplyChildren`, so a stale uid set writes nothing, params included; the enabled comparison and the slot move/toggle refusal are correct. - origin/main is an ancestor of the head: no conflict to resolve.

dominik.polakovics merged commit 90f5e81083 into main 2026-10-13 13:04:53 +02:00

dominik.polakovics referenced this pull request from a commit 2026-10-13 13:04:55 +02:00

Merge pull request 'feat(editor): manage a container's children in the owner's form, saved with it' (#261) from afk/259 into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

bug

Something is broken

  

enhancement

New feature or improvement

  

needs-triage

Maintainer needs to evaluate this issue

  

ready-for-agent

Fully specified, ready for an AFK agent

  

ready-for-human

Requires human implementation

No labels

bug

enhancement

needs-triage

ready-for-agent

ready-for-human

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Cloonar/deckle!261

No description provided.