Extract page editor controllers from PagePane/TreePane (FormController pattern) #113

Closed
opened 2026-07-18 14:19:31 +02:00 by dominik.polakovics · 1 comment

This was generated by AI during triage.

Found by an architecture review (deepening opportunities — turning shallow modules into deep ones).

Files: web/src/components/PagePane.tsx (2,569 lines), web/src/components/TreePane.tsx (1,453 lines); contrast web/src/components/FormPane.tsx and its FormController interface.

Problem: the entire page-editing controller (publish/discard/undo/insert/apply-result, 32 drag handlers, optimistic mutate() vs authoritative refetch()) is fused with JSX. Every operation repeats the same busy/error/switch-on-tagged-result/refetch shape; TreePane's followMutation/run wrappers re-implement PagePane's applyResult dance. The only way to test any of it is mounting the whole <App/> and driving the DOM (App.test.tsx, ElementCrud.test.tsx, Publish.test.tsx, LayoutGrid.test.tsx all boot router + tree + elements + git state per assertion).

Solution: extract createPageEditor / createTreeController stores holding the operation logic, leaving the panes as thin presentation — the pattern FormPane already proves: it exports ElementForm + a FormController interface so tests drive the controller directly (FormItems.test.tsx).

Benefits: Locality — the "mutate → refetch → follow selection" dance lives once instead of per-pane. Tests shrink from whole-app DOM boots to controller-against-mock-api units; the drag/DOM layer stays thin enough to leave to e2e.

> *This was generated by AI during triage.* Found by an architecture review (deepening opportunities — turning shallow modules into deep ones). **Files**: `web/src/components/PagePane.tsx` (2,569 lines), `web/src/components/TreePane.tsx` (1,453 lines); contrast `web/src/components/FormPane.tsx` and its `FormController` interface. **Problem**: the entire page-editing controller (publish/discard/undo/insert/apply-result, 32 drag handlers, optimistic `mutate()` vs authoritative `refetch()`) is fused with JSX. Every operation repeats the same busy/error/switch-on-tagged-result/refetch shape; `TreePane`'s `followMutation`/`run` wrappers re-implement `PagePane`'s `applyResult` dance. The only way to test any of it is mounting the whole `<App/>` and driving the DOM (`App.test.tsx`, `ElementCrud.test.tsx`, `Publish.test.tsx`, `LayoutGrid.test.tsx` all boot router + tree + elements + git state per assertion). **Solution**: extract `createPageEditor` / `createTreeController` stores holding the operation logic, leaving the panes as thin presentation — the pattern `FormPane` already proves: it exports `ElementForm` + a `FormController` interface so tests drive the controller directly (`FormItems.test.tsx`). **Benefits**: *Locality* — the "mutate → refetch → follow selection" dance lives once instead of per-pane. Tests shrink from whole-app DOM boots to controller-against-mock-api units; the drag/DOM layer stays thin enough to leave to e2e.
Author
Owner

This was generated by AI during triage.

Agent Brief

Category: enhancement
Summary: Extract the page-editing and tree operation logic out of the PagePane and TreePane components into testable controller/store units, following the FormController pattern that FormPane already establishes.

Current behavior:
PagePane and TreePane are the two largest components in the web app. Each fuses its full operation logic (publish/discard/undo, element insert/move/delete, page create/rename/move/delete, drag-and-drop orchestration) with its JSX. Both independently implement the same mutation shape — set a busy flag, call the API, switch on the tagged result, optimistically mutate() the resource or refetch() for the authoritative state, then repair/follow the selection. PagePane centralizes this in an internal applyResult helper; TreePane re-implements it with its own run wrapper plus followMutation. Because the logic lives inside the components, the only way to test any operation is mounting the whole <App/> shell via the test harness and driving the DOM — which is what the publish, element-CRUD, page-management, and layout test suites all do today, booting router + tree + elements + git state per assertion.

Desired behavior:
The operation logic lives in dedicated controller/store factories (e.g. createPageEditor for element-level operations and publish/discard/undo, createTreeController for page-tree operations) alongside the existing store modules (pageTree, gitState, hugoStatus, pagetypes). Each factory takes its dependencies (API surface, selection/navigation callbacks) as inputs and exposes reactive accessors (busy/error state, current document) plus operation methods, so a test can drive a controller directly against the mock API without mounting the shell. The panes become thin presentation: they consume a controller and render, keeping only DOM concerns (drag event wiring, focus, layout). The "mutate → refetch → follow selection" dance exists exactly once, shared by both controllers.

Key interfaces:

  • FormPane's exported FormController interface (submit/dirty/saving/saved accessors) — the proven precedent: a plain interface of stable accessors plus operation methods that a host or test drives directly. The new controllers should follow its spirit; they do not need to literally extend it.
  • The existing store-module idiom in the web app's stores directory (created when the API layer was split into transport/domain/stores) — the new factories should live there and match its conventions.
  • The tagged-result types returned by the domain API modules (element ops, page management, publish) — the controllers own switching on these; the panes should no longer contain that branching.
  • The mock API used by the current test suites — controller unit tests should run against it.
  • The route-repair/followMutation helpers — selection-following after mutations belongs to the controllers, not the JSX layer.

Acceptance criteria:

  • PagePane and TreePane no longer contain API-result branching, busy/error bookkeeping, or refetch/selection-repair logic — only presentation and DOM event wiring that delegates to a controller.
  • The mutate/refetch/follow-selection mechanism is implemented once and shared by both controllers, not duplicated per pane.
  • New unit tests drive createPageEditor and createTreeController directly against the mock API (no <App/> mount) covering at least: a successful mutation with optimistic update, a failed mutation surfacing an error state, and a mutation that changes the selected page/element path.
  • All existing test suites pass unchanged in behavior — existing whole-app tests may be slimmed down or ported to controller tests, but the behaviors they assert must remain covered.
  • No user-visible behavior changes: this is a pure refactor of the web client.

Out of scope:

  • Changing the drag-and-drop UX or rewriting the drag handlers themselves — they stay in the components; only the mutations they trigger move to the controllers.
  • Any Go/engine changes, API contract changes, or scaffold/site/AGENTS.md updates (nothing engine-facing changes here).
  • Refactoring FormPane/ElementForm itself, or other large components (MediaModule, fields).
  • Deleting the whole-app e2e-style tests wholesale — thinning them is fine, removing coverage is not.

Suggested sequencing (advisory, not binding): this can land as two PRs — extract createTreeController first (smaller surface), then createPageEditor — if a single change proves unwieldy.

> *This was generated by AI during triage.* ## Agent Brief **Category:** enhancement **Summary:** Extract the page-editing and tree operation logic out of the `PagePane` and `TreePane` components into testable controller/store units, following the `FormController` pattern that `FormPane` already establishes. **Current behavior:** `PagePane` and `TreePane` are the two largest components in the web app. Each fuses its full operation logic (publish/discard/undo, element insert/move/delete, page create/rename/move/delete, drag-and-drop orchestration) with its JSX. Both independently implement the same mutation shape — set a busy flag, call the API, switch on the tagged result, optimistically `mutate()` the resource or `refetch()` for the authoritative state, then repair/follow the selection. `PagePane` centralizes this in an internal `applyResult` helper; `TreePane` re-implements it with its own `run` wrapper plus `followMutation`. Because the logic lives inside the components, the only way to test any operation is mounting the whole `<App/>` shell via the test harness and driving the DOM — which is what the publish, element-CRUD, page-management, and layout test suites all do today, booting router + tree + elements + git state per assertion. **Desired behavior:** The operation logic lives in dedicated controller/store factories (e.g. `createPageEditor` for element-level operations and publish/discard/undo, `createTreeController` for page-tree operations) alongside the existing store modules (`pageTree`, `gitState`, `hugoStatus`, `pagetypes`). Each factory takes its dependencies (API surface, selection/navigation callbacks) as inputs and exposes reactive accessors (busy/error state, current document) plus operation methods, so a test can drive a controller directly against the mock API without mounting the shell. The panes become thin presentation: they consume a controller and render, keeping only DOM concerns (drag event wiring, focus, layout). The "mutate → refetch → follow selection" dance exists exactly once, shared by both controllers. **Key interfaces:** - `FormPane`'s exported `FormController` interface (`submit`/`dirty`/`saving`/`saved` accessors) — the proven precedent: a plain interface of stable accessors plus operation methods that a host or test drives directly. The new controllers should follow its spirit; they do not need to literally extend it. - The existing store-module idiom in the web app's `stores` directory (created when the API layer was split into transport/domain/stores) — the new factories should live there and match its conventions. - The tagged-result types returned by the domain API modules (element ops, page management, publish) — the controllers own switching on these; the panes should no longer contain that branching. - The mock API used by the current test suites — controller unit tests should run against it. - The route-repair/`followMutation` helpers — selection-following after mutations belongs to the controllers, not the JSX layer. **Acceptance criteria:** - [ ] `PagePane` and `TreePane` no longer contain API-result branching, busy/error bookkeeping, or refetch/selection-repair logic — only presentation and DOM event wiring that delegates to a controller. - [ ] The mutate/refetch/follow-selection mechanism is implemented once and shared by both controllers, not duplicated per pane. - [ ] New unit tests drive `createPageEditor` and `createTreeController` directly against the mock API (no `<App/>` mount) covering at least: a successful mutation with optimistic update, a failed mutation surfacing an error state, and a mutation that changes the selected page/element path. - [ ] All existing test suites pass unchanged in behavior — existing whole-app tests may be slimmed down or ported to controller tests, but the behaviors they assert must remain covered. - [ ] No user-visible behavior changes: this is a pure refactor of the web client. **Out of scope:** - Changing the drag-and-drop UX or rewriting the drag handlers themselves — they stay in the components; only the mutations they trigger move to the controllers. - Any Go/engine changes, API contract changes, or `scaffold/site/AGENTS.md` updates (nothing engine-facing changes here). - Refactoring `FormPane`/`ElementForm` itself, or other large components (`MediaModule`, `fields`). - Deleting the whole-app e2e-style tests wholesale — thinning them is fine, removing coverage is not. **Suggested sequencing (advisory, not binding):** this can land as two PRs — extract `createTreeController` first (smaller surface), then `createPageEditor` — if a single change proves unwieldy.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
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#113
No description provided.