edufeed/edufeed-app
Watch
1
0
Fork
You've already forked edufeed-app
 0
Code Issues 17 Pull requests 6 Projects Releases Packages 1 Wiki Activity Actions

feat(forms): preview the form as a respondent sees it, before publishing (#77) #78

Open
laoc wants to merge 5 commits from feat/issue-77-form-preview into dev
pull from: feat/issue-77-form-preview
merge into: edufeed:dev
Conversation 0 Commits 5 Files changed 23 +1633 -56
laoc commented 2026-08-03 11:34:59 +00:00
Owner
Copy link

Closes #77.

Authors can now see their form exactly as a respondent will, before publishing.

What changed

  • New FormPreview.svelte — renders the in-progress form definition through the same FormRenderer a respondent gets, so the preview is the real component, not a mock-up of it.
  • FormBuilder.svelte gains an Edit / Preview tab pair; builder state moves into src/lib/helpers/forms/builder-state.js so the preview and the editor read one source of truth instead of two.
  • /forms/[naddr] (the published form page) gains the same Preview tab.
  • Adapter behaviour is pinned (FormPreview.adapters.test.js), and a comment in CreatorFieldAdapter.svelte that this change made false is corrected.
  • playwright.config.js pins every app-read relay var so a populated local .env cannot leak a production relay into an e2e run.

Behavioural change on an already-shipped page — please read

The published form page's Preview tab now renders a Submit button that publishes nothing. The previous readonly preview had no such button.

This is deliberate: #77 asks the author to be able to see what a respondent sees, and the post-submit confirmation is part of that. The button runs the full validation path and then shows a confirmation whose copy states that nothing was published.

The counter-argument, stated plainly: the same page also carries a real "Fill Form" link, so two controls on one page now look like ways to respond. Suppressing Submit on that route only is ~10 lines if you'd rather. laoc chose to keep it (DM, 2026-08-03).

Verification

  • pnpm test at 25545f8e: 481 files / 5321 tests, all passing.
  • e2e (e2e/form-preview.test.js, real Chrome, first attempt) verified independently by TestOER at both 854ff46d and the tip 25545f8e, with a positive control in the same run.
  • Four mutation controls fail at lines 147 / 119 / 133 / 110 — routing, validation and show-if are each independently live, not passing vacuously.
  • Measured on the wire: nothing was published at any point, using a detector proven able to fire in the same run.

Note on the commit range

The first two commits in this range (95db6336, cc4368f8 — an inbox kind-1 reply fix) are an unrelated change that sits on the local dev tip but has not been pushed to origin/dev. They are not part of this work; they appear here only because this branch forked from that tip. origin/dev itself is untouched.

Closes #77. Authors can now see their form exactly as a respondent will, before publishing. ## What changed - **New `FormPreview.svelte`** — renders the in-progress form definition through the same `FormRenderer` a respondent gets, so the preview is the real component, not a mock-up of it. - **`FormBuilder.svelte`** gains an Edit / Preview tab pair; builder state moves into `src/lib/helpers/forms/builder-state.js` so the preview and the editor read one source of truth instead of two. - **`/forms/[naddr]`** (the published form page) gains the same Preview tab. - Adapter behaviour is pinned (`FormPreview.adapters.test.js`), and a comment in `CreatorFieldAdapter.svelte` that this change made false is corrected. - `playwright.config.js` pins every app-read relay var so a populated local `.env` cannot leak a production relay into an e2e run. ## Behavioural change on an already-shipped page — please read The published form page's Preview tab now renders a **Submit button that publishes nothing**. The previous readonly preview had no such button. This is deliberate: #77 asks the author to be able to see what a respondent sees, and the post-submit confirmation is part of that. The button runs the full validation path and then shows a confirmation whose copy states that nothing was published. The counter-argument, stated plainly: the same page also carries a real "Fill Form" link, so two controls on one page now look like ways to respond. Suppressing Submit on that route only is ~10 lines if you'd rather. laoc chose to keep it (DM, 2026-08-03). ## Verification - `pnpm test` at `25545f8e`: **481 files / 5321 tests, all passing**. - e2e (`e2e/form-preview.test.js`, real Chrome, first attempt) verified independently by TestOER at both `854ff46d` and the tip `25545f8e`, with a positive control in the same run. - Four mutation controls fail at lines 147 / 119 / 133 / 110 — routing, validation and show-if are each independently live, not passing vacuously. - Measured on the wire: nothing was published at any point, using a detector proven able to fire in the same run. ## Note on the commit range The first two commits in this range (`95db6336`, `cc4368f8` — an inbox kind-1 reply fix) are an unrelated change that sits on the local `dev` tip but has not been pushed to `origin/dev`. They are not part of this work; they appear here only because this branch forked from that tip. `origin/dev` itself is untouched. 
Replies written by other Nostr clients are kind 1 (NIP-10), not kind 1111
(NIP-22). The thread view already renders both — createCommentLoaderForEvent
merges a kind 1 #e filter for kind 1 roots — but the inbox only ever queried
kinds 1070, 1069, 7, 9 and 1111, so a reply to your own note was structurally
invisible there.

Add kind 1 to the p-tagged notification filter and classify it: with an
e-tag it is a 'reply', without one a 'mention'. Both link to the note's
nevent, which the [nevent] route renders as a thread.

The eventStore model filters were a hand-copied duplicate of the loader
filters — exactly how the two drifted apart — so derive them from
buildMainFilter instead and pin that with a test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merge branch 'worktree-fix+inbox-kind1-replies' into dev
Some checks failed
Build and Push Docker Image / build (push) Failing after 5m5s
Details
cc4368f8c1 
fix(inbox): surface kind 1 replies and note mentions
To see your own form you had to publish it: a kind-30168 under a d-tag you
may not have wanted to burn, then click through the fill link. And the one
thing already called "Preview" — the tab on the published form page — was
`<FormRenderer readonly />`, whose flat branch concatenates every section
with disabled inputs and no navigation. It could not show the section
wizard, option->section routing, show-if, or required-field validation,
which is precisely the authoring work of the last ~10 commits (5620910f,
abe1feaa, 757bc0d3). Both halves of that are what the user feedback on #77
is pointing at.

No new rendering path was needed. buildFormTemplateTags and extractSections
are pure and tolerate an empty d-tag, and parseFormTemplate reads only
event.tags — never id, sig or pubkey. So a preview is builder state -> the
same tags publish emits -> the same parser respondents use, with no signer,
relay or naddr anywhere near it.

That sameness is the design constraint, not an implementation detail: the
FieldState->tags mapping was inline inside publish() and reachable from
nowhere else, so it moves to helpers/forms/builder-state.js and publish()
now calls it too. One encoder means the preview cannot drift from the
artifact. A preview that re-rendered builder state on its own path would
prove nothing.

FormPreview renders NOT readonly, so branching and validation are live, and
catches onsubmit to show the author's own confirmation message instead of
publishing. It never imports publish-service. The published-form page's
preview tab now uses it too, fixing the same misrepresentation there.

Two traps worth recording:

- FormRenderer seeds `values` behind a plain non-$state `initialized` latch
  that fires once per instance, so an edited template reaching a live
  instance keeps the stale seed — new fields get no default and text-array
  falls back to ['']. FieldsRenderer is defensive enough that this degrades
  silently rather than throwing: a preview quietly lying about defaults.
  Fixed by remounting on the tag signature, local to the preview. The latch
  itself is deliberately untouched — /respond relies on it so an
  eventStore.replaceable update mid-fill cannot wipe a respondent's answers.
- The preview drags in FormRenderer's whole field-adapter chain (AMB search,
  creator input, concept picker), which the builder otherwise never loads,
  so it is imported on demand rather than statically.

Drive-by: FormRenderer's submit button hardcoded "Submit" beside a wall of
m.* calls; form_submit already existed in both locales, unused.

Tests: the FormPreview spec asserts wizard chrome, linear fallthrough,
explicit routing that skips a section, show-if on a reached section, and
validation blocking Next. Pointed at the old `readonly` rendering, 7 of its
9 cases fail — the 2 that survive are preview chrome, correctly independent
of render mode. Encoder mutations (drop sections / displayIf / output) each
fail exactly the named round-trip cases. e2e/form-preview.test.js mirrors
form-builder-authoring.test.js's assertions with no publish; it is authored
but UNRUN here — this box has no chromium — and needs an independent run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-ups to 854ff46d, all surfaced by TestOER's verification of it.

Swapping the published-form page's Preview tab off `readonly` has a runtime
consequence the original commit did not account for. `readonly` was suppressing
two interactive adapters: RelationFieldAdapter's `{#if !readonly}` around
AMBResourceSearchInput, and CreatorFieldAdapter's static <ul> in place of
CreatorInput. So an `amb-relation` field on that tab now mounts a live NIP-50
search input where previously nothing rendered — on a shipped page.

Reading AMBResourceSearchInput says mounting is quiet: `runSearch` has one
caller, `onInput`, behind a 300ms debounce with an early return under two
characters. But no test mounted that tab with a relation field at all, so the
claim rested on a source read. FormPreview.adapters.test.js pins it: mounting
issues zero searches, with an in-test positive control proving the spy fires
once on typing — and not before the debounce. Without that control the
assertion is satisfied equally well by a spy wired to nothing, which passes
vacuously while reading as proof. Mutation-checked with an onMount(runSearch)
in AMBResourceSearchInput: both the mount assertion and the control go red.

CreatorFieldAdapter's comment claimed "preview route renders FormRenderer
readonly". #77 made that false. The branch is still reachable via
FormResponses, so the code is right and only the comment was lying — but a
comment asserting a fact about the codebase is the same weak class of evidence
as a wiring line, and it outlives what it describes.

webServer.timeout was 120s for a command that runs a full production build
first. That budget assumed a warm build and expired before a single test ran in
a freshly-installed worktree, which is how anyone first meets this suite.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TestOER measured an unpinned KANBAN_RELAYS opening wss://relay-rpi.edufeed.org
from the e2e sandbox and issuing a REQ for the test user's kinds 10050/30078/
30168. Reads only — no EVENT frame went anywhere but ws://localhost:17003 in any
run — and it needs a populated local .env, so CI is unaffected.

Auditing the whole set rather than the one var found a second: DM_RELAYS
(wss://dm.edufeed.org), read at routes/api/config/+server.js:165 and used as the
DM default in relay-helper.js:36-44. Nothing in the current specs walks a DM
path, which is why no ledger showed it — but the rule has to be every var the
app reads, not the ones a current spec happens to exercise.

EDUFEED_PUBLISH_RELAYS stays out on purpose. I first called it dead config,
having searched only src/, vite.config.js and svelte.config.js — TestOER found
it in three scripts/ operator CLIs, one of which deletes events. The action is
unchanged (the app never reads it, the e2e never runs those scripts) but the
label mattered: "dead" invites tidying away the var that decides which relay a
delete script points at. The comment records why it is absent so the next
audit doesn't have to rediscover it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reported on #77: a checkbox field with options test/test2/test3 previewed as
a single unlabelled toggle. The preview was faithful; the form was broken.

Three defects, all present at cc4368f8, all older than the preview:

1. The builder offers an options editor for select, checkbox and radio
   (CHOICE_TYPES), but the options were serialised for only two — and in TWO
   independent places: the builder→wire mapping (FormBuilder.publish, since
   extracted to builder-state.js) and buildFormTemplateTags' isOptionType.
   Either one alone discards an author's choices. So checkbox options have
   never survived publishing, preview or not.

2. FieldsRenderer had no multi-checkbox branch for type:'checkbox' — it drew
   a lone boolean box, while the checkbox-group markup sat right beside it
   gated on select && multiple.

3. Consequently validateField's `value !== 'true'` rule rejected every
   selection a checkbox group can produce. A required checkbox group had no
   way to validate at all, because the shape it emits (';'-joined ids) could
   never equal 'true'.

The four copies of the choice-type list are now one exported CHOICE_TYPES in
forms/format.js, imported by the builder row, the encoder and the serialiser,
because a list restated per file is what let these drift apart.

A checkbox with options is a multi-select group (';'-joined ids, same shape
select+multiple already used); with none it stays a boolean toggle and
serialises exactly as before, so existing forms are untouched. Measured:
buildFormTemplateTags over 13 types x has-options x has-vocab gives 51
identical outputs and 1 differing — the broken case. Nothing can have relied
on the group shape; it was never publishable.

The new rule also accepts a raw `true` beside the legacy `'true'`. That part
is a contract tightening, NOT a repair: FormRenderer.handleFieldChange
stringifies the boolean at the boundary and has since before this branch, so
no caller in the app ever reached the old rule with one. Verified on the real
/respond route with the pre-fix expression restored — it submits. An earlier
version of this message claimed every published form with a required consent
checkbox was unsubmittable; that was wrong. I read FieldsRenderer's emit and
validateField's test and never read the function between them, which is named
for exactly that conversion.

Why the suite missed the real defects: FieldsRenderer.test.js asserted
onchange(…, true) and forms.test.js asserted validateField(…, 'true'), each
green, neither crossing the seam. The e2e built its form from radio and text
only, so it was green and blind — which is how the report arrived from a human
clicking rather than from CI. The new builder-state test asserts on the TAGS
rather than the intermediate mapping; asserting the intermediate is what let
the second gate hide behind the first.

Tests: unit assertions written against the unfixed source and confirmed
failing there, plus a checkbox e2e reproducing the report end to end.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three specs written by TestOER while verifying the checkbox fix, kept because
each measures something the commit's own tests could not.

  src/lib/__tests__/forms.field-type-sweep.test.js
      All 13 types FormBuilder offers, driven builderStateToTags →
      parseFormTemplate against every property the builder lets an author set:
      label, required, defaultValue, placeholder, min/max, output, choices.
      The checkbox defect was a serialisation defect and this is the shape that
      finds another one. Asserts on the PARSED field, never on the intermediate
      mapping — asserting the intermediate is what let the second of two serial
      gates hide behind the first.

  src/lib/__tests__/forms.checkbox-downstream.test.js
      "A checkbox group has the same value shape select+multiple already used"
      is a claim about consumers, and the fix exercised none of them. Drives
      both types through validateField, formValuesToAmbJson (ext / amb-scalar /
      keywords) and evaluateDisplayIf `contains`, plus the optionless
      checkbox's boolean contract.

  e2e/form-checkbox-published.test.js
      The only spec in the repo that drives a required checkbox past publish.
      It is the one that refuted this branch's original claim that a required
      checkbox was unsubmittable on every published form — run with the pre-fix
      expression restored, it still submits, because
      FormRenderer.handleFieldChange bridges the boolean/string seam and always
      has. Kept precisely because it measures a seam a code read got wrong.

Two changes on adoption, both mine:

  - JSDoc types. These were verified with svelte-check run on a tree with them
    removed, so they had never been type-checked; adding them raised 9 errors
    (implicit any, and `undefined` rows in it.each that the param type does not
    admit). Fixed in the tests with casts and annotations — validateField's
    signature is NOT widened to accommodate a test.
  - The e2e header cited the retracted claim as though it were open. Rewritten
    to record what the spec actually established.

Re-proven live in this tree rather than assumed from TestOER's run: mutating
gate 2 (format.js isOptionType) turns the sweep red 2, mutating gate 1
(builder-state.js CHOICE_TYPES) turns it red 4, and restoring the pre-fix
`value !== 'true'` turns the downstream suite red 1. Each patch confirmed
landed via git diff --numstat, tree restored between rows.

Suite at this commit: 481+2 files, 5436 passed, 0 failed, exit 0 on two
consecutive runs. svelte-check 0 errors / 4 warnings = baseline. All three e2e
specs pass on real Chrome with --retries=0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This pull request has changes conflicting with the target branch.
  • messages/de.json
  • messages/en.json
View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin feat/issue-77-form-preview:feat/issue-77-form-preview
git switch feat/issue-77-form-preview

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch dev
git merge --no-ff feat/issue-77-form-preview
git switch feat/issue-77-form-preview
git rebase dev
git switch dev
git merge --ff-only feat/issue-77-form-preview
git switch feat/issue-77-form-preview
git rebase dev
git switch dev
git merge --no-ff feat/issue-77-form-preview
git switch dev
git merge --squash feat/issue-77-form-preview
git switch dev
git merge --ff-only feat/issue-77-form-preview
git switch dev
git merge feat/issue-77-form-preview
git push origin dev
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug
Something is broken

  
discussion
Needs a decision or concept first

  
enhancement
New feature or improvement

No labels
bug
discussion
enhancement
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
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
edufeed/edufeed-app!78
No description provided.