feat(board): support host-integrated Card details - #65
Merged
Conversation
📝 WalkthroughWalkthroughThe embed now accepts an optional card path. After the initial snapshot loads, the board opens a matching card once. Missing cards produce localized banner errors. Embed tests cover successful opening, dismissal, and missing-card handling. ChangesInitial card deep-link flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant EmbedFixture
participant JTypeBoard
participant BoardSurface
participant CardSnapshot
participant CardDialog
EmbedFixture->>JTypeBoard: pass initialCardPath
JTypeBoard->>BoardSurface: pass initialCardId
BoardSurface->>CardSnapshot: wait for loaded cards
CardSnapshot-->>BoardSurface: return matching card
BoardSurface->>CardDialog: open card once
CardDialog-->>BoardSurface: close card
BoardSurface-->>CardDialog: do not reopen initial card
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shared/components/board/BoardSurface.tsx`:
- Around line 152-163: The deep-link handling in BoardSurface must not own
mutable lifecycle state or call setSelectedId internally. Remove
initialCardHandled and the effect-driven internal selection; move one-time
initialCardId resolution to the platform owner, or make BoardSurface controlled
by receiving selection/open state through props and emitting selection/open
actions via callbacks while preserving the existing card-open behavior.
In `@tests/e2e/board-react-embed.spec.ts`:
- Around line 177-190: Update the test around the closed Card details dialog to
wait for a deterministic confirmed refresh rather than relying on
page.waitForTimeout. Force polling for this scenario and wait for the second
snapshot request, or observe an equivalent refresh signal, before asserting
dialog remains hidden; preserve the existing initial deep-link and close-button
assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3c06e8e4-115e-4930-9541-ec88c64f6748
⛔ Files ignored due to path filters (2)
packages/board-react/dist/index.d.tsis excluded by!**/dist/**packages/board-react/dist/index.jsis excluded by!**/dist/**
📒 Files selected for processing (7)
packages/board-react/README.mdpackages/board-react/src/JTypeBoard.tsxpackages/board-react/src/strings.tsshared/components/board/BoardSurface.tsxshared/components/board/types.tstests/e2e/board-react-embed.spec.tstests/fixtures/board-react-embed.tsx
Comment on lines
+152
to
+163
| const initialCardHandled = useRef(false); | ||
|
|
||
| useEffect(() => { | ||
| if (initialCardHandled.current || !initialCardId) return; | ||
| if (!cards.some((card) => card.id === initialCardId)) return; | ||
| initialCardHandled.current = true; | ||
| if (onCardOpen) { | ||
| onCardOpen(cards.find((card) => card.id === initialCardId)!); | ||
| return; | ||
| } | ||
| setSelectedId(initialCardId); | ||
| }, [cards, initialCardId, onCardOpen]); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Keep the deep-link lifecycle state outside BoardSurface.
Lines [152-163] add a private mutable guard and an internal selection update to a shared component. Lift the one-time resolution to the platform owner, or expose controlled selection/open callbacks so BoardSurface receives state through props and emits actions through callbacks.
As per coding guidelines, shared React components must follow the Props-in/Callbacks-out pattern and must not depend on a specific state pattern.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@shared/components/board/BoardSurface.tsx` around lines 152 - 163, The
deep-link handling in BoardSurface must not own mutable lifecycle state or call
setSelectedId internally. Remove initialCardHandled and the effect-driven
internal selection; move one-time initialCardId resolution to the platform
owner, or make BoardSurface controlled by receiving selection/open state through
props and emitting selection/open actions via callbacks while preserving the
existing card-open behavior.
Source: Coding guidelines
Comment on lines
+177
to
+190
| test("editable package embed opens an exact Card deep link once", async ({ page }) => { | ||
| await page.setViewportSize({ width: 1440, height: 900 }); | ||
| await page.goto("/tests/fixtures/board-react-embed.html?card=jcode%2Fcard-0.md"); | ||
| await expect(page.getByTestId("cloud-board-host").getByText("Jcode", { exact: true })).toBeVisible(); | ||
|
|
||
| const dialog = page.getByRole("dialog", { name: "Card details" }); | ||
| await expect(dialog).toBeVisible(); | ||
| await expect(dialog.getByPlaceholder("Untitled card")).toHaveValue(cardTitles[0]!); | ||
|
|
||
| await dialog.getByRole("button", { name: "Close" }).click(); | ||
| await expect(dialog).toBeHidden(); | ||
| await page.waitForTimeout(5_100); | ||
| await expect(dialog).toBeHidden(); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Wait for a confirmed refresh before checking the closed state.
page.waitForTimeout(5_100) does not prove that a poll occurred. The timer starts after the initial load, and JTypeBoard skips the timer while SSE is active. Count the second snapshot request, force polling for this test, or use a deterministic refresh signal before asserting that the dialog remains hidden.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/e2e/board-react-embed.spec.ts` around lines 177 - 190, Update the test
around the closed Card details dialog to wait for a deterministic confirmed
refresh rather than relying on page.waitForTimeout. Force polling for this
scenario and wait for the second snapshot request, or observe an equivalent
refresh signal, before asserting dialog remains hidden; preserve the existing
initial deep-link and close-button assertions.
cnjack
changed the title
This was referenced Jul 31, 2026
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
1 participant
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
Verification
pnpm buildinpackages/board-reactCloud integration
Cloud pins commit
a83f55b8112cc534f42c66e9f8f1b6274ce3706d, passesadditionalCardRoots={["jcode-automation"]}, and keeps Viewer embeds read-only.