feat(kanban): add editable swimlanes - #61
Merged
Merged
Conversation
|
Warning Review limit reached
Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (10)
📒 Files selected for processing (15)
📝 WalkthroughWalkthroughAdds persistent custom swimlanes across the shared board model, desktop, web, and board-react consumers. It introduces management, deletion, assignment, dangling-reference, and derived-lane conversion flows, with accompanying UI mockups, design documentation, unit tests, and Playwright coverage. ChangesCustom swimlane management
Estimated code review effort: 5 (Critical) | ~120 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)
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: 12
🧹 Nitpick comments (7)
packages/board-react/src/JTypeBoard.tsx (1)
262-279: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueIn-place mutation of
snap.metaByPathmutates a React state object.
snapRef.currentis the same object held insnapshotstate, so writing into its map updates rendered state invisibly (no re-render, and any memo keyed onsnapshotwon't see it). It works today because the map is only read for base-hash lookups, but a copy-on-write snapshot would be safer against future memoization.🤖 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 `@packages/board-react/src/JTypeBoard.tsx` around lines 262 - 279, Update saveDocContent so it does not mutate snap.metaByPath in place after client.saveDocument; create a copy-on-write snapshot/map update and keep the updated snapshot reference synchronized with the relevant state/ref used by JTypeBoard. Preserve the existing metadata merge and saved contentHash behavior for the relativePath entry.shared/components/board/BoardSurface.tsx (1)
392-426: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSilent drop of lane move/color actions while a mutation is in flight.
customLaneMutationBusyreturns early with no feedback, so a quick second "Move up" or color click is discarded invisibly. Consider disabling the affordance (pass a busy flag toBoardSwimlanes) rather than swallowing the intent.🤖 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 392 - 426, Update the custom lane controls around moveCustomLane and setCustomLaneColor so in-flight mutations expose a busy state to BoardSwimlanes and disable the move/color affordances while customLaneMutationBusy is true, instead of silently ignoring clicks. Preserve the existing mutation lock and reset it in finally.shared/components/board/StatusManagerDialog.tsx (1)
104-229: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftThis status action menu duplicates
statusMenuinBoardSwimlanes.tsxalmost verbatim.Rename / move / done-toggle / WIP / color swatches / delete are repeated in both files, including the color palette markup and disabled rules. Extracting one
StatusActionsMenucomponent (props: column, index, siblings, actions, portalClassName) would keep the two surfaces from drifting.🤖 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/StatusManagerDialog.tsx` around lines 104 - 229, Extract the duplicated status action menu markup from StatusManagerDialog and BoardSwimlanes into a shared StatusActionsMenu component, accepting column, index, siblings, actions, and portalClassName props. Move the rename, reorder, done-toggle, WIP, color palette, and delete behavior—including disabled rules—into that component, then replace both existing implementations with it while preserving their current behavior and styling.shared/components/board/BoardSwimlanes.tsx (1)
98-118: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueOptional: memoize the partition and reuse
groupValueOffor header counts.
partitionSwimlanesplus the per-columncards.filter(...)re-run on every render (including menu open/close and copy-state changes), and the header count re-implements the grouping switch already exported asgroupValueOf. AuseMemoon the grid plus deriving counts from it would remove both the duplication and the repeated O(cards × columns) work.Also applies to: 529-534
🤖 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/BoardSwimlanes.tsx` around lines 98 - 118, In BoardSwimlanes, memoize the partitionSwimlanes result and derive header counts from the memoized grid instead of rerunning per-column cards.filter calls on every render. Reuse the exported groupValueOf logic for grouping/count derivation, preserving the existing custom-lane handling and visible-lane behavior while removing the duplicated grouping switch.shared/components/board/SwimlaneManagerDialog.tsx (1)
338-341: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the index from the row key.
lane.keyis the stable identity this PR introduces; appending-${index}makes React treat a reordered lane as a new node, remounting inputs/popovers mid-edit. If the intent was to survive duplicate keys, note thatvalidateSwimlanesalready surfacesduplicate_swimlane_key.🤖 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/SwimlaneManagerDialog.tsx` around lines 338 - 341, Update the row key in the SwimlaneManagerDialog lane list to use only the stable lane.key value, removing the index suffix. Preserve validateSwimlanes duplicate-key handling and leave the row’s other attributes unchanged.shared/components/board/SwimlaneConversionDialog.tsx (1)
85-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded English plural for card counts. Both dialogs render
{count} <Trans>cards</Trans>, separating the number from the translatable unit so locales with different plural rules or word order can't be translated correctly. Use Lingui'sPluralmacro at each site.
shared/components/board/SwimlaneConversionDialog.tsx#L85-L87: replace with<Plural value={row.cardCount} one="# card" other="# cards" />.shared/components/board/SwimlaneManagerDialog.tsx#L436-L438: replace with<Plural value={counts.get(lane.key) ?? 0} one="# card" other="# cards" />(the “Used by” row near Line 552 uses the same 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/SwimlaneConversionDialog.tsx` around lines 85 - 87, Replace the separated count and hardcoded “cards” text with Lingui’s Plural macro in shared/components/board/SwimlaneConversionDialog.tsx lines 85-87, using row.cardCount with singular and plural card forms. Apply the same change in shared/components/board/SwimlaneManagerDialog.tsx lines 436-438, using counts.get(lane.key) ?? 0; also update the nearby “Used by” row in that file if it uses the same pattern.shared/components/board/SwimlaneDeleteDialog.tsx (1)
45-48: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueEffect reads
targetsbut omits it from deps. Iftargetsis computed after the lane is set (parent recompute),targetKeystays"". It also tripsreact-hooks/exhaustive-deps, which the repo otherwise silences explicitly. Either add the dep with a guard or annotate the intent.🤖 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/SwimlaneDeleteDialog.tsx` around lines 45 - 48, Update the useEffect in SwimlaneDeleteDialog to include targets in its dependency handling so targetKey refreshes when targets are recomputed after the lane changes. Add an appropriate guard if needed to avoid unintended resets, and preserve the existing mode reset behavior while satisfying the repository’s exhaustive-deps convention.
🤖 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 `@internal-docs/kanban/gaps-and-roadmap.md`:
- Line 42: Update the roadmap’s aggregate counts and priority scheduling
sections to remove C4 from pending scope or explicitly label those references as
historical, keeping them consistent with C4’s implemented status in the roadmap
table.
In `@packages/board-react/src/JTypeBoard.tsx`:
- Around line 347-365: Add a readOnly guard at the start of updateCards,
matching setConfig’s existing behavior, so bulk card updates return without
writing when the adapter is configured read-only; preserve the current update,
reload, progress, and error handling for writable configurations.
In `@services/jtype-web/frontend/src/pages/WebBoardView.tsx`:
- Around line 365-394: In the updateCards handler, remove the per-card
setMetaByPath call inside the updates loop and commit the accumulated
workingMeta once after processing, or rely on the subsequent load refresh.
Ensure missing metadata entries are handled so skipped update.cardId values do
not leave the swimlane conversion pending; preserve progress reporting and error
reload behavior.
In `@shared/components/board/BoardSurface.tsx`:
- Around line 276-375: Prevent the conversion effect around the shown useEffect
from retrying indefinitely when card updates make no observable progress. Track
consecutive passes with the same pending card update set, and after the defined
retry limit surface a conversionError and stop conversion instead of
unconditionally scheduling another pass; reset the no-progress tracking when the
pending set changes or progress occurs, while preserving normal migration
completion.
In `@shared/components/board/BoardSwimlanes.tsx`:
- Around line 443-448: Update the copy click handler in BoardSwimlanes around
the navigator.clipboard call to guard the clipboard object before invoking
writeText, and handle rejected writeText promises without creating an unhandled
rejection. Only update copied state and schedule its reset after a successful
copy; leave the behavior unchanged when the API is unavailable or the permission
is denied.
- Around line 400-405: Move the Details PopoverButton and its associated popover
out of the MenuItem structure in BoardSwimlanes, and control or trigger it
independently from the menu’s close-on-click behavior. Preserve the existing
Details label, icon, styling, and popover content while ensuring keyboard and
mouse activation reliably opens it after the menu closes.
- Around line 642-646: Update the status swimlane menu flow around statusMenu so
neighbour lookup and boundary checks use the status lane list (visibleLanes),
not the unrelated columns array. Pass the appropriate lane collection or
explicit neighbour array into statusMenu(lane, laneIndex), while preserving the
existing ordering behavior for other swimlane dimensions.
In `@shared/components/board/SwimlaneConversionDialog.tsx`:
- Around line 74-95: The empty-state message in the rows list should use the
same condition as the confirm-button logic. Update the conditional rendering
within the SwimlaneConversionDialog component to check rows.length === 0 instead
of total < 1, then remove the now-unused total value from the component if
nothing else references it.
In `@shared/components/board/SwimlaneDeleteDialog.tsx`:
- Around line 110-119: In SwimlaneDeleteDialog, move the ListboxSelect out of
the Radio option and render it as a sibling after the Radio, enabled only when
mode === "move". Keep its targetKey/onChange behavior intact, and ensure target
validation or updates remain outside the RadioGroup so the radio contains no
nested interactive control.
In `@shared/components/board/SwimlaneManagerDialog.tsx`:
- Around line 536-541: Update the clipboard handler around
navigator.clipboard.writeText so rejected writes are handled explicitly,
including permission-denied and unavailable-context cases. Preserve the existing
success behavior that sets copiedKey and clears it after the timeout, while
adding the appropriate user feedback through the dialog’s established UI state
or notification mechanism.
In `@shared/lib/board.ts`:
- Around line 154-156: Update the patch.custom loop in the board update flow to
skip entries whose keys are reserved core frontmatter fields, including
swimlaneKey, status, and other canonical fields. Apply only non-reserved custom
entries to next, preserving the existing null-to-empty-string conversion.
In `@tests/e2e/board-swimlanes.spec.ts`:
- Around line 94-100: Update the test around the swimlane removal flow to record
fixture action calls, then assert that the card-moving updateCards action occurs
before the setConfig call that removes the Growth lane. Keep the existing
final-state assertions, and identify the lane-removal call specifically rather
than asserting against unrelated configuration updates.
---
Nitpick comments:
In `@packages/board-react/src/JTypeBoard.tsx`:
- Around line 262-279: Update saveDocContent so it does not mutate
snap.metaByPath in place after client.saveDocument; create a copy-on-write
snapshot/map update and keep the updated snapshot reference synchronized with
the relevant state/ref used by JTypeBoard. Preserve the existing metadata merge
and saved contentHash behavior for the relativePath entry.
In `@shared/components/board/BoardSurface.tsx`:
- Around line 392-426: Update the custom lane controls around moveCustomLane and
setCustomLaneColor so in-flight mutations expose a busy state to BoardSwimlanes
and disable the move/color affordances while customLaneMutationBusy is true,
instead of silently ignoring clicks. Preserve the existing mutation lock and
reset it in finally.
In `@shared/components/board/BoardSwimlanes.tsx`:
- Around line 98-118: In BoardSwimlanes, memoize the partitionSwimlanes result
and derive header counts from the memoized grid instead of rerunning per-column
cards.filter calls on every render. Reuse the exported groupValueOf logic for
grouping/count derivation, preserving the existing custom-lane handling and
visible-lane behavior while removing the duplicated grouping switch.
In `@shared/components/board/StatusManagerDialog.tsx`:
- Around line 104-229: Extract the duplicated status action menu markup from
StatusManagerDialog and BoardSwimlanes into a shared StatusActionsMenu
component, accepting column, index, siblings, actions, and portalClassName
props. Move the rename, reorder, done-toggle, WIP, color palette, and delete
behavior—including disabled rules—into that component, then replace both
existing implementations with it while preserving their current behavior and
styling.
In `@shared/components/board/SwimlaneConversionDialog.tsx`:
- Around line 85-87: Replace the separated count and hardcoded “cards” text with
Lingui’s Plural macro in shared/components/board/SwimlaneConversionDialog.tsx
lines 85-87, using row.cardCount with singular and plural card forms. Apply the
same change in shared/components/board/SwimlaneManagerDialog.tsx lines 436-438,
using counts.get(lane.key) ?? 0; also update the nearby “Used by” row in that
file if it uses the same pattern.
In `@shared/components/board/SwimlaneDeleteDialog.tsx`:
- Around line 45-48: Update the useEffect in SwimlaneDeleteDialog to include
targets in its dependency handling so targetKey refreshes when targets are
recomputed after the lane changes. Add an appropriate guard if needed to avoid
unintended resets, and preserve the existing mode reset behavior while
satisfying the repository’s exhaustive-deps convention.
In `@shared/components/board/SwimlaneManagerDialog.tsx`:
- Around line 338-341: Update the row key in the SwimlaneManagerDialog lane list
to use only the stable lane.key value, removing the index suffix. Preserve
validateSwimlanes duplicate-key handling and leave the row’s other attributes
unchanged.
🪄 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: 3b48ba1b-a9c6-4790-9c22-9316ac051f94
⛔ Files ignored due to path filters (16)
internal-docs/kanban/mockups/swimlane-management-ui.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/**services/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 (27)
internal-docs/kanban/gaps-and-roadmap.mdinternal-docs/kanban/mockups/swimlane-management-ui.htmlinternal-docs/kanban/swimlane-management-design.mdinternal-docs/kanban/swimlane-management-ui-design.mdpackages/board-react/src/CardDetail.tsxpackages/board-react/src/JTypeBoard.tsxpackages/board-react/src/boardData.tspackages/board-react/src/strings.tsplaywright.board.config.tsservices/jtype-web/frontend/src/pages/WebBoardView.tsxshared/components/board/BoardPeek.tsxshared/components/board/BoardSurface.tsxshared/components/board/BoardSwimlanes.tsxshared/components/board/StatusManagerDialog.tsxshared/components/board/SwimlaneConversionDialog.tsxshared/components/board/SwimlaneDeleteDialog.tsxshared/components/board/SwimlaneManagerDialog.tsxshared/components/board/controls.tsxshared/components/board/types.tsshared/lib/board.tssrc/components/BoardView.tsxsrc/lib/types.tstests/e2e/board-swimlanes.spec.tstests/fixtures/board-swimlanes.htmltests/fixtures/board-swimlanes.tsxtests/unit/boardReactViewPrefs.spec.tstests/unit/boardSwimlanes.spec.ts
Comment on lines
+536
to
+541
| onClick={() => { | ||
| void navigator.clipboard?.writeText(lane.key).then(() => { | ||
| setCopiedKey(lane.key); | ||
| window.setTimeout(() => setCopiedKey(null), 1600); | ||
| }); | ||
| }} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Unhandled rejection when clipboard write is denied. navigator.clipboard.writeText rejects on permission denial or in non-secure contexts (e.g. embedded webviews); with only .then() this surfaces as an unhandled rejection and the user gets no feedback.
🛡️ Proposed fix
onClick={() => {
- void navigator.clipboard?.writeText(lane.key).then(() => {
- setCopiedKey(lane.key);
- window.setTimeout(() => setCopiedKey(null), 1600);
- });
+ void navigator.clipboard
+ ?.writeText(lane.key)
+ .then(() => {
+ setCopiedKey(lane.key);
+ window.setTimeout(() => setCopiedKey(null), 1600);
+ })
+ .catch(() =>
+ setRowError({ key: lane.key, message: t`Copy failed.` }),
+ );
}}📝 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
| onClick={() => { | |
| void navigator.clipboard?.writeText(lane.key).then(() => { | |
| setCopiedKey(lane.key); | |
| window.setTimeout(() => setCopiedKey(null), 1600); | |
| }); | |
| }} | |
| onClick={() => { | |
| void navigator.clipboard | |
| ?.writeText(lane.key) | |
| .then(() => { | |
| setCopiedKey(lane.key); | |
| window.setTimeout(() => setCopiedKey(null), 1600); | |
| }) | |
| .catch(() => | |
| setRowError({ key: lane.key, message: t`Copy failed.` }), | |
| ); | |
| }} |
🤖 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/SwimlaneManagerDialog.tsx` around lines 536 - 541,
Update the clipboard handler around navigator.clipboard.writeText so rejected
writes are handled explicitly, including permission-denied and
unavailable-context cases. Preserve the existing success behavior that sets
copiedKey and clears it after the timeout, while adding the appropriate user
feedback through the dialog’s established UI state or notification mechanism.
This was referenced Jul 30, 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
Design review
Validation
Summary by CodeRabbit
New Features
Bug Fixes