refactor(three-d-viewer): migrate iDevice to TypeScript - #40
Open
erseco wants to merge 21 commits into
Open
Conversation
Specify hotspots, guided navigation and single-choice questions for the 3D Viewer iDevice (issue exelearning#2153): versioned state, renderer-adapter architecture over the shared runtime, migration, accessibility, export and test strategy. No code changes yet.
Introduce state version 2 with an optional interaction block (markers, guided mode, questions). Add pure, idempotent normalization helpers (interaction/marker/anchor/camera/action/question) and single-choice grading, mirrored byte-for-byte between edition and export. Legacy state migrates transparently to a disabled, empty interaction; blob:/ data: URLs are never persisted. Colocated unit tests (TDD).
Add createInteractionLayer() to the shared eXe3DViewer runtime: a renderer-agnostic controller (markers, accessible content dialog with focus trap, single-choice questions with ARIA-live feedback, guided navigation) driven through two adapters — native model-viewer hotspots for GLB/GLTF and a DOM overlay projected per-frame for STL. Adds an onFrame hook and raycastFromPointer helper to the STL instance, and tears the layer down in destroy(). Colocated unit tests with a THREE stub cover projection, occlusion, placement, questions and cleanup.
renderView embeds enabled interaction state as an escaped JSON <script> block plus a static, escaped fallback marker list and (in guided mode) baked prev/next controls. renderBehaviour loads the shared runtime on demand and attaches the interaction layer per render path (native model-viewer hotspots or the STL instance, resolved once its mesh is ready). asset:// media survives into the block for the export rewriter; blob:/data: is never emitted. Colocated tests cover markup, escaping and guided controls.
Add a collapsed Interactions section to the editor: enable toggle, guided/labels/wrap flags, click-to-place marker creation, an editable marker list (reorder/edit/delete) and an accessible marker editor with per-action-type fields (information, image, video, link, single-choice question with options/feedback/attempts) and camera capture. The live preview attaches the shared interaction layer per render path. Adds the shared marker/dialog/question/guided-nav/fallback styles (mirrored in edition + export CSS) and editor-only authoring styles. Colocated editor unit tests.
…hange Add a Playwright spec covering the full flow: author an informational marker and a single-choice question, save, reload, open the preview, verify markers render with accessible labels + guided nav, the content dialog opens, and the question shows accessible feedback. Fix a data-loss bug the E2E surfaced: the generic display-option change listener ran readFormState(), which rebuilt state without the interaction block — wiping authored markers. readFormState now preserves interaction and the interaction toggles are excluded from the generic listener. Regression test added.
- Guided nav: bind prev/next once (re-render no longer stacks handlers so one click advances one step); re-enable buttons in wrap mode. - Fallback: keep the accessible text list visible when WebGL is unavailable and reveal it if STL boot never yields a mesh or the runtime fails to load, so assistive-tech users always reach marker content. - Marker button listeners bind on the (rebuilt/removed) button instead of the controller's lifetime array, removing a listener-ref leak. - Strip javascript:/vbscript:/blob:/data: from link URLs at normalize time (parity with image/video); sanitizer now also scrubs form/action/formaction and related URL attributes and drops <form>. - Add regression tests: wrap navigation, no-double-bind, setState active invalidation, link/image activation, model-viewer camera capture + placement, no-WebGL fallback, sanitizer form/action.
CI (headless Chromium) cannot create a WebGL context, so the Three.js STL scene and model-viewer projection do not render there. Adjust the E2E accordingly: - Activate markers via dispatchEvent instead of a real click — whether a model-viewer hotspot is visually clickable depends on WebGL projection, which is env-dependent and not what we assert. - Replace the STL 'renders in preview' test with a WebGL-independent one: author + persist an STL marker and assert the interaction data reaches the STL export path and the accessible fallback carries the content (the STL projection/placement math is unit-tested with a THREE stub). - Wait for the STL runtime instance with a generous, time-based budget (cold Three.js module load) before attaching / revealing the fallback.
Add optional SCORM scoring for question markers, reusing the shared gamification.scorm framework (no new scoring engine): - State gains isScorm (0/1/2) + weighted + textButtonScorm, normalized and round-tripped, mirrored between edition and export. - Editor shows an 'Assessment (SCORM)' section (the standard SCORM tab) when interactions are enabled and at least one question marker exists; everything is guarded so the iDevice never breaks without the framework. - The shared runtime reports each graded answer via a new onQuestionAnswered hook; in a SCORM export (body.exe-scorm) the export layer registers the activity and reports the fraction of question markers answered correctly through registerActivity + sendScoreNew. Colocated unit tests cover normalization, the data block, the scoring wiring and the runtime hook.
Bring in the convention-based TypeScript iDevice build from upstream PR exelearning#2147 so the 3D Viewer can be migrated to TypeScript without adding another bespoke build script. Imported verbatim from that PR: - scripts/build-idevices.ts + its spec (retargeted at three-d-viewer) - ADR-0006 (TypeScript iDevices build convention) - doc/development/idevices-typescript.md - slide/build.config.json, which replaces scripts/build-slide-editor.ts Ported by hand (no Interactive Video code, no dependency bumps): - package.json: typecheck:idevices / bundle:idevices / bundle:idevices:watch, wired into build:all before bundle:resources - vitest.config.mts: discover public/files/perm/idevices/**/src/**/*.spec.ts and measure coverage on base/*/src/**/*.ts instead of generated bundles - .gitignore + .github/workflows/e2e.yml: generated three-d-viewer bundles - scripts/build-resource-bundles.js: keep .map files out of resource ZIPs - AGENTS.md, .agents/skills/idevice/SKILL.md, mkdocs.yml The 3D Viewer ADR/SDD are renumbered to ADR-0007 / SDD-0002 so they do not collide with the IDs reserved by PR exelearning#2147. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01TDMjPiVVapwAo6DtpZZGfb
Move every maintained line of the 3D Viewer into `src/`, compiled by the
centralized TypeScript iDevice build into one edition IIFE and one export
IIFE. The generated bundles and their source maps are gitignored.
What was three hand-maintained JavaScript files with a duplicated schema
layer is now one source tree:
shared/ document model, schema v2, migration, colours, URLs, HTML
sanitizer, model-source detection, pure scoring
runtime/ instance registry, explicit lifecycle/teardown, asset
resolution, path resolution, lazy library loaders, the STL
Three.js scene, the `eXe3DViewer` facade
interactions/ the renderer-agnostic controller: marker state, accessible
dialog, questions, learner answer state, guided navigation,
text fallback
adapters/ pure projection/raycast maths plus the `<model-viewer>` and
STL implementations of one `MarkerAdapter` contract
edition/ `$exeDevice`: template, form, live preview, marker list,
marker editor, SCORM section
export/ `$threedviewer`: markup builders, boot, per-wrapper
controller, SCORM transport
Removed, because their source now lives in `src/`:
- `export/three-d-viewer-runtime.js` (no separate runtime file, no script
injection — both bundles carry a compiled copy and publish
`window.eXe3DViewer` idempotently, so one registry per page)
- the committed `edition/three-d-viewer.js` and `export/three-d-viewer.js`
Schema: one final v2 with an explicit `schemaVersion`. Original
unversioned content migrates straight to it and reopens with a disabled,
empty interaction layer; a future version is refused instead of being
opened and destructively overwritten. Persisted data is parsed as
`unknown`, validated and normalized — never cast.
Behaviour fixes:
- Question attempts and the resolved state now live per marker id in the
interaction controller, so the configured attempt limit applies for the
whole activity session instead of resetting each time a marker dialog is
reopened.
- The HTML sanitizer upper-cases tag names before the banned-element
check, so lowercase SVG/MathML foreign content (`<script>` inside
`<svg>`, `<foreignObject>`, `<annotation-xml>`) can no longer slip
through; `srcset`, `srcdoc` and `xlink:href` are URL-checked too.
- `ensureModelViewerLoaded` bounds its wait on `customElements.whenDefined`,
so a library that never registers cannot block the STL path.
`config.xml` is unchanged: the two bundles keep their existing filenames.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01TDMjPiVVapwAo6DtpZZGfb
…erage The JavaScript tests that lived beside the old `edition/` and `export/` sources are replaced by colocated `*.spec.ts` files next to the modules they cover: 36 files, 567 tests, 95% line coverage of `src/`. New ground the old suite did not cover: - generated-bundle contract tests that evaluate the ACTUAL compiled IIFEs and assert `$exeDevice`, `$threedviewer`, `ThreeDViewerExportObject` and `eXe3DViewer`, that neither bundle carries module syntax or chunk imports, that source maps are linked rather than inlined, and that a schema-v2 document renders end to end through the export bundle - schema-v2 migration: legacy → v2, future-version rejection, round-trip and idempotency, and that no blob:/data: URL is ever persisted - lifecycle and registry teardown, including multiple-instance isolation - the STL adapter's projection, occlusion, raycast and camera maths against a small deterministic Three.js stub - HTML sanitization of lowercase SVG/MathML foreign content Small Three.js and `<model-viewer>` stubs replace WebGL, so the suite is deterministic and needs no browser. E2E (`three-d-viewer-interactions.spec.ts`) gains a third marker with a one-attempt question that proves the allowance survives closing and reopening the dialog, an assertion that a resolved question stays resolved, and a new spec asserting interaction-free content still exports with no interaction payload, markers, guided controls or fallback list. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01TDMjPiVVapwAo6DtpZZGfb
Amend SDD-0002 and ADR-0007 — both still unpublished — so they describe what was built rather than the classic-script plan. The main correction: the SDD and the ADR both said the pure schema layer would be MIRRORED byte-for-byte between `edition/` and `export/`, with a `// mirror edition` marker and identical tests to police the drift. That is no longer true and no longer needed. The schema, the migration and the whole behavioural layer have a single TypeScript source under `src/`; the compiler puts a copy into each generated bundle. Duplicated bytes are accepted, duplicated maintained source is not — so the "duplication drift" risk is replaced by a "stale generated bundle" one, and the mitigation moves accordingly. Also documented: the `src/` tree and what each directory owns; one edition IIFE and one export IIFE, gitignored; no separate runtime JavaScript file; schema v2 with an explicit `schemaVersion`, the direct legacy → v2 migration and the safe rejection of future versions; the single canonical home for SCORM settings; the build, watch and typecheck commands; source maps and how they are kept out of resource ZIPs; the test layout including the generated-bundle contract; and the four browser globals the bundles publish. `doc/development/idevices-typescript.md` points at this iDevice as the reference for the convention now that it follows it. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01TDMjPiVVapwAo6DtpZZGfb
…kages The browser export path builds from `public/bundles/idevices.zip`, which `scripts/build-resource-bundles.js` now assembles without `.map` files, so a learner package produced in the browser never carries a source map. The server path (CLI commands and the external API) walks `public/files/perm/idevices/base/<name>/export/` directly and only filtered `.test.js` / `.spec.js`, so it would have shipped the ~230 KB `three-d-viewer.js.map` inside every HTML5, SCORM, EPUB and IMS package it generated. Filter `.map` alongside the tests so both paths agree. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01TDMjPiVVapwAo6DtpZZGfb
…rdown Two things the classic-script version did that the first pass of the migration dropped: - **Animation playback in the editor preview.** The old `$exeDevice` called `applyAnimationState()` from `updatePreview()` and from the model-viewer `load` handler, so ticking "Play animation" started the clip immediately. The preview now owns an `applyAnimation()` that does the same — pick the stored clip when the model still offers it, fall back to the first one, set the speed, play on repeat and announce it — and the device calls it after refreshing the animation picker. - **Canvas teardown.** `hideThreeJSCanvas()` used to hide the Three.js canvas when the author swapped an STL model for a GLB one. The runtime created that canvas, so `disposeInstance()` now removes it and un-hides the sibling `<model-viewer>`; otherwise a dead canvas stayed on top of the GLB preview. Also untrack `edition/three-d-viewer.js` and `export/three-d-viewer.js`. They were correctly `git rm --cached`-ed during the migration, but an intervening `git reset` put them back in the index, and .gitignore does not apply to already-tracked files. `git status` is clean after a build again. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01TDMjPiVVapwAo6DtpZZGfb
The `runtime/` entry in .gitignore was unanchored, so it matched a directory named `runtime` at any depth — including the TypeScript iDevice source directory `three-d-viewer/src/runtime/`. All 17 files there were silently excluded from the commit, so CI failed to type-check with 33 TS2307 "Cannot find module '../runtime/...'" errors while the working tree built fine. Anchor the rule to the repository root, which is what it was always for (it sits alongside `symfony_legacy` and `nestjs_legacy`), and restore the 17 lost source files. Add a regression test asserting no maintained iDevice source file is matched by an ignore rule. It uses `git check-ignore --no-index` so it tests the rules rather than the index, and therefore still catches a rule that would drop a newly added file. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01TDMjPiVVapwAo6DtpZZGfb
`test-unit-ci` was the only test target that did not depend on `bundle` — `test-unit`, `test-integration` and `test-frontend` all do. That went unnoticed while every iDevice bundle was committed. Now that `three-d-viewer/export/three-d-viewer.js` is generated, the backend spec that asserts the renderer does not mark it as a module found no file on a fresh checkout and failed in CI while passing locally, where the bundle happened to exist on disk. The test guards on the existence of the iDevice `export/` directory, which is still committed, so it could not detect the missing artifact either. Rather than weaken that assertion, give the CI target the same build precondition the other test targets already have. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01TDMjPiVVapwAo6DtpZZGfb
Bundle ReportBundle size has no change ✅ |
Resolve conflicts with upstream main: - doc/architecture/adr/records.md: keep ADR-0001/ADR-0042 from main and ADR-0006/ADR-0007 from this branch - package.json: auto-merged (iDevice build scripts + main dep/test updates)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrates the 3D Viewer implementation from upstream
exelearning/exelearning#2157to the centralized TypeScript iDevice conventionintroduced by
exelearning/exelearning#2147.Source
617e999, "Merge branch 'main' into 2153-3d-viewer-edevice").8c486ec).reference/pr-2157.Only the reusable parts of exelearning#2147 were taken —
scripts/build-idevices.tsand itsspec, ADR-0006,
doc/development/idevices-typescript.mdandslide/build.config.jsonverbatim, plus hand-ported edits topackage.json,vitest.config.mts,.gitignore,.github/workflows/e2e.yml,scripts/build-resource-bundles.js,AGENTS.md, the iDevice skill andmkdocs.yml. No Interactive Video source, tests, E2E helpers, workarea/importerchanges, dependency bumps or
bun.lockchanges were imported.Architecture
three-d-viewer/src/.export/three-d-viewer-runtime.jsis gone. Both bundles compile in the runtimeand publish
window.eXe3DVieweridempotently, so a page still has exactly oneinstance registry and nothing is injected at runtime any more.
config.xmlis unchanged: the two generated bundles keep their existingfilenames.
Schema
schemaVersionfield.disabled, empty interaction layer and renders exactly as before.
the form and
save()returns the original object untouched, so a newerdocument can never be opened and destructively overwritten.
development branch's internal
version: 2field is simply ignored; its shapeis compatible and normalizes as legacy content.
unknown, validated and normalized — never cast.scormblock);normalizeScorm()is the single adapter to the gamification framework'sflatter
isScorm/weighted/textButtonScormvocabulary.Improvements
strict,noUncheckedIndexedAccess,noImplicitOverride,useUnknownInCatchVariables); noanyanywhere insrc/.src/.Behaviour fixes
recreated every time a marker dialog was built, so the configured allowance
reset on each reopen. Answer and attempt state now live per marker id in the
interaction controller, so the limit applies to the marker for the whole
activity session. A correct answer also stays resolved, and the chosen option
is restored. Covered by unit tests and by the E2E spec.
check compared
tagNameagainst an upper-case table, which lowercaseSVG/MathML elements bypass (
<script>inside<svg>,<foreignObject>,<annotation-xml>). Tag names are now upper-cased before the lookup, thoseelements are banned explicitly, and
srcset,srcdocandxlink:hrefareURL-checked alongside
href/src. Still pure DOM traversal — no regexscrubbing.
customElements.whenDefined()never settles for an element that fails toload; the wait is now bounded.
browser export path builds from the resource ZIP, which now excludes
.map,but the server path (CLI + external API) walked the iDevice
export/directory and only filtered
.test.js/.spec.js..gitignoreno longer swallows iDevice source directories. Therepository's
runtime/rule was unanchored, so it matched a directory namedruntimeat any depth — including this iDevice'ssrc/runtime/. The workingtree built fine while the commit was missing 17 files. The rule is now
anchored to the repository root, which is what it was always for (it sits
beside
symfony_legacy/nestjs_legacy), and a regression test asserts thatno maintained iDevice source file is matched by an ignore rule. It uses
git check-ignore --no-index, so it tests the rules rather than the index andstill catches a rule that would drop a newly added file.
test-unit-ciwas theonly test target without a
bundledependency —test-unit,test-integrationandtest-frontendall have one. That was invisible whileevery iDevice bundle was committed. Now that
export/three-d-viewer.jsisgenerated,
IdeviceRenderer.spec.tsfound no file on a fresh checkout andfailed in CI while passing locally, where the bundle happened to exist on
disk. The spec guards on the iDevice
export/directory, which is stillcommitted, so it could not detect the missing artifact either. The assertion
is left intact and the CI target gets the build precondition the other test
targets already had.
Compatibility
<model-viewer>.(score is still correct question markers / total question markers on the 0..10
convention, with de-duplication by marker id, off by default, auto-save and
save-button modes, weighting, translated button text, and safe degradation
when the gamification helpers are absent).
data-configupgrade path for persisted HTML.$exeDevice,$threedviewer,ThreeDViewerExportObjectandeXe3DViewerare all still published, assigned explicitly from the entry points rather than
relying on the bundler's
globalName.Generated files
The following files are generated during build and intentionally ignored:
edition/three-d-viewer.jsedition/three-d-viewer.js.mapexport/three-d-viewer.jsexport/three-d-viewer.js.mapbuild:allrunstypecheck:idevices+bundle:idevicesbeforebundle:resources, somake bundle, the static build and every test targetregenerate them first.
scripts/build-resource-bundles.jskeeps.mapfilesout of the resource ZIPs, and the E2E workflow uploads both bundles as artifacts
because the runners get a fresh checkout.
Documentation
SDD-0002andADR-0007(renumbered from SDD-0001/ADR-0001 so they do notcollide with the IDs reserved by [proposal] Refactor interactive-video iDevice exelearning/exelearning#2147) are amended — both are still
unpublished. The statement that schema normalization is mirrored byte-for-byte
between
edition/andexport/is removed; there is one TypeScript source.ADR-0006anddoc/development/idevices-typescript.mdcome from [proposal] Refactor interactive-video iDevice exelearning/exelearning#2147 and arereferenced rather than restated.
Testing
Every command below was run on this branch.
bun installbun.lockunchangedbun scripts/build-idevices.ts --typecheck-only --only three-d-viewerbun scripts/build-idevices.ts --only three-d-viewerbun x vitest run --config vitest.config.mts public/files/perm/idevices/base/three-d-viewer/srcbun x vitest run --config vitest.config.mts(whole frontend suite)three-d-viewer/srcbun test scripts/build-idevices.spec.tsgit archive HEAD)typecheck:idevices,bundle:idevices,build:staticand the full Vitest suite all passbun test ./test/integrationmake fix/make lintbun x biome check public/files/perm/idevices/base/three-d-viewer/srcmake bundleidevices.zipcontainsthree-d-viewer/three-d-viewer.jsand zero.mapentriesgit status --shortemptybun x playwright test --project=chromium .../three-d-viewer-interactions.spec.tsGitHub Actions on this branch is green on every check:
build,build_and_test,merge-report,codecov/patch,codecov/bundles, and all twelvee2eshards(
chromium1–4,firefox1–4,static1–4).Remaining limitations
The notes below describe the local development sandbox only; CI is green on
every check for this branch.
(
home-is-where-art-is,latex-rendering,page-properties,file-manager*,block-reorder-stability,component-export-import,project-shared-delete,idevices/interactive-video, and others). Arepresentative subset was run on this branch and on the untouched
reference/pr-2157worktree: 13 failed / 24 passed on both, with anidentical set of failing test ids, so they are environmental. None of them
touch the 3D Viewer, and all twelve CI E2E shards pass.
bun test ./src ./test/helpersreports two failures in that sandbox(
convert.spec.tsinternal-error handling and theelp:convertunwritable-output-directory check). They reproduce identically on the
untouched
mainof this repository, which is green in CI, and they do notappear in this branch's CI logs — the sandbox runs as root, so a
"directory is not writable" assertion cannot hold. Not caused by this change.
make test-e2e-staticwas not run locally; the foure2e (static, …)CIshards cover it and are green.
make fixstill does not coverpublic/files/perm/idevices/base/*/src/.Extending it would pull pre-existing Biome findings in the Slide iDevice into
this PR, so it is left for a separate change; the 3D Viewer sources are
Biome-clean and can be checked with
bun x biome check public/files/perm/idevices/base/three-d-viewer/src.