PosterBoard step 2: render cards at grid positions - #5560
Conversation
0d692f4 to
12b2bca
Compare
Render linked cards at grid positions Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> update test coverage fix types
b0aa3ee to
3d9f743
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3d9f743f2b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
backspace
left a comment
There was a problem hiding this comment.
I’m curious why the base cards don’t render, is there an issue for this? Might be something for me to look into
habdelra
left a comment
There was a problem hiding this comment.
[Claude Code 🤖]
Lens: I focused on the tile-rendering pipeline — how the board turns linked-card references into a search query, maps results back to slots, and handles the broken/missing paths — plus the test's fidelity to that pipeline, since the stub replaces the real search engine.
Bottom line: no blocking issues. The query construction and reference-based mapping are correct and match established call sites; the design is sound for an experiments-realm demo. All findings are non-blocking (one test-coverage gap, one cleanup, one design note for step 3).
What lands right (and is safe to rely on):
- The query —
searchEntryWireQueryFromQuery({}, { scope: 'cards' })+cardUrls: refs.map((r) => r + '.json')+filter: { eq: { htmlQuery: { eq: { format: 'fitted' } } } }— is exactly the shapehost-mode-service.tsandsearch-cards.tsuse.scope: 'cards'maps toentryType: 'instance'inrealm-index-query-engine.ts, dropping each card's dual-indexed file row, andcardUrlscompiles toi.url IN (...)on the.jsonfile URLs. Verified against those call sites and the wire grammar insearch-entry.ts. - Mapping by
entry.id === refrather than by result position is correct:entry.idis the extensionless card URL andlinkedRefsreads the same reference from membership; the reversed-order test pins this. - Reactivity is safe.
SearchEntriesResource.modifycompares queries with a deepisEqual, sotilesQueryreturning a fresh object each render does not re-issue the search; andtilesQueryreads only membership (never rig state), so pan/zoom don't re-run it. This is the subtle failure mode a display-on-a-canvas card could easily hit, and it's avoided. - Reading references via
getRelationshipMembershipState(a pure read that never triggerslazilyLoadLink) is the right call for a display board — tiles render from references without fetching the linked instances.
On the earlier automated P1 (window-scoped keydown): resolved in this head — the keydown listener is element-scoped ({{on 'keydown'}} on .poster-board-root), no window listener, so the double-fire concern doesn't apply to this commit. Noting so it isn't re-raised.
@backspace, on why @cardstack/base linked cards don't render: I ruled out the obvious cause — the base realm is searched. realm-server's availableRealms seeds { type: 'base', url: baseRealm.url }, and with no realms on the query the resource searches every available realm identifier, base included. So the issue is downstream of realm selection. The distinguishing factor is the reference form: the relative links (../../Author/jane-doe) resolve to canonical realm URLs, while @cardstack/base/Theme/... is the odd one out — most likely it isn't resolving to the canonical https://cardstack.com/base/... that both the cardUrls filter and the entry.id match expect (or those Theme cards simply have no prerendered fitted rendering). Quickest confirmation: log this.tilesQuery.cardUrls for the base slots against the entry.ids that come back — a mismatch confirms the reference-form theory. Host-side follow-up, not this PR.
Numbered recommendations (all non-blocking):
- Test the fallback branches, or soften the stub comment that claims they're covered — see the thread on the stub comment in
poster-board.test.gts. - Consolidate the repeated
getRelationshipMembershipState('cards')into one source of truth — see the thread onbrokenSlotAt. - Consider keying frame settings by card reference rather than slot index before step 3 persists positions — see the thread on
tilePlacements.
Adjacent, out of scope:
- The
tsconfig.jsonincludereaches into../host/node_modules/qunit-dom/dist/es/install.d.tsto pull qunit-dom's assertion augmentations into the experiments-realm project. It works because host depends on qunit-dom, but it couples this project's typecheck to host'snode_moduleslayout and qunit-dom's internal dist path; a shared test-types reference would be sturdier. (I couldn't confirm the exact path resolves in this environment —node_modulesisn't installed here.) - The board relies on
@overlaysdefaulting totruefor its tile chrome; passing it explicitly would be robust to a future default change. This matches the unguarded(component @context.searchResultsComponent)convention already used across experiments-realm (components/grid.gts,components/card-list.gts), so it's consistent as-is.
Generated by Claude Code
| // The board renders its tiles through `@context.searchResultsComponent`, which | ||
| // only the host app provides (operator mode / index / prerender routes). The | ||
| // stub below stands in for it: it captures each query the board issues and | ||
| // yields a test-controlled entry set, so tile mapping and fallbacks are | ||
| // exercised deterministically without a live prerender index. |
There was a problem hiding this comment.
[Claude Code 🤖] Non-blocking (test coverage / follow-up). This comment says tile mapping and fallbacks are exercised deterministically, but neither test in this file actually reaches a fallback branch:
brokenSlotAt→BrokenLinkTemplate(the error / not-found slot) never runs — both tests link only healthypresentcards.- The synthesized
missingEntryErrorDoc"not-found" placeholder ({{#unless results.isLoading}}+{{#if ref}}when no entry matches the ref) never runs — every linked card has a matchingstubEntry. results.isLoadingis hard-codedfalsein the stub, so the loading branch is never taken either.
These fallbacks are the subtlest logic in the component, so leaving them uncovered is where a regression would hide. The missing-entry placeholder is cheap to pin without a broken link: link a saved card, leave stubEntries empty (isLoading stays false), and assert [data-test-poster-board-broken-tile] renders with @state='not-found'. If you'd rather lean on the live tests for the error paths, drop "and fallbacks" from this comment so it doesn't read as covering something it doesn't. Not a blocker for an experiments-realm demo.
Generated by Claude Code
| brokenSlotAt = (index: number) => { | ||
| let owner = this.args.model as unknown as PosterBoard | undefined; | ||
| if (!owner) { | ||
| return undefined; | ||
| } | ||
| let { membership } = getRelationshipMembershipState(owner, 'cards'); | ||
| let rel = (membership ?? [])[index]; | ||
| return rel && (rel.kind === 'error' || rel.kind === 'not-found') | ||
| ? rel | ||
| : undefined; | ||
| }; |
There was a problem hiding this comment.
[Claude Code 🤖] Non-blocking (cleanup). getRelationshipMembershipState(owner, 'cards') is recomputed here on every brokenSlotAt(index) call, on top of the call in linkedRefs (which tilePlacements, tilesQuery, entryFor, and refAt all read through). Each call re-runs getField + peekAtField and rebuilds a fresh membership array.
To be fair to the design: Glimmer's per-reference caching keeps this off the pan/zoom hot path — these getters don't read rig state, so a pan frame doesn't recompute them — so it isn't a per-frame cost. It's the same array rebuilt several times per membership change, and it splits "read the cards' membership" across two call sites that must stay in sync.
Consider one source of truth, e.g. @cached get slots() returning getRelationshipMembershipState(owner, 'cards').membership ?? [] once; then linkedRefs is this.slots.map((s) => s.reference), brokenSlotAt(i) is this.slots[i] (test its kind), and entryFor / refAt index the same array. Purely a readability/efficiency cleanup.
Generated by Claude Code
| get tilePlacements(): TilePlacement[] { | ||
| let settings = this.args.model?.frameSettings ?? []; | ||
| return this.linkedRefs.map((_ref, index) => { | ||
| let setting = settings.find((s) => Number(s.cardIndex) === index); | ||
| // Number() guards against non-numeric values in hand-edited JSON | ||
| let x = Number(setting?.x); | ||
| let y = Number(setting?.y); | ||
| if (setting && Number.isFinite(x) && Number.isFinite(y)) { | ||
| return { index, x, y }; | ||
| } | ||
| return defaultPlacement(index); | ||
| }); | ||
| } |
There was a problem hiding this comment.
[Claude Code 🤖] Non-blocking (design note, relevant to step 3). Frame settings are matched to cards by array index (Number(s.cardIndex) === index, where index is the position in the cards linksToMany). That means removing or reordering a linked card silently reassigns every persisted position after it to a different card — the settings don't move with the card they were authored for.
Step 3 (drag-to-move) will start writing real positions into these records, so the index-keyed model becomes load-bearing exactly when a mismatch is most visible (a user drags card A, later removes an earlier card, and card A's saved position now belongs to card B). Worth considering keying each FrameSettingsField by the linked card's reference/id instead of its slot index, so a position tracks its card across insert/remove/reorder. Not this PR's blocker — flagging now because step 3 builds directly on this shape.
Generated by Claude Code
Render prerendered html versions of selected cards on the poster board.
Sample card is in experiments-realm. Just renders the attached cards.
The live card tests are in the same directory and are not part of CI. They can be run locally via
https://localhost:4200/tests/index.html?liveTest=true&realmURL=https%3A%2F%2Flocalhost%3A4201%2Fexperiments%2F&filter=poster-boardNote:
@cardstack/baselinked cards did not render. This seems to require a fix in host.