Skip to content

fix(frontend): wizard state-loss bugs, bounded zip memory, accessibility pass#136

Merged
jodeleeuw merged 5 commits into
mainfrom
claude/frontend-wizard-fixes
Jul 20, 2026
Merged

fix(frontend): wizard state-loss bugs, bounded zip memory, accessibility pass#136
jodeleeuw merged 5 commits into
mainfrom
claude/frontend-wizard-fixes

Conversation

@jodeleeuw

Copy link
Copy Markdown
Member

Summary

Implements the frontend workstream of the repository improvement plan (docs/dev/improvement-plan.md, Phase 1d + frontend memory items for #95).

Wizard state-loss bugs

  • Revisiting Project Info no longer reverts session edits: loadMetadata ran on every page mount (pages remount on each step change), resurrecting deleted authors and clobbering edited variables and form fields. Loading is now gated on a file-identity token in the AppShell session; uploading a different file still triggers a fresh load.
  • "Upload additional files" is now truly additive: previously it store.clear()ed the staged dataset but kept the old batch's variables in metadata, so the downloaded zip contained only the last batch while the metadata described all of them — and the in-browser validator then flagged mismatches. Used-filename sets persist in the session so disambiguation stays correct across batches. Replacing the folder is now an explicit confirm that clears the store and resets generated variables (reloading the existing metadata file and project-info fields).
  • Navigation is locked during processing (sidebar + Start over disabled) — previously navigating mid-run silently lost the run, orphaned the staged store, and kept partially-absorbed variables.
  • Picked-but-unprocessed files survive navigation ('ready' phase derives from the session; source name moved into the session); the "variables loaded from existing metadata" state now requires the load to have actually succeeded; folder inputs set webkitdirectory declaratively in JSX (it was silently lost after phase-branch remounts, degrading to a file picker).

Zip memory (issue #95)

  • Download: datasetZip.ts previously created one fflate AsyncZipDeflate (= one Web Worker) per file up front with whole-file arrayBuffer() reads and no backpressure — worst case most of the compressed dataset buffered in heap. Entries are now compressed strictly sequentially in 1 MiB blob slices, awaiting each entry's completion. Public API unchanged.
  • Upload: zip entries are extracted sequentially as blobs (not concurrent strings), and files are named by entry basename (the full in-archive path previously defeated the compliant-name check and produced nested data/raw/ paths).
  • Small fixes: folder input value reset (re-picking the same folder works), .psychds-ignore write inside error handling (an OPFS failure no longer hangs "processing"), download button disabled while a zip build is in flight, zip filename sanitized.

Accessibility / UX

  • aria-live/role="alert" on pick errors, processing summary, validation results, and the required-name error (there were zero live regions in the app).
  • JSON preview drawer converted to a native <dialog> with showModal/Escape/backdrop-close (was a role="dialog" div with no focus management).
  • Label associations on the Variables page; beforeunload guard while there is unsaved work (cleared after download).
  • Deleted dead code: unused global.d.ts and 5 unreferenced SVG assets.

Testing

  • 673 tests pass (baseline 656; frontend suite 246 → 263). New integration-style tests with real components: AppShell + real ProjectInfo (load-once + failed-parse gating), DataUpload batch consistency (additive/replace/cancel), navigation lock, session round-trip, zip basename handling, sequential zip backpressure (fake fflate asserting entry N+1 never starts before N completes), downloadDatasetZip (previously zero coverage: abort, error → sink abort, blob fallback), PreviewDrawer modal behavior.
  • npm run build -w frontend succeeds. Introduces no new lint violations (src errors 6 → 4 vs main).

Notes for review

🤖 Generated with Claude Code

https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg


Generated by Claude Code

claude added 5 commits July 6, 2026 21:07
Remove the pre-React window declaration (src/global.d.ts, unreferenced) and
the empty/unused SVGs (react, uparrow, downarrow, trash, plus) — all verified
unreferenced across src.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg
Compress dataset entries strictly one at a time: for each file create a single
AsyncZipDeflate, feed it in bounded ~1 MiB Blob slices, and await that entry's
final chunk before starting the next. fflate spawns one worker per live
AsyncZipDeflate and buffers later entries until earlier ones finish, so the old
"create every deflater up front" loop held N workers plus most of the compressed
dataset in the heap; this keeps ~one worker and one file's working set resident.
Public API is unchanged; the header comment now matches reality.

Tests: fake-fflate ordering test asserts entry N+1 isn't created/pushed before
entry N emits its final chunk; downloadDatasetZip (previously uncovered) gains
picker-abort -> false, streaming-error -> sink.abort()+propagate, blob fallback,
and streaming-success cases.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg
Load existing metadata exactly once per uploaded file: gate ProjectInfo's load
effect on a file-identity token stored in the AppShell-level session, so
revisiting Project Info no longer re-runs loadMetadata and clobbers session
edits (resurrecting deleted authors / reverting variables). Propagate load
success/failure into the session and gate Data pre-completion and the "variables
loaded from existing metadata" banner on it, so a failed parse no longer
pre-completes Data.

Make metadata and staged data stay consistent across batches: "Upload
additional files" is now truly additive (no store.clear(), used-filename sets
persisted in the session and seeded for the new batch, files appended);
"Replace all data" / "Change folder" confirm first, then fully reset — clear the
store AND reset the metadata variables via a resetMetadata callback owned by
AppShell (drops generated variables, reloads the existing metadata file if
present, re-applies project-info fields). Re-configure join keys reprocesses the
whole dataset as a replace so it can't double-stage a batch.

Block sidebar navigation and Start over while a run is in flight (DataUpload
reports busy by phase; Sidebar gains a `locked` prop), with a polite processing
summary. Restore the 'ready' phase from session.files when files were picked but
not processed, and move sourceName into the session. Set webkitdirectory
declaratively on every folder input so it survives phase-branch remounts. Reset
the folder input value after selection; move the .psychds-ignore write inside the
per-file try so an OPFS failure can't hang the run. Warn before unload while
there is unsaved work, cleared after a download. aria-live on the pick error and
processing summary.

Tests: real-page AppShell integration (load-once + failed-parse gating), batch
consistency (additive keeps both batches + persists name sets; replace confirms,
clears the store, fires reset), navigation-lock busy reporting, session
round-trip, zip basename handling, and Sidebar locked. Existing AppShell/
DataUpload tests updated for the load-status gating and zip basename change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg
Convert PreviewDrawer to a native <dialog> opened with showModal() so it gets a
real focus trap, Escape-to-close, and an inert backdrop (matching Sidebar),
replacing the div+role="dialog" with no focus management. Associate every
Variables control with a label (Type select, description textarea, and
levels/range groups via aria-labelledby). Review: sanitize the ${projectName}.zip
filename (strip characters illegal in filenames), disable the zip button while a
build is in flight, and wrap validation results in an aria-live region.

Tests: PreviewDrawer modal/Escape/backdrop behaviour under the jsdom dialog stub.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg
@changeset-bot

changeset-bot Bot commented Jul 6, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 259d51e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
frontend Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@jodeleeuw
jodeleeuw merged commit b3d9c6b into main Jul 20, 2026
2 checks passed
@jodeleeuw
jodeleeuw deleted the claude/frontend-wizard-fixes branch July 20, 2026 21:46
jodeleeuw added a commit that referenced this pull request Jul 20, 2026
…docs-site CLI reference

- Reconcile README with the merged Docusaurus docs site (#139): keep the
  monorepo intro/quick-start/development sections, point Documentation at
  metadata.jspsych.org and website/, drop the legacy docs/ links.
- Drop Node 20 support (maintainer decision): engines >=22 in all three
  packages, release workflow bumped 20.x -> 22.x to match the CI matrix
  (22/24) that npm test actually runs on.
- Widen the CLI's @jspsych/metadata range to '>=0.0.3 <0.2.0' so a future
  0.0.x or 0.1.x patch release cannot re-break npm ci (caret ranges on
  0.0.x match only that exact version).
- Sync website/docs/reference/cli-reference.md with the exit-code scheme
  and flag-validation behavior that landed in #135 (the docs/ copy was
  updated there; the site copy predated it).
- Resolve merge conflicts from main: take #136's DataUpload busy-state
  effect (webkitdirectory is now set via JSX spread, making #137's typed
  effect obsolete) and accept #136's deletion of global.d.ts.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jodeleeuw added a commit that referenced this pull request Jul 20, 2026
…, README (#137)

* ci: add typecheck and lint gates, refresh node test matrix

CI never ran tsc or eslint, so type errors and lint violations (including
an untypechecked packages/cli) could land silently. Add a `typecheck`
script to every package (tsc --noEmit, or the frontend's existing `tsc -b`
project setup) plus a root script that runs them all, wire both typecheck
and `lint --workspaces --if-present` into the Build job, and bump the pinned
Node 20.x (EOL) to a 22.x/24.x matrix for the Test job.

Fixing packages/cli's typecheck required skipLibCheck (an @types/node
version bump introduced generic TypedArrays that conflict with this
tsconfig's ES2018 lib under this TypeScript version — the frontend
tsconfig already carries the same flag) and a narrow `as any` cast in one
test assertion where the library's own return type is untyped `{}`.

Making frontend lint pass in CI required replacing a handful of
`any`-typed showSaveFilePicker/webkitdirectory workarounds with narrow
structural types, and scoping the lint script to src/ (tests aren't
typechecked by this project either, so this mirrors existing convention)
with a small --max-warnings allowance for the pre-existing react-hooks/
react-refresh warnings.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg

* fix(metadata,cli): correct package metadata

packages/metadata's repository/bugs URLs pointed at jspsych/jsPsych
(the wrong repo) and would leak that to npm on publish; point them at
jspsych/metadata instead. Reorder the exports map so the "types"
condition comes first — TS condition matching is order-sensitive, so a
trailing "types" entry can be skipped. Drop the dead build:types script
(references a nonexistent tsconfig.build.json and isn't part of the
build chain) and the vestigial husky devDependency (no .husky/ directory
in this repo).

packages/cli depended on "@jspsych/metadata": "*", which would happily
resolve to an incompatible future major. Pin it to the 0.0.x/0.1.x line
(0.1.0 is the pending release in PR #47) rather than a bare "^0.1.0":
until that release ships, the local workspace package is still at 0.0.3,
and a range that excludes it breaks `npm install` for local dev and CI
alike since npm workspaces only symlinks a dependency when the workspace
version satisfies the declared range.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg

* docs: expand root README, drop ignored build flag

The root README was two lines; expand it into a proper overview (what
the project is, the monorepo layout, a CLI quick-start, links to the
docs/ guides, and dev/contributing commands) without turning it into a
duplicate of the package-level READMEs.

Also drop --platform=node from the root `build` script: npm silently
ignores flags passed after `--workspaces` that aren't recognized by
`npm run`, so it had no effect (each package's own build script already
sets --platform=node where esbuild needs it).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg

* chore: add changeset

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QMjKvi9vHT3LfikGfW84Kg

* ci,docs: post-merge fixes — drop Node 20, widen metadata range, sync docs-site CLI reference

- Reconcile README with the merged Docusaurus docs site (#139): keep the
  monorepo intro/quick-start/development sections, point Documentation at
  metadata.jspsych.org and website/, drop the legacy docs/ links.
- Drop Node 20 support (maintainer decision): engines >=22 in all three
  packages, release workflow bumped 20.x -> 22.x to match the CI matrix
  (22/24) that npm test actually runs on.
- Widen the CLI's @jspsych/metadata range to '>=0.0.3 <0.2.0' so a future
  0.0.x or 0.1.x patch release cannot re-break npm ci (caret ranges on
  0.0.x match only that exact version).
- Sync website/docs/reference/cli-reference.md with the exit-code scheme
  and flag-validation behavior that landed in #135 (the docs/ copy was
  updated there; the site copy predated it).
- Resolve merge conflicts from main: take #136's DataUpload busy-state
  effect (webkitdirectory is now set via JSX spread, making #137's typed
  effect obsolete) and accept #136's deletion of global.d.ts.

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

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants