Add poster-board to the experiments realm (CS-12213)#5554
Conversation
Host Test Results 1 files 1 suites 2h 38m 48s ⏱️ Results for commit be87b70. Realm Server Test Results 1 files ± 0 1 suites +1 16m 42s ⏱️ + 16m 42s Results for commit be87b70. ± Comparison against earlier commit faafe37. |
a9eb6af to
faafe37
Compare
FigJam-style infinite-canvas board card, step 1 of CS-12193: pan, cursor-anchored wheel/pinch zoom with momentum (rig.gts camera engine), zoom toolbar, and layout-independent keyboard shortcuts, with live tests and a demo instance. Continues development in the monorepo; the boxel-catalog PRs (#666, #667) hold the earlier history. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
qunit/qunit-dom devDependencies plus the qunit-dom types entry give assert.dom its Assert augmentation, and the @cardstack/host/* path mapping resolves the host service registry so getService is typed — mirroring the catalog package's setup for .test.gts files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
faafe37 to
be87b70
Compare
habdelra
left a comment
There was a problem hiding this comment.
[Claude Code 🤖]
Reviewed with two lenses: the correctness/cleanup of the pan-zoom engine and card, and — because the file counts are dominated by it — the dependency/lockfile change riding along with the card.
Bottom line: the card and rig code are solid and I found no functional bugs. The one blocking issue is the pnpm-lock.yaml diff, which is ~3,900 lines of unrelated whole-graph re-resolution around a ~6-line intended change, and should be regenerated before merge.
What lands right
rig.gtsis a clean, DOM-free camera engine. Momentum is time-normalized (current * Math.exp(velocity * dt)withMath.pow(decay, dt/16.67)), programmatic zoom (zoomAtPoint/zoomCentered) callsstopAll()first so stale pinch velocity can't drift the camera off target, anddestroy()tears down both rAF loops and the pending timeout. The momentum-regression test pins exactly that stop-before-zoom contract.- The pointer-pan hardening carried over from the earlier review is all present and correct: primary-button-only, single-pointer-id sessions,
pointercancelsharing the up path with a guardedreleasePointerCapture, and the HUDclosest()bail so toolbar clicks don't start a pan. - Keyboard handling matches
event.code(physical key) and bails on ctrl/meta/alt, so Shift+-/0work and browser zoom passes through — and the test asserts both the shifted-character case and the ctrl pass-through.
Coverage gap (non-blocking, already disclosed)
The live tests only register when the realm's _mtimes is publicly readable, and the experiments realm returns 401, so this file runs in neither CI nor against experiments today — it was verified only against the catalog realm. Worth confirming the plan to make the realm readable (or add a CI path filter) so this becomes real coverage rather than manual-only. The runTests export shape does match the in-repo harness (packages/host/tests/live-test.js), so once discovery works it should register cleanly.
Recommendations
- Regenerate
pnpm-lock.yamlfrommainwith the pinned pnpm11.0.9, adding onlyqunit/qunit-dom— file-level comment onpnpm-lock.yaml. (blocking) - Drop the redundant
@ember/test-helpersoverride inpnpm-workspace.yaml— inline thread there. (part of #1) - Fix the two README references (in-repo harness path;
test:liveworking directory) — README thread. (nit) - Consider scoping the
windowkeydown listener to board focus when this graduates —poster-board.gtsthread. (nit)
Nothing adjacent/out-of-scope beyond the above. Submitted as COMMENT — approve / request-changes is the maintainer's call.
There was a problem hiding this comment.
[Claude Code 🤖] This lockfile diff is ~200× larger than the change that motivates it, and almost none of it is intended.
What the change actually needs. The only manifest change in this PR is adding qunit and qunit-dom to packages/experiments-realm's devDependencies. Both already resolve in main's lockfile (qunit@2.26.0, qunit-dom@3.5.1), so the correct diff is ~6 lines — the two catalog: entries added to the packages/experiments-realm: importer stanza.
What's actually in the diff. 3,911 changed lines. Two dominant sources, neither related to adding two devDeps:
- 1,824 new
supports-color@8.1.1peer-key threads —mainhas zero. Keys like@babel/core@7.29.7become@babel/core@7.29.7(supports-color@8.1.1), fanned out across the graph. esbuild@0.25.12threaded into webpack peer keys, duplicatingcss-loader/mini-css-extract-plugin/style-loaderentries.
Those are whole-graph re-resolution artifacts. They indicate the lockfile was regenerated under different pnpm tooling than the repo pins (mise.toml → npm:pnpm = 11.0.9). CI's pnpm install --frozen-lockfile (.github/actions/init/action.yml) only verifies the lockfile satisfies the manifests — not that it's minimal — which is why this is green despite the churn.
Why it matters. It silently reshapes transitive peer resolution for every package in the workspace, it will conflict with any concurrent lockfile-touching PR, and the next clean pnpm install on 11.0.9 is likely to churn it right back — lockfile thrash.
The fix. Regenerate from a clean main with the pinned pnpm (pnpm@11.0.9) after adding only the two devDeps, so the diff is just the packages/experiments-realm: importer entries. If there's a genuine lockfile-maintenance update worth landing, it belongs in its own PR rather than riding along with a card.
Class: regression introduced by this PR. Blocking — please don't merge the ~3,900-line re-resolution into main.
| allowedDeprecatedVersions: | ||
| babel-eslint: 10.1.0 | ||
| overrides: | ||
| "@ember/test-helpers": ^5.4.1 |
There was a problem hiding this comment.
[Claude Code 🤖] This override is a no-op and can be dropped. The default catalog already pins @ember/test-helpers: ^5.4.1 (line 35), and experiments-realm already declared "@ember/test-helpers": "catalog:" before this PR — the only manifest additions here are qunit/qunit-dom. The sole resolved version is 5.4.3 on both main and this branch, with or without the override, so it constrains nothing.
Its only lockfile effect is to move @ember/test-helpers out of the catalogs: snapshot into override-governed resolution — that's the deleted catalogs: entry you can see near the top of the pnpm-lock.yaml diff. Removing this line keeps test-helpers catalog-governed like every other test dep and shrinks the diff.
Class: cleanup, non-blocking — but part of the same lockfile regeneration as the file-level comment on pnpm-lock.yaml.
|
|
||
| Tests live in `poster-board.test.gts` and run on the catalog live-test | ||
| infrastructure (see the catalog repo's `tests/live/README.md` for the full details): the |
There was a problem hiding this comment.
[Claude Code 🤖] Two small doc-accuracy points now that this lives in the monorepo:
- "see the catalog repo's
tests/live/README.md" points out to a separate repo, but the harness that actually runs these tests is in-repo atpackages/host/tests/live-test.js— it fetches_mtimes, filters*.test.gts, and calls each module's exportedrunTests. Point the reader there. - The
pnpm test:livecommand at the bottom (line 44) resolves againstpackages/host— that's the only package where thetest:livescript is defined; there's none inexperiments-realm. It reads correctly only because it sits under thecd packages/hostfrom step 2b, so make the working directory explicit on that line to keep a reader who jumps straight to it from running it in the realm package.
Non-blocking, doc accuracy.
| handleInserted = (el: HTMLElement) => { | ||
| this.rootElement = el; | ||
| this.keydownHandler = this.handleKeyDown; | ||
| window.addEventListener('keydown', this.keydownHandler); |
There was a problem hiding this comment.
[Claude Code 🤖] Confirmation plus one scoping note.
The teardown is correct. willDestroy removes this exact listener reference, and SurfaceRig.destroy() → stopAll() cancels both rAF loops and clears the pending momentum timeout. Component here extends @glimmer/component, whose willDestroy fires on teardown, so nothing leaks — across app navigation or across rendering-test cases (each renderPosterBoard() gets a clean listener). Good.
The note. This is a window-level keydown listener guarded only against INPUT/TEXTAREA/contentEditable. It fires regardless of whether the board holds focus, so Shift+=/−/0 typed while focus is on unrelated host chrome will still zoom the board, and two isolated instances would both react to one keystroke. Fine for an experiment card; when this graduates, consider scoping to the board (a tabindex root with an element-level listener, or a focus/hover check) so the shortcut only acts when the board is the active surface. Non-blocking.
Adds
packages/experiments-realm/poster-board/: a FigJam-style infinite-canvas board card — pan, cursor-anchored wheel/pinch zoom with momentum, a zoom toolbar, and layout-independent keyboard shortcuts — with live tests and a demo instance.Closes CS-12213.
Moved from boxel-catalog
This work started as cardstack/boxel-catalog#666; we've since decided to continue development in the monorepo's experiments realm, so this PR carries that branch's final state. It includes all fixes from the code review on #666:
event.codeEqual/Minus/Digit0) instead ofevent.key, which reports shifted characters and left Shift+-/ Shift+0dead; ctrl/meta/alt chords bail out so browser zoom (Ctrl/Cmd+Shift+=) is never swalloweddeltaModelike the zoom path, so line-mode wheels (Firefox) pan at pixel-equivalent speedzoomCentered/zoomAtPoint) stops pending momentum first, so a pinch followed by the 100% button can't drift off targetThe remaining review follow-ups are ticketed: rig hardening (final-frame
onChange, dt clamp) on CS-12217, the finite dot grid on CS-12216. This supersedes the test copy in #5544.What's here
rig.gts— pure-TS pan/zoom camera engine: cursor-anchored zoom, drag-pan sessions with velocity sampling, rAF momentum with time-normalized inertia decayposter-board.gts— thePosterBoardcard def: single-transform plane, dot grid, zoom HUD (+ / % / − / 100% / Fit), Shift+=/−/0keyboard zoomposter-board.test.gts— 3 consolidated live tests / 13 assertions: toolbar rendering + all zoom controls, keyboard shortcuts (incl. browser-zoom pass-through), and the momentum regressionpackages/experiments-realmpackage.json/tsconfig gain qunit + qunit-dom types and the@cardstack/host/*path mapping so.test.gtsfiles type-check in the editor (mirrors the catalog package's setup)Testing
The experiments realm is not publicly readable locally, so the live-test loader can't discover tests against it yet (401 at
_mtimes). The identical test file was verified against the catalog realm: 3 tests / 13 assertions, all passing. Template lint (the package's CI lint) passes; addinglint:jsfor this package is ticketed as CS-12258.🤖 Generated with Claude Code