feat(board): redesign card workflows and fix embeds - #63
Merged
Conversation
📝 WalkthroughWalkthroughThe pull request redesigns board card creation and detail interactions across shared UI, desktop, web, and React embeds. It adds relation-aware card identity, queued persistence, markdown renderer injection, read-only/open interception behavior, path-based parent references, and expanded unit and E2E coverage. ChangesCard actions redesign
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 14 UNAVAILABLE: Connection dropped 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/e2e/board-swimlanes.spec.ts (1)
11-21: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
HarnessState.cardsis missing aparentfield used by the new sub-card test.The type was extended with
columnKey,assignee,due,notes,tags, but the new "sub-card creation persists the parent" test (Line 206) assertsparent: "roadmap/offline"against this same shape. Addparent?: string | nullalongside the other new optional fields for consistency and IDE/type support.🩹 Proposed fix
cards: Array<{ id: string; title: string; priority?: string | null; columnKey?: string; assignee?: string | null; due?: string | null; notes?: string; tags?: Array<{ label: string }>; swimlaneKey?: string | null; + parent?: string | null; }>;🤖 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-swimlanes.spec.ts` around lines 11 - 21, Add an optional nullable parent field to the HarnessState.cards element type, alongside columnKey, assignee, due, notes, tags, and swimlaneKey, so the sub-card persistence assertion is type-safe.
🧹 Nitpick comments (2)
shared/components/board/controls.tsx (1)
184-184: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded hex swatch fallback in shared code.
"#d6d3d1"is a literal color inshared/; prefer a token fromshared/styles/tokens.css(or a semantic class on the swatch with the inline style applied only wheno.coloris set).As per coding guidelines, shared components "must use semantic Tailwind classes only ... and never hardcode hex values".
🤖 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/controls.tsx` at line 184, Update the swatch in the option-rendering component around the color style so it no longer hardcodes "`#d6d3d1`". Use the appropriate semantic Tailwind class or shared token from tokens.css as the fallback, while retaining the inline background color only when o.color is provided.Source: Coding guidelines
shared/components/board/BoardPeek.tsx (1)
261-293: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffDuplicate reference-resolution logic.
This mirrors
cardReferenceResolverinshared/lib/board.ts(same basename/suffix maps, same uniqueness gating) over a different shape. If the resolution rules ever change, the peek's labels andblockedCounts/childCardsByParentwill silently disagree. Consider exporting a generic resolver fromshared/lib/board.tsparameterized by a key extractor and reusing it here.🤖 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/BoardPeek.tsx` around lines 261 - 293, Replace the duplicated dependency reference-resolution logic in BoardPeek’s dependencyByBasename, dependencyBySuffix, and resolveDependencyValue with the shared cardReferenceResolver from shared/lib/board.ts. Export or generalize cardReferenceResolver to accept the dependency card shape via key extractors, then reuse it here so dependency labels and relationship calculations follow the same uniqueness and suffix rules.
🤖 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
`@services/jtype-web/frontend/src/help/content/articles/kanban-sub-cards-and-batch-edits.en.md`:
- Line 18: Update the sub-card navigation wording to use the current card detail
dialog terminology instead of the legacy peek-panel UI. Change the English
article at
services/jtype-web/frontend/src/help/content/articles/kanban-sub-cards-and-batch-edits.en.md:18
and the Chinese article at
services/jtype-web/frontend/src/help/content/articles/kanban-sub-cards-and-batch-edits.zh.md:18,
replacing “peek panel” and “侧边详情” respectively while preserving the navigation
instructions.
In `@services/jtype-web/frontend/src/lib/boardCardWrites.ts`:
- Around line 36-63: Serialize saveBoardCardPatch operations per relativePath
inside the board-card write module, ensuring each invocation rereads the latest
metadata and uses the completed prior save before applying its patch. Implement
per-path promise chaining or an equivalent internal queue, preserve the existing
null and metadata-update behavior, and update
services/jtype-web/frontend/src/pages/WebBoardView.tsx lines 381-396 only as
needed to rely on the internal serialization rather than caller ordering.
In `@services/jtype-web/frontend/src/pages/WebBoardView.tsx`:
- Line 5: Update WebBoardView’s markdown imports to use the local frontend
lib/markdown wrapper, including both renderMarkdownToHtml and renderToContainer
if the wrapper exports them; first verify that the wrapper exists and exposes
these helpers, and retain the direct shared import only if the wrapper is absent
or does not provide the required API.
- Around line 469-470: Update the remaining metadata lookups in the actions
object—deleteCards, duplicateCard, and deleteColumn—to read from
metaByPathRef.current instead of the stale metaByPath state. Preserve each
action’s existing behavior while ensuring rapid consecutive mutations use the
latest metadata.
In `@shared/components/board/BoardSurface.tsx`:
- Around line 1404-1436: Update the detail action handlers in BoardSurface’s
selected-card props so readOnly disables every mutation affordance: guard
onChange and onDelete the same way as onAddChild, while preserving their
existing behavior when readOnly is false. Ensure read-only details cannot edit
or delete cards even when rendered through peekComponent without onCardOpen.
In `@shared/components/board/CardCreateDialog.tsx`:
- Around line 60-73: Update the reset effect in CardCreateDialog so it runs only
on the dialog’s closed-to-open transition, not when initialStatus,
initialPriority, or initialAssignee changes while already open. Track the
previous open state or use an equivalent open-transition guard, while preserving
the existing field resets, submission-state cleanup, and title focus behavior.
---
Outside diff comments:
In `@tests/e2e/board-swimlanes.spec.ts`:
- Around line 11-21: Add an optional nullable parent field to the
HarnessState.cards element type, alongside columnKey, assignee, due, notes,
tags, and swimlaneKey, so the sub-card persistence assertion is type-safe.
---
Nitpick comments:
In `@shared/components/board/BoardPeek.tsx`:
- Around line 261-293: Replace the duplicated dependency reference-resolution
logic in BoardPeek’s dependencyByBasename, dependencyBySuffix, and
resolveDependencyValue with the shared cardReferenceResolver from
shared/lib/board.ts. Export or generalize cardReferenceResolver to accept the
dependency card shape via key extractors, then reuse it here so dependency
labels and relationship calculations follow the same uniqueness and suffix
rules.
In `@shared/components/board/controls.tsx`:
- Line 184: Update the swatch in the option-rendering component around the color
style so it no longer hardcodes "`#d6d3d1`". Use the appropriate semantic Tailwind
class or shared token from tokens.css as the fallback, while retaining the
inline background color only when o.color is provided.
🪄 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: 62bde8cd-7b05-420d-bb21-3fc898e91f7b
⛔ Files ignored due to path filters (18)
docs/implementation-notes/card-actions-ui-redesign.pngis excluded by!**/*.pngpackages/board-react/dist/index.d.tsis excluded by!**/dist/**packages/board-react/dist/index.jsis excluded by!**/dist/**packages/board-react/dist/style.cssis excluded by!**/dist/**packages/board-react/example/package-lock.jsonis excluded by!**/package-lock.jsonpackages/board-react/package-lock.jsonis excluded by!**/package-lock.jsonservices/jtype-web/frontend/src/i18n/locales/en/messages.pois excluded by!**/i18n/locales/**services/jtype-web/frontend/src/i18n/locales/ja/messages.pois excluded by!**/i18n/locales/**services/jtype-web/frontend/src/i18n/locales/ko/messages.pois excluded by!**/i18n/locales/**services/jtype-web/frontend/src/i18n/locales/zh/messages.pois excluded by!**/i18n/locales/**shared/i18n/locales/en/messages.mjsis excluded by!**/i18n/locales/**shared/i18n/locales/en/messages.pois excluded by!**/i18n/locales/**shared/i18n/locales/ja/messages.mjsis excluded by!**/i18n/locales/**shared/i18n/locales/ja/messages.pois excluded by!**/i18n/locales/**shared/i18n/locales/ko/messages.mjsis excluded by!**/i18n/locales/**shared/i18n/locales/ko/messages.pois excluded by!**/i18n/locales/**shared/i18n/locales/zh/messages.mjsis excluded by!**/i18n/locales/**shared/i18n/locales/zh/messages.pois excluded by!**/i18n/locales/**
📒 Files selected for processing (33)
docs/implementation-notes/card-actions-ui-redesign.mdpackages/board-react/README.mdpackages/board-react/package.jsonpackages/board-react/src/CardDetail.tsxpackages/board-react/src/JTypeBoard.tsxpackages/board-react/src/boardData.tsplaywright.board.config.tsservices/jtype-cli/src/main.rsservices/jtype-web/frontend/src/api.tsservices/jtype-web/frontend/src/help/content/articles/kanban-boards-and-cards.en.mdservices/jtype-web/frontend/src/help/content/articles/kanban-boards-and-cards.tsservices/jtype-web/frontend/src/help/content/articles/kanban-boards-and-cards.zh.mdservices/jtype-web/frontend/src/help/content/articles/kanban-sub-cards-and-batch-edits.en.mdservices/jtype-web/frontend/src/help/content/articles/kanban-sub-cards-and-batch-edits.zh.mdservices/jtype-web/frontend/src/lib/boardCardWrites.tsservices/jtype-web/frontend/src/pages/WebBoardView.tsxservices/jtype-web/src/mcp/kanban_tools.rsshared/components/board/BoardPeek.tsxshared/components/board/BoardSurface.tsxshared/components/board/CardCreateDialog.tsxshared/components/board/SwimlaneDeleteDialog.tsxshared/components/board/controls.tsxshared/components/board/index.tsshared/components/board/types.tsshared/lib/board.tssrc/components/BoardView.tsxtests/e2e/board-react-embed.spec.tstests/e2e/board-swimlanes.spec.tstests/fixtures/board-react-embed.htmltests/fixtures/board-react-embed.tsxtests/fixtures/board-swimlanes.tsxtests/unit/boardDependencies.spec.tstests/unit/webBoardSaveQueue.spec.ts
|
|
||
| You rarely type this by hand: | ||
|
|
||
| - In a card's peek panel, pick a **Parent** from the dropdown. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Update sub-card navigation wording to match the dialog redesign.
The implementation note says existing cards open a wide detail dialog, but these localized articles still direct users to the old side-peek UI.
services/jtype-web/frontend/src/help/content/articles/kanban-sub-cards-and-batch-edits.en.md#L18-L18: replace “peek panel” with “card detail dialog” or the current UI name.services/jtype-web/frontend/src/help/content/articles/kanban-sub-cards-and-batch-edits.zh.md#L18-L18: replace侧边详情with the current card-detail-dialog terminology.
📍 Affects 2 files
services/jtype-web/frontend/src/help/content/articles/kanban-sub-cards-and-batch-edits.en.md#L18-L18(this comment)services/jtype-web/frontend/src/help/content/articles/kanban-sub-cards-and-batch-edits.zh.md#L18-L18
🤖 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
`@services/jtype-web/frontend/src/help/content/articles/kanban-sub-cards-and-batch-edits.en.md`
at line 18, Update the sub-card navigation wording to use the current card
detail dialog terminology instead of the legacy peek-panel UI. Change the
English article at
services/jtype-web/frontend/src/help/content/articles/kanban-sub-cards-and-batch-edits.en.md:18
and the Chinese article at
services/jtype-web/frontend/src/help/content/articles/kanban-sub-cards-and-batch-edits.zh.md:18,
replacing “peek panel” and “侧边详情” respectively while preserving the navigation
instructions.
Comment on lines
+36
to
+63
| export async function saveBoardCardPatch( | ||
| metaRef: WebBoardCardMetaRef, | ||
| relativePath: string, | ||
| patch: Partial<BoardViewCard>, | ||
| saveDocument: SaveCardDocument, | ||
| ): Promise<Map<string, WebBoardCardMeta> | null> { | ||
| const meta = metaRef.current.get(relativePath) | ||
| if (!meta) return null | ||
|
|
||
| const content = applyBoardCardPatch(meta.content, patch) | ||
| const saved = await saveDocument({ | ||
| relativePath, | ||
| content, | ||
| baseContentHash: meta.contentHash, | ||
| baseContent: meta.content, | ||
| }) | ||
| const currentMeta = metaRef.current.get(relativePath) | ||
| if (!currentMeta) return null | ||
|
|
||
| const nextMeta = new Map(metaRef.current) | ||
| nextMeta.set(relativePath, { | ||
| ...currentMeta, | ||
| content: saved.content, | ||
| contentHash: saved.contentHash, | ||
| }) | ||
| metaRef.current = nextMeta | ||
| return nextMeta | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg 'services/jtype-web/frontend/src/(lib/boardCardWrites\.ts|pages/WebBoardView\.tsx|.*CardDetail|.*BoardSurface)' || true
echo
echo "boardCardWrites outline/content:"
if [ -f services/jtype-web/frontend/src/lib/boardCardWrites.ts ]; then
wc -l services/jtype-web/frontend/src/lib/boardCardWrites.ts
cat -n services/jtype-web/frontend/src/lib/boardCardWrites.ts
fi
echo
echo "WebBoardView relevant section:"
if [ -f services/jtype-web/frontend/src/pages/WebBoardView.tsx ]; then
wc -l services/jtype-web/frontend/src/pages/WebBoardView.tsx
sed -n '340,420p' services/jtype-web/frontend/src/pages/WebBoardView.tsx | cat -n
fi
echo
echo "Search updateCard/saveBoardCardPatch usages:"
rg -n "updateCard|saveBoardCardPatch|setActions|actions\.updateCard|CardDetail|BoardSurface" services/jtype-web/frontend/src -S || trueRepository: cnjack/jtype
Length of output: 1913
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Current directory:"
pwd
echo "Files:"
git ls-files 2>/dev/null | rg 'services/jtype-web/frontend/src/(lib/boardCardWrites\.ts|pages/WebBoardView\.tsx|.*CardDetail|.*BoardSurface)' || true
echo
echo "boardCardWrites outline/content:"
if [ -f services/jtype-web/frontend/src/lib/boardCardWrites.ts ]; then
wc -l services/jtype-web/frontend/src/lib/boardCardWrites.ts
cat -n services/jtype-web/frontend/src/lib/boardCardWrites.ts
fi
echo
echo "WebBoardView relevant section:"
if [ -f services/jtype-web/frontend/src/pages/WebBoardView.tsx ]; then
wc -l services/jtype-web/frontend/src/pages/WebBoardView.tsx
sed -n '340,420p' services/jtype-web/frontend/src/pages/WebBoardView.tsx | cat -n
fi
echo
echo "Search updateCard/saveBoardCardPatch usages:"
rg -n "updateCard|saveBoardCardPatch|setActions|actions\.updateCard|CardDetail|BoardSurface" services/jtype-web/frontend/src -S || trueRepository: cnjack/jtype
Length of output: 1913
Avoid relying on callers to serialize per-card saves. saveBoardCardPatch optimistically reads meta.contentHash/meta.content, then writes with those stale base values if two patches for the same relativePath can start before the first save awaits. Serialize these saves inside saveBoardCardPatch per path so correctness doesn’t depend on BoardSurface/CardDetail callers always awaiting one update before firing the next.
📍 Affects 2 files
services/jtype-web/frontend/src/lib/boardCardWrites.ts#L36-L63(this comment)services/jtype-web/frontend/src/pages/WebBoardView.tsx#L381-L396
🤖 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 `@services/jtype-web/frontend/src/lib/boardCardWrites.ts` around lines 36 - 63,
Serialize saveBoardCardPatch operations per relativePath inside the board-card
write module, ensuring each invocation rereads the latest metadata and uses the
completed prior save before applying its patch. Implement per-path promise
chaining or an equivalent internal queue, preserve the existing null and
metadata-update behavior, and update
services/jtype-web/frontend/src/pages/WebBoardView.tsx lines 381-396 only as
needed to rely on the internal serialization rather than caller ordering.
| import { BoardSettingsDialog } from '../components/BoardSettingsDialog' | ||
| import { useConfirm, usePrompt } from '@shared/components/PromptDialogContext' | ||
| import { parseFrontmatter, writeFrontmatter } from '@shared/lib/frontmatter' | ||
| import { renderMarkdownToHtml, renderToContainer } from '@shared/lib/markdown' |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Markdown helpers imported directly from @shared/lib/markdown, bypassing the local wrapper.
Per coding guidelines, web-service markdown rendering must import renderMarkdownToHtml from the local services/jtype-web/frontend/src/lib/markdown.ts (kept synchronized with desktop's src/lib/markdown.ts), not directly from @shared/lib/markdown. Please confirm whether that local wrapper still exists/is the intended import path, or whether the guideline is stale now that markdown lives in shared/lib/.
#!/bin/bash
# Check whether the local web markdown wrapper still exists and what other web files import.
fd markdown.ts services/jtype-web/frontend/src/lib
rg -nP "from '(`@shared/lib/markdown`|\.\./lib/markdown|\./lib/markdown)'" services/jtype-web/frontend/src🤖 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 `@services/jtype-web/frontend/src/pages/WebBoardView.tsx` at line 5, Update
WebBoardView’s markdown imports to use the local frontend lib/markdown wrapper,
including both renderMarkdownToHtml and renderToContainer if the wrapper exports
them; first verify that the wrapper exists and exposes these helpers, and retain
the direct shared import only if the wrapper is absent or does not provide the
required API.
Comment on lines
469
to
+470
| deleteCard: async (card) => { | ||
| const meta = metaByPath.get(card.id) | ||
| const meta = metaByPathRef.current.get(card.id) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Sibling mutation paths still use stale metaByPath state instead of the ref.
deleteCard here now reads metaByPathRef.current for freshness, but deleteCards, duplicateCard, and deleteColumn (further below in this same actions object) still call metaByPath.get(...) — the very state staleness this migration was meant to fix. Rapid consecutive actions before React commits metaByPath can still read outdated content for these three paths.
♻️ Apply the same fix to the remaining sites
- deleteCards: async (cardsToDelete) => {
- ...
- const meta = metaByPath.get(card.id)
+ deleteCards: async (cardsToDelete) => {
+ ...
+ const meta = metaByPathRef.current.get(card.id)(same swap for duplicateCard and deleteColumn)
🤖 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 `@services/jtype-web/frontend/src/pages/WebBoardView.tsx` around lines 469 -
470, Update the remaining metadata lookups in the actions object—deleteCards,
duplicateCard, and deleteColumn—to read from metaByPathRef.current instead of
the stale metaByPath state. Preserve each action’s existing behavior while
ensuring rapid consecutive mutations use the latest metadata.
Comment on lines
+1404
to
+1436
| onAddChild={ | ||
| readOnly | ||
| ? undefined | ||
| : async (title) => { | ||
| await cardUpdateQueues.current.get(selected.id); | ||
| // Status sub-cards enter the first workflow state; other | ||
| // dimensions keep the new card beside its parent. | ||
| const startLane = | ||
| groupKey === "status" | ||
| ? config.columns[0]?.key ?? selected.columnKey | ||
| : boardLaneValueOf(selected, config); | ||
| await actions.createCard(startLane, title, { | ||
| parent: cardRelationKey(selected), | ||
| }); | ||
| } | ||
| } | ||
| loadNotes={loadNotes} | ||
| onUploadAttachment={onUploadAttachment} | ||
| loadComments={loadComments} | ||
| addComment={addComment} | ||
| updateComment={updateComment} | ||
| deleteComment={deleteComment} | ||
| toggleReaction={toggleReaction} | ||
| resolveComment={resolveComment} | ||
| currentUser={currentUser} | ||
| loadActivity={loadActivity} | ||
| renderMarkdownToContainer={renderMarkdownToContainer} | ||
| renderMarkdownToHtml={renderMarkdownToHtml} | ||
| portalClassName={portalClassName} | ||
| onChange={(patch) => void queueCardUpdate(selected.id, patch)} | ||
| onClose={() => setSelectedId(null)} | ||
| onDelete={() => void actions.deleteCard(selected)} | ||
| onOpenFull={actions.openCardFull ? () => actions.openCardFull?.(selected) : undefined} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
readOnly only gates onAddChild; edits and delete still reach the detail.
onAddChild is disabled under readOnly, but onChange and onDelete are wired unconditionally, so a read-only host that supplies peekComponent without onCardOpen gets a fully editable/deletable detail — contrary to the documented readOnly contract ("hides every mutation affordance").
🛡️ Suggested guard
- onChange={(patch) => void queueCardUpdate(selected.id, patch)}
+ onChange={readOnly ? () => {} : (patch) => void queueCardUpdate(selected.id, patch)}
onClose={() => setSelectedId(null)}
- onDelete={() => void actions.deleteCard(selected)}
+ onDelete={readOnly ? () => {} : () => void actions.deleteCard(selected)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| onAddChild={ | |
| readOnly | |
| ? undefined | |
| : async (title) => { | |
| await cardUpdateQueues.current.get(selected.id); | |
| // Status sub-cards enter the first workflow state; other | |
| // dimensions keep the new card beside its parent. | |
| const startLane = | |
| groupKey === "status" | |
| ? config.columns[0]?.key ?? selected.columnKey | |
| : boardLaneValueOf(selected, config); | |
| await actions.createCard(startLane, title, { | |
| parent: cardRelationKey(selected), | |
| }); | |
| } | |
| } | |
| loadNotes={loadNotes} | |
| onUploadAttachment={onUploadAttachment} | |
| loadComments={loadComments} | |
| addComment={addComment} | |
| updateComment={updateComment} | |
| deleteComment={deleteComment} | |
| toggleReaction={toggleReaction} | |
| resolveComment={resolveComment} | |
| currentUser={currentUser} | |
| loadActivity={loadActivity} | |
| renderMarkdownToContainer={renderMarkdownToContainer} | |
| renderMarkdownToHtml={renderMarkdownToHtml} | |
| portalClassName={portalClassName} | |
| onChange={(patch) => void queueCardUpdate(selected.id, patch)} | |
| onClose={() => setSelectedId(null)} | |
| onDelete={() => void actions.deleteCard(selected)} | |
| onOpenFull={actions.openCardFull ? () => actions.openCardFull?.(selected) : undefined} | |
| onAddChild={ | |
| readOnly | |
| ? undefined | |
| : async (title) => { | |
| await cardUpdateQueues.current.get(selected.id); | |
| // Status sub-cards enter the first workflow state; other | |
| // dimensions keep the new card beside its parent. | |
| const startLane = | |
| groupKey === "status" | |
| ? config.columns[0]?.key ?? selected.columnKey | |
| : boardLaneValueOf(selected, config); | |
| await actions.createCard(startLane, title, { | |
| parent: cardRelationKey(selected), | |
| }); | |
| } | |
| } | |
| loadNotes={loadNotes} | |
| onUploadAttachment={onUploadAttachment} | |
| loadComments={loadComments} | |
| addComment={addComment} | |
| updateComment={updateComment} | |
| deleteComment={deleteComment} | |
| toggleReaction={toggleReaction} | |
| resolveComment={resolveComment} | |
| currentUser={currentUser} | |
| loadActivity={loadActivity} | |
| renderMarkdownToContainer={renderMarkdownToContainer} | |
| renderMarkdownToHtml={renderMarkdownToHtml} | |
| portalClassName={portalClassName} | |
| onChange={readOnly ? () => {} : (patch) => void queueCardUpdate(selected.id, patch)} | |
| onClose={() => setSelectedId(null)} | |
| onDelete={readOnly ? () => {} : () => void actions.deleteCard(selected)} | |
| onOpenFull={actions.openCardFull ? () => actions.openCardFull?.(selected) : undefined} |
🤖 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 1404 - 1436, Update
the detail action handlers in BoardSurface’s selected-card props so readOnly
disables every mutation affordance: guard onChange and onDelete the same way as
onAddChild, while preserving their existing behavior when readOnly is false.
Ensure read-only details cannot edit or delete cards even when rendered through
peekComponent without onCardOpen.
Comment on lines
+60
to
+73
| useEffect(() => { | ||
| if (!open) return; | ||
| setTitle(""); | ||
| setNotes(""); | ||
| setStatus(initialStatus); | ||
| setPriority(initialPriority); | ||
| setAssignee(initialAssignee); | ||
| setDue(""); | ||
| setTags([]); | ||
| setTagText(""); | ||
| setSubmitting(false); | ||
| setSubmitError(""); | ||
| window.setTimeout(() => titleRef.current?.focus(), 0); | ||
| }, [initialAssignee, initialPriority, initialStatus, open]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reset also fires on initial* changes while the dialog is open.
The effect depends on initialStatus/initialPriority/initialAssignee, which in BoardSurface are derived from config.columns and addingIn. A background board reload that changes the first column key would clear a half-typed title/description. Gate the reset on the open transition instead.
♻️ Reset only when the dialog opens
+ const wasOpen = useRef(false);
useEffect(() => {
- if (!open) return;
+ if (!open) {
+ wasOpen.current = false;
+ return;
+ }
+ if (wasOpen.current) return;
+ wasOpen.current = true;
setTitle("");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEffect(() => { | |
| if (!open) return; | |
| setTitle(""); | |
| setNotes(""); | |
| setStatus(initialStatus); | |
| setPriority(initialPriority); | |
| setAssignee(initialAssignee); | |
| setDue(""); | |
| setTags([]); | |
| setTagText(""); | |
| setSubmitting(false); | |
| setSubmitError(""); | |
| window.setTimeout(() => titleRef.current?.focus(), 0); | |
| }, [initialAssignee, initialPriority, initialStatus, open]); | |
| const wasOpen = useRef(false); | |
| useEffect(() => { | |
| if (!open) { | |
| wasOpen.current = false; | |
| return; | |
| } | |
| if (wasOpen.current) return; | |
| wasOpen.current = true; | |
| setTitle(""); | |
| setNotes(""); | |
| setStatus(initialStatus); | |
| setPriority(initialPriority); | |
| setAssignee(initialAssignee); | |
| setDue(""); | |
| setTags([]); | |
| setTagText(""); | |
| setSubmitting(false); | |
| setSubmitError(""); | |
| window.setTimeout(() => titleRef.current?.focus(), 0); | |
| }, [initialAssignee, initialPriority, initialStatus, open]); |
🤖 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/CardCreateDialog.tsx` around lines 60 - 73, Update
the reset effect in CardCreateDialog so it runs only on the dialog’s
closed-to-open transition, not when initialStatus, initialPriority, or
initialAssignee changes while already open. Track the previous open state or use
an equivalent open-transition guard, while preserving the existing field resets,
submission-state cleanup, and title focus behavior.
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
jtype-board-reactas 0.1.1 with scoped portal CSS and a real Cloud-sized embed regressionRoot cause
Cloud layout was bounded correctly. Package 0.1.0 routed every card open through its legacy read-only detail even though
readOnlydefaults to false. The shared editor was never mounted. Rapid Web field edits also reused a closed-over metadata snapshot before React rerendered.Impact matrix
Acceptance criteria
onCardOpenembed paths remain distinctVerification
npm run test:unit— 65 passednpm run build— Desktop passedcd services/jtype-web/frontend && npm run build— Web passedcd packages/board-react && npm run build && npm test— 21 package unit + 3 embed E2E passednpx playwright test --config playwright.board.config.ts --workers=1— 16 passedcargo check --manifest-path services/jtype-cli/Cargo.toml— passedcargo check --manifest-path services/jtype-web/Cargo.toml— passedDocumentation and release
packages/board-react/distis rebuilt for 0.1.1.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests