refactor(web): extract page editor controllers from PagePane/TreePane #120

Merged

dominik.polakovics merged 2 commits from afk/113 into main

2026-07-18 20:34:14 +02:00

dominik.polakovics commented

2026-07-18 19:33:28 +02:00

Owner

Closes #113

Extracts the page-editing operation logic out of the two largest components into testable controller stores, following the FormController precedent.

What moved where

stores/mutations.ts — the shared engine. The "mutate → refetch → follow selection" dance exists exactly once: attempt (error-channel lifecycle around a throwing postOp-style mutation, optional resync on failure) and follow (apply authoritative state, then move the selection past the unsaved-changes guard with route repair suspended for the duration).
stores/treeController.ts (createTreeController) — every tree mutation (draft toggle, reorder, create/root-create, rename, drag-move, the post-delete follow) plus the state those follows manipulate: the expansion set (with its sessionStorage persistence and root-seed) and the pagetype library. Replaces TreePane's run wrapper and followMutation handlers.
stores/pageEditor.ts (createPageEditor) — the doc resource with its cross-page validity gate, applyResult's authoritative-swap-on-success / resync-on-failure discipline for element ops, and the publish/discard/undo tagged-outcome routing. Replaces the logic previously fused into PagePane.
TreePane / PagePane — now presentation only: menus, dialogs, inline rename/phantom inputs, drag geometry, and DOM wiring that delegates to the controllers. JSX and DOM structure are untouched; transient-input lifetimes (phantom row, inline rename) are preserved via onAccepted hooks that fire exactly when the old inline code cleared them.

Acceptance criteria

Panes contain no API-result branching, busy/error bookkeeping, or refetch/selection-repair logic — verified by sweep: no catch/unguarded/followMutation/mutation-API imports remain in either pane.
The mutate/refetch/follow-selection mechanism is implemented once (stores/mutations.ts) and shared by both controllers.
New unit suites (37 tests) drive createTreeController and createPageEditor directly against the existing mock API — no <App/> mount — covering successful mutations with authoritative/optimistic update, failed mutations surfacing error state, and selection-changing mutations (rename/move follows, subtree-delete clearing, discard/undo of a removed page).
All pre-existing suites pass unchanged (they were not edited): 46 files / 935 tests green, tsc --noEmit clean, vite build and go build ./... + go vet ./... pass.

Notes for review

Two deliberate, test-verified micro-drifts: follows for create/delete now also run under route-repair suspension (previously only rename/move did — semantically the correct scope), and a vestigial setDialog(null) inside the old handleMoved was dropped (moves only originate from drops; the delete flow's dialog close is preserved).
The whole-app DOM suites were deliberately kept rather than thinned: they now pin the pane wiring while the controller suites pin the operation logic. Thinning them is possible follow-up work, per the issue's "may be slimmed down".

🤖 Generated with Claude Code

Closes #113 Extracts the page-editing operation logic out of the two largest components into testable controller stores, following the `FormController` precedent. ## What moved where - **`stores/mutations.ts`** — the shared engine. The "mutate → refetch → follow selection" dance exists exactly once: `attempt` (error-channel lifecycle around a throwing postOp-style mutation, optional resync on failure) and `follow` (apply authoritative state, then move the selection past the unsaved-changes guard with route repair suspended for the duration). - **`stores/treeController.ts`** (`createTreeController`) — every tree mutation (draft toggle, reorder, create/root-create, rename, drag-move, the post-delete follow) plus the state those follows manipulate: the expansion set (with its sessionStorage persistence and root-seed) and the pagetype library. Replaces `TreePane`'s `run` wrapper and `followMutation` handlers. - **`stores/pageEditor.ts`** (`createPageEditor`) — the doc resource with its cross-page validity gate, `applyResult`'s authoritative-swap-on-success / resync-on-failure discipline for element ops, and the publish/discard/undo tagged-outcome routing. Replaces the logic previously fused into `PagePane`. - **`TreePane` / `PagePane`** — now presentation only: menus, dialogs, inline rename/phantom inputs, drag geometry, and DOM wiring that delegates to the controllers. JSX and DOM structure are untouched; transient-input lifetimes (phantom row, inline rename) are preserved via `onAccepted` hooks that fire exactly when the old inline code cleared them. ## Acceptance criteria - Panes contain no API-result branching, busy/error bookkeeping, or refetch/selection-repair logic — verified by sweep: no `catch`/`unguarded`/`followMutation`/mutation-API imports remain in either pane. - The mutate/refetch/follow-selection mechanism is implemented once (`stores/mutations.ts`) and shared by both controllers. - New unit suites (37 tests) drive `createTreeController` and `createPageEditor` directly against the existing mock API — no `<App/>` mount — covering successful mutations with authoritative/optimistic update, failed mutations surfacing error state, and selection-changing mutations (rename/move follows, subtree-delete clearing, discard/undo of a removed page). - All pre-existing suites pass unchanged (they were not edited): 46 files / 935 tests green, `tsc --noEmit` clean, `vite build` and `go build ./...` + `go vet ./...` pass. ## Notes for review - Two deliberate, test-verified micro-drifts: follows for create/delete now also run under route-repair suspension (previously only rename/move did — semantically the correct scope), and a vestigial `setDialog(null)` inside the old `handleMoved` was dropped (moves only originate from drops; the delete flow's dialog close is preserved). - The whole-app DOM suites were deliberately kept rather than thinned: they now pin the pane wiring while the controller suites pin the operation logic. Thinning them is possible follow-up work, per the issue's "may be slimmed down". 🤖 Generated with [Claude Code](https://claude.com/claude-code)

dominik.polakovics added 2 commits

2026-07-18 19:33:28 +02:00

refactor(web): extract pageEditor/treeController stores from the panes 54ad654fbd

PagePane and TreePane each fused their full operation surface — element
CRUD, publish/discard/undo, page create/rename/move/delete, and the
busy/error/tagged-result/refetch bookkeeping around every mutation — with
their JSX, so the only way to exercise any of it was mounting the whole
<App/> and driving the DOM.

The operation logic now lives in two controller stores next to the
existing ones, over one shared engine:

- stores/mutations.ts: the mutate → resync → follow-selection dance,
  written once. `attempt` runs a throwing (postOp-style) mutation into
  the error channel; `follow` applies authoritative state, then moves
  the selection past the unsaved-changes guard with route repair
  suspended for the duration.
- stores/treeController.ts: every tree mutation and its follow
  (expansion carry, push-vs-replace identity rules), plus the expansion
  set and the pagetype library the drops validate against.
- stores/pageEditor.ts: the doc resource with its cross-page validity
  gate, applyResult's authoritative-swap-or-resync discipline, and the
  publish/discard/undo outcome routing.

The panes keep only presentation: menus, dialogs, inline inputs, drag
geometry, and DOM wiring that delegates to the controllers. No
user-visible behavior changes; the pre-existing whole-app suites pass
unchanged.

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

test(web): drive the pane controllers directly against the mock api

ci / build-and-test (pull_request) Successful in 6m52s

Details

fd3f5c392b

Unit suites for the extracted stores — no <App/> mount, no DOM: the
mutation engine's ordering guarantees (error lifecycle, prepare-before-
select, repair suspension, guard bypass), the tree controller's follows
(expansion carry, push-vs-replace, subtree-deletion selection clearing)
and the page editor's disciplines (authoritative swap without refetch on
success, resync + error strip on failure, publish outcome routing, the
undo layout dance, the cross-page doc validity gate).

The whole-app DOM suites stay as they were: they now cover the wiring,
while the operation logic is pinned here against the same mock transport.

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

dominik.polakovics commented

2026-07-18 20:27:08 +02:00

Author

Owner

This was generated by AI while landing a PR.

Landing audit — PASS

Verification signal relied on: Forgejo Actions ci / build-and-test (pull_request) — success in 6m52s (run 107). Not re-run. This repo has a single unconditional CI job, so the green light did exercise this diff's path (web unit suites + tsc + vite build + go build/vet).

Mergeability: origin/main is already an ancestor of afk/113; a merge-tree test resolves cleanly. No conflict to resolve.

Conventions: title is Conventional Commits (refactor(web): …); two clean conventional commits; body carries Closes #113 matching the afk/113 head branch, so the merge will auto-close the issue and release the run's claim. labctl secret scan over the outgoing diff: clean. No engine-facing surface touched, so the scaffold/site/AGENTS.md drift rule does not apply.

Claims audited (all verified true):

"37 tests" — exact (8 mutations + 15 pageEditor + 14 treeController); no .skip/.only inflating the count.
"pre-existing suites not edited" — mechanically confirmed: the diff touches exactly 8 files (2 panes, 3 stores, 3 new test files). mockApi.ts untouched.
The pane sweep (catch/unguarded/followMutation/mutation-API imports) holds; remaining api/* imports in the panes are read-only fetches, pure helpers, or types.
New tests drive the controllers under createRoot with a hand-rolled selection store and the mock API — no <App/> mount anywhere.

Behavior-drift review. Three independent passes compared each removed handler against its replacement line by line. The extraction is faithful throughout — applyResult, the doc resource's cross-page gate, the publish tagged-outcome switch, dialog-close ordering, mutate vs refetch choices, and the rename/move double-refetch (matching the old run() trailing refetch) are all preserved. createPage/createRoot correctly omit that trailing refetch, matching the old paths that bypassed run().

Both self-declared drifts are accurate. Two notes, neither blocking:

Declared drift (1) is understated as "semantically correct scope" — the create case is a latent bug fix. A create converts a leaf parent into a branch bundle, changing its path. On main, with repair live, the refetch landed a tree missing the routed path, so route repair could bounce the editor to the empty state and leave a stray replace in history before the follow's push. Suspending repair removes that. Real improvement, but user-reachable and uncovered by the new tests. The delete and createRoot cases genuinely are inert (the route already names no page at that point).
One undeclared, cosmetic drift. main's doDiscard never cleared opError on entry; routing it through the shared attempt now does (stores/mutations.ts:53). Consequence: a stale element-op error (e.g. a 409 "placement violation") now disappears when a subsequent discard succeeds, where it used to linger. This is the more consistent idiom — every other operation already cleared on entry — but it is a behavior change in something billed as pure refactor, and no test pins it either way.

Test quality: above baseline. Assertions target observable state and wire payloads, not call counts — the insert test proves the authoritative swap by asserting /api/page fetch count is unchanged; events arrays pin ordering (["suspend","prepare","select","resume"]); several tests assert wire shape negatively (an omitted move index must be absent, not undefined). All three of the issue's required cases are covered.

Suggested follow-ups (not gating): reword declared drift (1) to name the create-into-open-leaf fix, and consider a test asserting the create follow runs under suspension. The panes remain large (PagePane 2570→2303, TreePane 1453→1208) since JSX and drag handlers stay by design; the issue's "whole-app suites may be slimmed down" remains open follow-up work, as the PR notes.

> *This was generated by AI while landing a PR.* ## Landing audit — PASS **Verification signal relied on:** Forgejo Actions `ci / build-and-test (pull_request)` — **success** in 6m52s ([run 107](/Cloonar/deckle/actions/runs/107/jobs/0)). Not re-run. This repo has a single unconditional CI job, so the green light did exercise this diff's path (web unit suites + tsc + vite build + go build/vet). **Mergeability:** `origin/main` is already an ancestor of `afk/113`; a merge-tree test resolves cleanly. No conflict to resolve. **Conventions:** title is Conventional Commits (`refactor(web): …`); two clean conventional commits; body carries `Closes #113` matching the `afk/113` head branch, so the merge will auto-close the issue and release the run's claim. `labctl secret scan` over the outgoing diff: clean. No engine-facing surface touched, so the `scaffold/site/AGENTS.md` drift rule does not apply. **Claims audited (all verified true):** - "37 tests" — exact (8 mutations + 15 pageEditor + 14 treeController); no `.skip`/`.only` inflating the count. - "pre-existing suites not edited" — mechanically confirmed: the diff touches exactly 8 files (2 panes, 3 stores, 3 new test files). `mockApi.ts` untouched. - The pane sweep (`catch`/`unguarded`/`followMutation`/mutation-API imports) holds; remaining `api/*` imports in the panes are read-only fetches, pure helpers, or types. - New tests drive the controllers under `createRoot` with a hand-rolled selection store and the mock API — no `<App/>` mount anywhere. **Behavior-drift review.** Three independent passes compared each removed handler against its replacement line by line. The extraction is faithful throughout — `applyResult`, the doc resource's cross-page gate, the publish tagged-outcome switch, dialog-close ordering, `mutate` vs `refetch` choices, and the rename/move double-refetch (matching the old `run()` trailing refetch) are all preserved. `createPage`/`createRoot` correctly omit that trailing refetch, matching the old paths that bypassed `run()`. Both self-declared drifts are accurate. Two notes, **neither blocking**: 1. **Declared drift (1) is understated as "semantically correct scope" — the create case is a latent bug fix.** A create converts a leaf parent into a branch bundle, changing its path. On `main`, with repair live, the refetch landed a tree missing the routed path, so route repair could bounce the editor to the empty state and leave a stray `replace` in history before the follow's push. Suspending repair removes that. Real improvement, but user-reachable and uncovered by the new tests. The delete and `createRoot` cases genuinely are inert (the route already names no page at that point). 2. **One undeclared, cosmetic drift.** `main`'s `doDiscard` never cleared `opError` on entry; routing it through the shared `attempt` now does (`stores/mutations.ts:53`). Consequence: a stale element-op error (e.g. a 409 "placement violation") now disappears when a subsequent discard succeeds, where it used to linger. This is the more consistent idiom — every other operation already cleared on entry — but it is a behavior change in something billed as pure refactor, and no test pins it either way. **Test quality:** above baseline. Assertions target observable state and wire payloads, not call counts — the insert test proves the authoritative swap by asserting `/api/page` fetch count is *unchanged*; `events` arrays pin ordering (`["suspend","prepare","select","resume"]`); several tests assert wire shape negatively (an omitted move index must be absent, not `undefined`). All three of the issue's required cases are covered. **Suggested follow-ups (not gating):** reword declared drift (1) to name the create-into-open-leaf fix, and consider a test asserting the create follow runs under suspension. The panes remain large (PagePane 2570→2303, TreePane 1453→1208) since JSX and drag handlers stay by design; the issue's "whole-app suites may be slimmed down" remains open follow-up work, as the PR notes.

dominik.polakovics merged commit b34ce8d68d into main

2026-07-18 20:34:14 +02:00

dominik.polakovics referenced this pull request from a commit

2026-07-18 20:34:15 +02:00

Merge pull request 'refactor(web): extract page editor controllers from PagePane/TreePane' (#120) from afk/113 into main