ehoandClaude Opus 5
fc2a3c348b
RD-22 and RD-23 brought the wizard containers under the 250-line budget, so `max-lines` reports nothing. The files still read badly. Line count was never the problem. Fix three things in all three containers: 1. The member order was scrambled, and it differed per file. `registratie` declared `draftSync` in the middle of a run of `computed`s; `herregistratie` read `this.stepLabels.length` seven lines before `stepLabels` existed; the three files put the copy arrays in three different places. All three now use one nine-section order, so they compare side by side. 2. Pure logic sat in the container. Extract `digitalDocumentIds` into `upload.machine.ts` — the "digital and finished uploading" filter was written out four times, and it removes a `documentId!` assertion from both containers. Extract `diplomaMsg` into a sibling of the step files. 3. Comments carried archaeology. Drop the three RD-05 references and keep the rule. Drop "replaces sessionStorage" and the note about focus management that moved to the shell. Fix `intake`'s class comment, which claimed answers persist to sessionStorage and was contradicted 30 lines below. `phase` deliberately stays in all three: it cannot live in `domain/`, and three siblings plus three specs is a worse trade than 17 readable lines. The store ⇄ `draftSync` cycle also stays — both callbacks are deferred, so it is safe, and one comment now names it. No behaviour change. Member lists and every `private`/`protected`/`readonly` modifier are unchanged, which the showcase depends on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
101 lines
5.8 KiB
Markdown
101 lines
5.8 KiB
Markdown
# RD-38 — One member order for the three wizard containers
|
|
|
|
Status: done
|
|
Source: user report — "the registratie-wizard component still doesn't look readable"
|
|
|
|
## Why
|
|
|
|
RD-22 and RD-23 split the intake and registratie wizards into step components. That brought
|
|
every container under the 250-line budget, so `max-lines` reports nothing. The user read the
|
|
result and still called it unreadable. Line count was never the problem.
|
|
|
|
Three problems remained, and the user named all three:
|
|
|
|
1. **The member order is scrambled, and it differs per file.** You bounce up and down to
|
|
follow one thread, and the three containers cannot be compared side by side.
|
|
2. **Pure logic sits in the container**, where it has no test.
|
|
3. **Comments carry archaeology** — ticket numbers, history, and one comment that describes
|
|
code which is no longer in the file.
|
|
|
|
RD-02 measures a file. This ticket is about what a file reads like at a fixed size.
|
|
|
|
## Evidence, before the change
|
|
|
|
- `registratie` declared `stepLabels`/`stepTitles` between the `seed` input and
|
|
`state`/`dispatch`; `intake` declared them near the bottom; `herregistratie` declared them
|
|
after `draftSync`. Three files, three orders for the same three roles.
|
|
- `registratie` declared `draftSync` in the middle of a run of `computed`s.
|
|
- `herregistratie`'s `snapshot` read `this.stepLabels.length` seven lines before
|
|
`stepLabels` was declared.
|
|
- `intake`'s class comment claimed answers persist to `sessionStorage`. Thirty lines below,
|
|
another comment said draft-sync replaced it. The class comment was false.
|
|
- All three cited "RD-05's tag-transition rule" in an identical sentence.
|
|
|
|
## Decisions (pre-made, don't relitigate)
|
|
|
|
1. **One nine-section member order, identical in all three files.** Injected stores → inputs
|
|
→ the store (`createStore`, `state`, `dispatch`) → static copy → state projections →
|
|
controllers → shell wiring → event handlers → constructor and `restart`. Each section
|
|
carries a `// --- <name> ---` header. Section 7's header already existed in all three
|
|
files, so it is reused byte for byte.
|
|
|
|
2. **The store ⇄ `draftSync` cycle stays.** The store's effect map calls `this.draftSync`,
|
|
and `draftSync`'s `snapshot` calls `this.state()`. Both are arrow functions that run after
|
|
construction, so there is no temporal-dead-zone hazard; the cycle cannot be removed by
|
|
reordering. One comment at the effect map names it.
|
|
|
|
3. **Extract `digitalDocumentIds` into `libs/shared/src/domain/upload.machine.ts`.** The
|
|
"digital and finished uploading" filter was written out four times, in two shapes: mapped
|
|
to ids in the two container snapshots, and as `.some(...)` inside two machines'
|
|
`hasProgress`. One function serves all four, and it removes the `r.documentId!` non-null
|
|
assertion from both containers. It joins an existing file beside `deliveryRefs`, and its
|
|
spec joins the existing `deliveryRefs` block.
|
|
|
|
4. **Extract `diplomaMsg` into a new sibling of the step files.** `onDiplomaKeuze` was the
|
|
fattest member in the three containers and had no test. It is pure: a `DuoLookupDto` and a
|
|
selected id in, a machine message or `null` out. `HANDMATIG` does not move — the new file
|
|
is its sibling and imports it exactly as the container did.
|
|
|
|
5. **`phase` stays in all three containers.** It maps this machine's tags onto the shell's
|
|
`WizardPhase` vocabulary and composes a `$localize` failure message. That is a container's
|
|
job. It cannot move to `domain/`: `WizardPhase` comes from an Angular component, and
|
|
`domain/` points inward only. Moving it to a per-wizard sibling would add three files and
|
|
three specs to relocate 17 readable lines each. Decision 1 already fixes what was wrong
|
|
with it — it belongs in the shell-wiring section, and only two of three files had it there.
|
|
|
|
6. **The four `err*` computeds in `herregistratie` stay.** They are one-line projections
|
|
feeding four template bindings. Folding them into one `errors()` would edit the template,
|
|
which is behaviour-shaped work this ticket does not do.
|
|
|
|
7. **Comment policy, four rules.** Delete ticket references and keep the sentence (RD-18
|
|
decision 1 stripped `WP-`/`RB-` for the same reason; `RD-` is the same debt). Delete
|
|
history — a comment says what the code does now. Delete a comment that describes code
|
|
which is not in the file. Keep a comment that states a current why: the `untracked`
|
|
loop-avoidance notes, the `IntakePolicy.RejectIncompleteScholing` seam pointer, and the
|
|
`demo-*` preview note all stay.
|
|
|
|
## Traps
|
|
|
|
- **`messages.en.xlf`.** Every `$localize` id in these files is translated. The reorder moves
|
|
the copy arrays; it must not touch an id or its source text.
|
|
- **`enabled: () => this.seed() === initial`** is a reference-identity check against the
|
|
exported `initial` singleton. Never clone or rebuild it — breaking the identity turns
|
|
draft-sync on inside Storybook and the tests.
|
|
- **Public members are load-bearing.** `showcase/vragenlijst.section.ts` reads
|
|
`IntakeWizardComponent.steps`; `showcase/form-machine.section.ts` reads
|
|
`HerregistratieWizardComponent.state`. No `private`/`protected`/`readonly` modifier changes.
|
|
- **`intake.machine.ts` carries `#region showcase:steps` markers** that feed `gen:snippets`.
|
|
This ticket does not touch them.
|
|
- **New specs change a generated document.** `scripts/ci-local.sh` regenerates
|
|
`libs/shared/docs/behaviour-spec.mdx` and diffs it. Four new `it()` titles land there, so
|
|
the regenerated file belongs in the same commit.
|
|
|
|
## Acceptance
|
|
|
|
- `npm run ci` passes.
|
|
- Each container's member list and every modifier are unchanged:
|
|
`diff <(git show HEAD:$f | grep -oE '^ (private|protected|readonly)? ?[a-zA-Z]+ *[=(]' | sort) <(...)`
|
|
reports no difference for all three files.
|
|
- `git grep -nE "\bRD-[0-9]+" -- 'apps/ssp/src/app/*/ui/*wizard*'` returns nothing.
|
|
- `git grep -n "sessionStorage" -- 'apps/ssp/src/app/herregistratie/ui'` returns nothing.
|