feat: introduce Sortable component with DnD capabilities and Storybook examples#54
feat: introduce Sortable component with DnD capabilities and Storybook examples#54
Conversation
…k examples - Added `Sortable`, `SortableItem`, `SortableDragHandle`, `SortableProvider`, and `DragOverlay` components using `@dnd-kit` for drag-and-drop functionality. - Integrated examples for vertical, horizontal, grid, drag handle, multiple lists, and custom UI in `Sortable.stories.tsx` within Storybook. - Exported the new components in `ui/index.ts` for external use. - Updated `package.json` to include `@dnd-kit` dependencies.
📝 WalkthroughWalkthroughThis pull request introduces a drag-and-drop sortable component system powered by dnd-kit. It includes core components (Sortable, SortableItem, SortableDragHandle, SortableProvider), comprehensive Storybook documentation with eight usage examples, and updates to package dependencies and public API exports. Changes
Sequence DiagramsequenceDiagram
actor User
participant SortableItem
participant DndContext as DnD Context<br/>(Sensors & Collision)
participant Sortable
participant DragOverlay
User->>SortableItem: Activate drag via<br/>SortableDragHandle
SortableItem->>DndContext: Register drag start<br/>(activeId set)
DndContext->>Sortable: Notify drag active
Sortable->>DragOverlay: Render overlay for<br/>active item
User->>SortableItem: Drag over items
DndContext->>Sortable: Track drag position<br/>(collision detection)
User->>DndContext: Drop item
DndContext->>Sortable: Trigger drag end
Sortable->>Sortable: Calculate new order<br/>(arrayMove)
Sortable->>Sortable: Invoke onValueChange<br/>& onMove callbacks
Sortable->>DragOverlay: Clear overlay
SortableItem->>User: Render reordered items
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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: 3
🧹 Nitpick comments (6)
src/components/ui/sortable.tsx (3)
1-1:"use strict"is unnecessary in TypeScript/ESM files.TypeScript modules and ES modules are always in strict mode. This directive is a no-op here.
🧹 Remove unnecessary directive
-"use strict"; - import * as React from "react";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/sortable.tsx` at line 1, Remove the unnecessary "use strict" directive at the top of this module: delete the standalone string literal "use strict" from src/components/ui/sortable.tsx since TypeScript/ESM modules are already strict; no other changes required.
232-250:SortableProviderdoesn't exposeonDragEnd/onDragStarthandling — document the intended usage.
SortableProvideris a bareDndContextwrapper with no built-in state management for active items or reordering. This is presumably intended for advanced use cases (e.g., cross-list dragging), but there are no stories or documentation showing how a consumer would wire it up. A doc comment or a corresponding Storybook story usingSortableProviderwith<Sortable container={false}>would help clarify the intended pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/sortable.tsx` around lines 232 - 250, SortableProvider currently wraps DndContext but doesn't document or demonstrate how consumers should handle drag lifecycle (onDragEnd/onDragStart) or manage active item/reordering state; add a short doc comment above the SortableProvider function explaining that it intentionally forwards DndContext props and that consumers must supply handlers like onDragEnd/onDragStart (and maintain item order/active id) for reordering or cross-list behavior, and add a Storybook story demonstrating usage (e.g., using SortableProvider with sensors prop and a consumer-managed onDragEnd that updates an array, and an example using <Sortable container={false}> for advanced patterns) so users can see the expected wiring.
36-49: Consider documenting theonMovevsonValueChangedistinction.The props expose both
onMove(called with the rawDragEndEvent) andonValueChange(called with the reordered array). Their relationship isn't immediately obvious —onMovefires on every drag end regardless of whether a reorder occurred (line 95), whileonValueChangeonly fires when items actually swap. A JSDoc comment ononMovewould help consumers understand the contract.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/sortable.tsx` around lines 36 - 49, Add a JSDoc comment to the onMove prop in the SortableProps interface clarifying its contract: state that onMove receives the raw DragEndEvent (type DragEndEvent) and is invoked on every drag end regardless of whether items changed, while onValueChange is only called when the items array actually reorders; mention that consumers can use onMove for low-level event handling and onValueChange for finalized reordered data. Reference the SortableProps interface, onMove, onValueChange, and DragEndEvent so readers can find the relevant types and behavior.src/components/ui/Sortable.stories.tsx (1)
4-8: Consider re-exporting sorting strategies from the sortable module.Stories (and consumers) currently import
verticalListSortingStrategy,horizontalListSortingStrategy, andrectSortingStrategydirectly from@dnd-kit/sortable. For a cleaner DX, re-export these from yoursortable.tsxmodule so consumers don't need to add@dnd-kit/sortableas a direct dependency or know the underlying library.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/Sortable.stories.tsx` around lines 4 - 8, Update your sortable.tsx module to re-export verticalListSortingStrategy, horizontalListSortingStrategy, and rectSortingStrategy from "@dnd-kit/sortable" (e.g., export { verticalListSortingStrategy, horizontalListSortingStrategy, rectSortingStrategy } from "@dnd-kit/sortable"), and then change imports in Sortable.stories.tsx to import these strategies from your sortable module instead of directly from "@dnd-kit/sortable" so consumers can import strategies via your component module.src/components/ui/popover.tsx (1)
15-17: Inconsistent export style forPopoverPortal.
PopoverPortaluses an inlineexport functiondeclaration while all other components in this file are declared withoutexportand then exported together in the named export block at the bottom (Lines 87–94). Consider movingPopoverPortalinto the bottom export block for consistency.♻️ Suggested fix
-export function PopoverPortal({ ...props }: PopoverPrimitive.Portal.Props) { +function PopoverPortal({ ...props }: PopoverPrimitive.Portal.Props) { return <PopoverPrimitive.Portal data-slot="popover-portal" {...props} />; }Then add it to the bottom export (which it already is in
index.ts, but keep the source consistent):export { Popover, PopoverContent, PopoverDescription, PopoverHeader, + PopoverPortal, PopoverTitle, PopoverTrigger, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/popover.tsx` around lines 15 - 17, PopoverPortal is declared with an inline export while the rest of the components are declared without export and then exported together; change PopoverPortal to a non-exported function declaration (function PopoverPortal(props: PopoverPrimitive.Portal.Props) { ... }) so it matches the other components, and ensure its identifier PopoverPortal is included in the named export block at the bottom alongside the other components; keep the existing JSX (<PopoverPrimitive.Portal data-slot="popover-portal" {...props} />) and type (PopoverPrimitive.Portal.Props) intact.src/index.ts (1)
146-151: Re-exportingDragOverlaydirectly from dnd-kit couples the public API to the library.
DragOverlayis a raw dnd-kit component re-exported as part of this package's public surface. If dnd-kit's API changes in a future version, it becomes a breaking change for your consumers. Consider wrapping it in a thin component (similar to howSortablewrapsDndContext) so you own the API contract.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.ts` around lines 146 - 151, The public export currently re-exports DragOverlay from dnd-kit (symbol DragOverlay in src/index.ts) which couples our API to the external library; create a thin wrapper component (e.g., OurDragOverlay or SortableDragOverlay) that imports DragOverlay from '@dnd-kit/core', forwards props/children and any refs, and export that wrapper from the module instead of re-exporting DragOverlay directly; update the export list (replace DragOverlay with the new wrapper name), add matching TypeScript types/exports for the wrapper, and adjust any internal imports/tests to use the new wrapper so consumers rely on our stable API surface.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Around line 60-64: Remove the unused and incompatible dependency entry
"@dnd-kit/react": "^0.3.0" from package.json (it's not imported anywhere and
conflicts with the legacy dnd-kit packages like "@dnd-kit/core" and
"@dnd-kit/sortable"); delete that line from the dependencies block, then update
your lockfile by reinstalling (run npm install or yarn install) so the lockfile
and node_modules are consistent.
In `@src/components/ui/Sortable.stories.tsx`:
- Around line 319-362: The MultipleLists story uses two independent Sortable
components (each with its own DndContext) so items cannot be dragged between
lists; either change the implementation to share a single
SortableProvider/DndContext around both lists to enable cross-list dragging
(wrap both lists with a common SortableProvider or move a single DndContext to
encompass both Sortable instances and coordinate on a shared onValueChange
handler for itemsA/itemsB and SortableItem ids), or if cross-list dragging is
not desired, rename MultipleLists to IndependentLists or SideBySideLists to make
behavior clear; update the story accordingly (referencing MultipleLists,
Sortable, SortableProvider/DndContext, SortableItem, itemsA/itemsB,
setItemsA/setItemsB).
In `@src/components/ui/sortable.tsx`:
- Around line 80-109: The DndContext handlers are being overridden by the
trailing {...props}; to fix, destructure onDragStart, onDragEnd, and
onDragCancel from the component props (keep onValueChange, onMove, etc.), then
compose them inside the DndContext: call your internal logic (setActiveId,
arrayMove + onValueChange, setActiveId(null), onMove) and after that call the
user-supplied handlers (the destructured onDragStart/onDragEnd/onDragCancel)
with the same event; finally spread the remaining props (the rest) into
DndContext so user props cannot silently override the internal handlers. Ensure
references to DndContext, onDragEnd, onDragStart, onDragCancel, setActiveId,
onMove, and onValueChange are updated accordingly.
---
Nitpick comments:
In `@src/components/ui/popover.tsx`:
- Around line 15-17: PopoverPortal is declared with an inline export while the
rest of the components are declared without export and then exported together;
change PopoverPortal to a non-exported function declaration (function
PopoverPortal(props: PopoverPrimitive.Portal.Props) { ... }) so it matches the
other components, and ensure its identifier PopoverPortal is included in the
named export block at the bottom alongside the other components; keep the
existing JSX (<PopoverPrimitive.Portal data-slot="popover-portal" {...props} />)
and type (PopoverPrimitive.Portal.Props) intact.
In `@src/components/ui/Sortable.stories.tsx`:
- Around line 4-8: Update your sortable.tsx module to re-export
verticalListSortingStrategy, horizontalListSortingStrategy, and
rectSortingStrategy from "@dnd-kit/sortable" (e.g., export {
verticalListSortingStrategy, horizontalListSortingStrategy, rectSortingStrategy
} from "@dnd-kit/sortable"), and then change imports in Sortable.stories.tsx to
import these strategies from your sortable module instead of directly from
"@dnd-kit/sortable" so consumers can import strategies via your component
module.
In `@src/components/ui/sortable.tsx`:
- Line 1: Remove the unnecessary "use strict" directive at the top of this
module: delete the standalone string literal "use strict" from
src/components/ui/sortable.tsx since TypeScript/ESM modules are already strict;
no other changes required.
- Around line 232-250: SortableProvider currently wraps DndContext but doesn't
document or demonstrate how consumers should handle drag lifecycle
(onDragEnd/onDragStart) or manage active item/reordering state; add a short doc
comment above the SortableProvider function explaining that it intentionally
forwards DndContext props and that consumers must supply handlers like
onDragEnd/onDragStart (and maintain item order/active id) for reordering or
cross-list behavior, and add a Storybook story demonstrating usage (e.g., using
SortableProvider with sensors prop and a consumer-managed onDragEnd that updates
an array, and an example using <Sortable container={false}> for advanced
patterns) so users can see the expected wiring.
- Around line 36-49: Add a JSDoc comment to the onMove prop in the SortableProps
interface clarifying its contract: state that onMove receives the raw
DragEndEvent (type DragEndEvent) and is invoked on every drag end regardless of
whether items changed, while onValueChange is only called when the items array
actually reorders; mention that consumers can use onMove for low-level event
handling and onValueChange for finalized reordered data. Reference the
SortableProps interface, onMove, onValueChange, and DragEndEvent so readers can
find the relevant types and behavior.
In `@src/index.ts`:
- Around line 146-151: The public export currently re-exports DragOverlay from
dnd-kit (symbol DragOverlay in src/index.ts) which couples our API to the
external library; create a thin wrapper component (e.g., OurDragOverlay or
SortableDragOverlay) that imports DragOverlay from '@dnd-kit/core', forwards
props/children and any refs, and export that wrapper from the module instead of
re-exporting DragOverlay directly; update the export list (replace DragOverlay
with the new wrapper name), add matching TypeScript types/exports for the
wrapper, and adjust any internal imports/tests to use the new wrapper so
consumers rely on our stable API surface.
package.json
Outdated
| "@dnd-kit/core": "^6.3.1", | ||
| "@dnd-kit/modifiers": "^9.0.0", | ||
| "@dnd-kit/react": "^0.3.0", | ||
| "@dnd-kit/sortable": "^10.0.0", | ||
| "@dnd-kit/utilities": "^3.2.2", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
dnd-kit npm packages @dnd-kit/core @dnd-kit/sortable @dnd-kit/react compatibility 2025 2026
💡 Result:
Compatibility (2025–2026)
1) “Legacy” dnd-kit (stable)
- Use
@dnd-kit/core+@dnd-kit/sortabletogether (Sortable is built on top of Core). [1][2] - Latest visible package versions (as of the sources crawled):
@dnd-kit/core6.3.1 and@dnd-kit/sortable10.0.0. [3][2]
2) “New” dnd-kit (rewrite)
@dnd-kit/reactis the new React API layer (published as 0.x, e.g. 0.1.21 in the npm listing snippet). [4]- The official migration guide says to migrate from
@dnd-kit/core/@dnd-kit/sortable/@dnd-kit/utilitiesto@dnd-kit/react(and@dnd-kit/helpers), i.e. it’s a replacement track, not an add-on. [5] - The legacy docs explicitly note there’s a new version and recommend using the latest instead of the legacy install path. [6]
Practical takeaway
- Don’t mix
@dnd-kit/react(new 0.x line) with the legacy@dnd-kit/core/@dnd-kit/sortablestack in the same implementation—pick one track:- Legacy:
@dnd-kit/core+@dnd-kit/sortable[1][2] - New:
@dnd-kit/react(+@dnd-kit/helpers, plus other new-scope packages as needed) [5][4]
- Legacy:
Sources: [1] @dnd-kit Sortable docs [2] Snyk package info for @dnd-kit/sortable [3] npm listing snippet for @dnd-kit/core [4] npm listing snippet for @dnd-kit/react [5] official migration guide (core → react) [6] legacy installation page noting a new version is available.
🏁 Script executed:
# Find and examine sortable.tsx to see which APIs are actually imported
fd -e tsx -e ts | xargs rg -l "sortable.tsx|useSortable|SortableContext" | head -5Repository: getdokan/plugin-ui
Length of output: 92
🏁 Script executed:
# Also check the actual package.json to see the full context
cat -n package.json | sed -n '50,75p'Repository: getdokan/plugin-ui
Length of output: 1084
🏁 Script executed:
cat -n src/components/ui/sortable.tsx | head -50Repository: getdokan/plugin-ui
Length of output: 1696
🏁 Script executed:
# Search for imports from dnd-kit packages
rg "from ['\"]@dnd-kit" src/components/ui/sortable.tsx -A 1 -B 1Repository: getdokan/plugin-ui
Length of output: 270
🏁 Script executed:
rg "@dnd-kit/react" --type ts --type tsxRepository: getdokan/plugin-ui
Length of output: 89
🏁 Script executed:
rg "@dnd-kit/react" --type typescriptRepository: getdokan/plugin-ui
Length of output: 96
🏁 Script executed:
# Also try searching without type restriction
rg "@dnd-kit/react" src/Repository: getdokan/plugin-ui
Length of output: 44
Remove unused @dnd-kit/react dependency — incompatible with the legacy dnd-kit stack in use.
The codebase uses the legacy dnd-kit track (@dnd-kit/core, @dnd-kit/sortable, @dnd-kit/utilities), which is explicitly incompatible with @dnd-kit/react. The latter is a replacement API from a complete dnd-kit rewrite (available as 0.x pre-release) and should not be mixed with the legacy packages. A search of the entire codebase confirms @dnd-kit/react is never imported or used anywhere—only the legacy packages are referenced in sortable.tsx. Remove @dnd-kit/react@^0.3.0 from dependencies.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 60 - 64, Remove the unused and incompatible
dependency entry "@dnd-kit/react": "^0.3.0" from package.json (it's not imported
anywhere and conflicts with the legacy dnd-kit packages like "@dnd-kit/core" and
"@dnd-kit/sortable"); delete that line from the dependencies block, then update
your lockfile by reinstalling (run npm install or yarn install) so the lockfile
and node_modules are consistent.
| export const MultipleLists: Story = { | ||
| render: () => { | ||
| function Demo() { | ||
| const [itemsA, setItemsA] = useState([ | ||
| { id: "A1", content: "A1" }, | ||
| { id: "A2", content: "A2" }, | ||
| ]); | ||
| const [itemsB, setItemsB] = useState([ | ||
| { id: "B1", content: "B1" }, | ||
| { id: "B2", content: "B2" }, | ||
| ]); | ||
|
|
||
| return ( | ||
| <div className="flex gap-8 p-4"> | ||
| <div className="w-48 space-y-4"> | ||
| <h3 className="font-bold">List A</h3> | ||
| <Sortable items={itemsA} onValueChange={setItemsA}> | ||
| <div className="min-h-[100px] rounded-lg border bg-muted/50 p-2 space-y-2"> | ||
| {itemsA.map(item => ( | ||
| <SortableItem key={item.id} id={item.id}> | ||
| <div className="rounded border bg-card p-2 shadow-sm text-center">{item.content}</div> | ||
| </SortableItem> | ||
| ))} | ||
| </div> | ||
| </Sortable> | ||
| </div> | ||
| <div className="w-48 space-y-4"> | ||
| <h3 className="font-bold">List B</h3> | ||
| <Sortable items={itemsB} onValueChange={setItemsB}> | ||
| <div className="min-h-[100px] rounded-lg border bg-muted/50 p-2 space-y-2"> | ||
| {itemsB.map(item => ( | ||
| <SortableItem key={item.id} id={item.id}> | ||
| <div className="rounded border bg-card p-2 shadow-sm text-center">{item.content}</div> | ||
| </SortableItem> | ||
| ))} | ||
| </div> | ||
| </Sortable> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
| return <Demo />; | ||
| }, | ||
| }; |
There was a problem hiding this comment.
MultipleLists story doesn't demonstrate cross-list dragging.
Each list has its own independent <Sortable> (and thus its own DndContext), so items can only be reordered within each list, not dragged between them. If cross-list DnD is intended, both lists would need to share a single SortableProvider. If the intent is just "two independent sortable lists on one page," consider renaming to something like IndependentLists or SideBySideLists to set correct expectations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/Sortable.stories.tsx` around lines 319 - 362, The
MultipleLists story uses two independent Sortable components (each with its own
DndContext) so items cannot be dragged between lists; either change the
implementation to share a single SortableProvider/DndContext around both lists
to enable cross-list dragging (wrap both lists with a common SortableProvider or
move a single DndContext to encompass both Sortable instances and coordinate on
a shared onValueChange handler for itemsA/itemsB and SortableItem ids), or if
cross-list dragging is not desired, rename MultipleLists to IndependentLists or
SideBySideLists to make behavior clear; update the story accordingly
(referencing MultipleLists, Sortable, SortableProvider/DndContext, SortableItem,
itemsA/itemsB, setItemsA/setItemsB).
| return ( | ||
| <DndContext | ||
| sensors={sensors} | ||
| collisionDetection={collisionDetection} | ||
| onDragStart={(event) => setActiveId(event.active.id)} | ||
| onDragEnd={(event) => { | ||
| const { active, over } = event; | ||
| if (over && active.id !== over.id) { | ||
| const oldIndex = items.findIndex((item) => item.id === active.id); | ||
| const newIndex = items.findIndex((item) => item.id === over.id); | ||
|
|
||
| if (oldIndex !== -1 && newIndex !== -1) { | ||
| onValueChange?.(arrayMove(items, oldIndex, newIndex)); | ||
| } | ||
| } | ||
| onMove?.(event); | ||
| setActiveId(null); | ||
| }} | ||
| onDragCancel={() => setActiveId(null)} | ||
| {...props} | ||
| > | ||
| {content} | ||
| {overlay && ( | ||
| <DragOverlay> | ||
| {activeId ? overlay : null} | ||
| </DragOverlay> | ||
| )} | ||
| </DndContext> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
{...props} spread after inline handlers will silently override internal drag logic.
SortableProps extends DndContextProps, so onDragStart, onDragEnd, and onDragCancel are not destructured out and remain in ...props. Since {...props} is spread after the explicit handlers (line 99), React uses the last value — meaning a consumer passing onDragEnd will completely bypass the internal arrayMove reordering logic.
Move {...props} before the inline handlers, or destructure onDragStart/onDragEnd/onDragCancel from props and compose them with the internal logic.
🐛 Proposed fix — destructure and compose handlers
const Sortable = <TData extends { id: UniqueIdentifier }>({
items,
onValueChange,
onMove,
strategy = verticalListSortingStrategy,
children,
overlay,
sensors: propSensors,
collisionDetection = closestCenter,
container = true,
+ onDragStart: onDragStartProp,
+ onDragEnd: onDragEndProp,
+ onDragCancel: onDragCancelProp,
...props
}: SortableProps<TData>) => {
const [activeId, setActiveId] = React.useState<UniqueIdentifier | null>(null);
...
return (
<DndContext
sensors={sensors}
collisionDetection={collisionDetection}
- onDragStart={(event) => setActiveId(event.active.id)}
- onDragEnd={(event) => {
+ onDragStart={(event) => {
+ setActiveId(event.active.id);
+ onDragStartProp?.(event);
+ }}
+ onDragEnd={(event) => {
const { active, over } = event;
if (over && active.id !== over.id) {
const oldIndex = items.findIndex((item) => item.id === active.id);
const newIndex = items.findIndex((item) => item.id === over.id);
if (oldIndex !== -1 && newIndex !== -1) {
onValueChange?.(arrayMove(items, oldIndex, newIndex));
}
}
onMove?.(event);
setActiveId(null);
+ onDragEndProp?.(event);
+ }}
+ onDragCancel={(event) => {
+ setActiveId(null);
+ onDragCancelProp?.(event);
}}
- onDragCancel={() => setActiveId(null)}
{...props}
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/sortable.tsx` around lines 80 - 109, The DndContext
handlers are being overridden by the trailing {...props}; to fix, destructure
onDragStart, onDragEnd, and onDragCancel from the component props (keep
onValueChange, onMove, etc.), then compose them inside the DndContext: call your
internal logic (setActiveId, arrayMove + onValueChange, setActiveId(null),
onMove) and after that call the user-supplied handlers (the destructured
onDragStart/onDragEnd/onDragCancel) with the same event; finally spread the
remaining props (the rest) into DndContext so user props cannot silently
override the internal handlers. Ensure references to DndContext, onDragEnd,
onDragStart, onDragCancel, setActiveId, onMove, and onValueChange are updated
accordingly.
# Conflicts: # package.json
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/index.ts (1)
146-151: ExportDragOverlayPropstype alongsideDragOverlayfor API consistency.
DragOverlay(line 151) is exported without a matching props type export, while the other Sortable-related components have type exports (lines 221–224):SortableProps,SortableItemProps,SortableDragHandleProps, andSortableProviderProps. Consumers needing to type-check or extendDragOverlayprops would have to import directly from@dnd-kit/react. AddDragOverlayPropsto maintain consistent API design.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.ts` around lines 146 - 151, Export the DragOverlayProps type alongside the existing DragOverlay export to maintain API consistency: add an export of DragOverlayProps (re-exporting the type from the same module you import DragOverlay from, e.g., `@dnd-kit/react`) so consumers can import { DragOverlay, DragOverlayProps } from this package; update the export list where Sortable, SortableItem, SortableDragHandle, SortableProvider and DragOverlay are exported to include DragOverlayProps.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/index.ts`:
- Around line 137-143: The barrel export in src/index.ts has dropped
PopoverClose and PopoverArrow which still exist in
src/components/ui/popover.tsx; restore the public API by re-adding PopoverClose
and PopoverArrow to the export list in src/index.ts (or, if the removal is
intentional, add a changelog/migration note explaining the breaking change and
update any consumer docs). Locate the export block containing Popover,
PopoverContent, PopoverDescription, PopoverHeader, PopoverPortal, PopoverTitle,
PopoverTrigger and include PopoverClose and PopoverArrow there so consumers
importing from the main barrel continue to work.
- Around line 146-151: There are mixed dnd-kit stacks being exported (Sortable,
SortableItem, SortableDragHandle, SortableProvider, DragOverlay) causing runtime
conflicts; pick one approach and update the exports and imports accordingly:
either remove all usages and exports of `@dnd-kit/react` (DragOverlay and any
react-stack imports) and keep the legacy stack (ensure
src/components/ui/sortable.tsx uses `@dnd-kit/core`@^6.3.x +
`@dnd-kit/sortable`@^10.0.0 and export only Sortable, SortableItem,
SortableDragHandle, SortableProvider), or migrate fully to the new stack by
removing `@dnd-kit/core` and `@dnd-kit/sortable`, update
src/components/ui/sortable.tsx to use `@dnd-kit/react`@0.3.0 (import sortable
utilities from `@dnd-kit/react/sortable`) and export the new equivalents
(including DragOverlay) consistently; make sure imports/exports across the
codebase match the chosen stack.
---
Nitpick comments:
In `@src/index.ts`:
- Around line 146-151: Export the DragOverlayProps type alongside the existing
DragOverlay export to maintain API consistency: add an export of
DragOverlayProps (re-exporting the type from the same module you import
DragOverlay from, e.g., `@dnd-kit/react`) so consumers can import { DragOverlay,
DragOverlayProps } from this package; update the export list where Sortable,
SortableItem, SortableDragHandle, SortableProvider and DragOverlay are exported
to include DragOverlayProps.
| // Sortable | ||
| Sortable, | ||
| SortableItem, | ||
| SortableDragHandle, | ||
| SortableProvider, | ||
| DragOverlay, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@dnd-kit/react 0.3.0 @dnd-kit/sortable 10.0.0 compatibility
💡 Result:
They’re not compatible.
@dnd-kit/sortable@10.0.0is part of the legacy stack and has a peer dependency on@dnd-kit/core^6.3.0(not@dnd-kit/react). [1]@dnd-kit/react@0.3.0is the new React package in the “migrate from@dnd-kit/coreto@dnd-kit/react” track, where you replace@dnd-kit/core+@dnd-kit/sortablewith@dnd-kit/react(+ related new packages). [2]- Sortable support in the new stack is exposed via
@dnd-kit/react/sortable(not@dnd-kit/sortable). [3]
So you should either:
- Stay legacy: use
@dnd-kit/core@^6.3.xwith@dnd-kit/sortable@10.0.0, or - Go new: use
@dnd-kit/react@0.3.0with@dnd-kit/react/sortable(matching the same new-version line). [1][2][3]
Sources: [1] [2] [3]
🏁 Script executed:
# Find and check package.json for actual dependencies and versions
fd -e package.json -type f | head -5Repository: getdokan/plugin-ui
Length of output: 232
🏁 Script executed:
# Find package.json correctly
find . -name "package.json" -type f | head -5Repository: getdokan/plugin-ui
Length of output: 76
🏁 Script executed:
# Check imports in src/index.ts
cat src/index.ts | head -20Repository: getdokan/plugin-ui
Length of output: 631
🏁 Script executed:
# Check package.json for `@dnd-kit` dependencies
cat package.json | grep -A 20 "@dnd-kit"Repository: getdokan/plugin-ui
Length of output: 914
🏁 Script executed:
# Look for dnd-kit imports in src/index.ts
rg "@dnd-kit|Sortable" src/index.ts -A 2 -B 2Repository: getdokan/plugin-ui
Length of output: 434
🏁 Script executed:
# Find where Sortable components are imported from
rg "from.*Sortable|from.*@dnd-kit" src/ -A 1 -B 1 | head -40Repository: getdokan/plugin-ui
Length of output: 659
🏁 Script executed:
# Check if there's a Sortable component file
find src -name "*ortable*" -o -name "*sortable*" | head -20Repository: getdokan/plugin-ui
Length of output: 131
Remove @dnd-kit/react or migrate to the new dnd-kit stack — do not mix them.
The codebase has incompatible @dnd-kit versions. @dnd-kit/sortable@10.0.0 is the legacy stack and requires @dnd-kit/core@^6.3.0, while @dnd-kit/react@0.3.0 is the new stack that replaces both. These are mutually exclusive—mixing them will cause runtime conflicts.
Choose one:
- Stay legacy: Remove
@dnd-kit/react, keep@dnd-kit/core@^6.3.1+@dnd-kit/sortable@^10.0.0(current setup insrc/components/ui/sortable.tsx). - Migrate to new: Remove
@dnd-kit/coreand@dnd-kit/sortable, use@dnd-kit/react@0.3.0and import sortable utilities from@dnd-kit/react/sortable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/index.ts` around lines 146 - 151, There are mixed dnd-kit stacks being
exported (Sortable, SortableItem, SortableDragHandle, SortableProvider,
DragOverlay) causing runtime conflicts; pick one approach and update the exports
and imports accordingly: either remove all usages and exports of `@dnd-kit/react`
(DragOverlay and any react-stack imports) and keep the legacy stack (ensure
src/components/ui/sortable.tsx uses `@dnd-kit/core`@^6.3.x +
`@dnd-kit/sortable`@^10.0.0 and export only Sortable, SortableItem,
SortableDragHandle, SortableProvider), or migrate fully to the new stack by
removing `@dnd-kit/core` and `@dnd-kit/sortable`, update
src/components/ui/sortable.tsx to use `@dnd-kit/react`@0.3.0 (import sortable
utilities from `@dnd-kit/react/sortable`) and export the new equivalents
(including DragOverlay) consistently; make sure imports/exports across the
codebase match the chosen stack.
…and `sortable` - Replaced `any` types with `DraggableSyntheticListeners` for better type safety. - Simplified `PopoverPortal` by removing unnecessary export keyword. - Removed unused imports (e.g., `GripVertical`, `defaultDropAnimationSideEffects`, `DropdownProvider`, and strategies). - Adjusted `Sortable.stories.tsx` to remove unused props and imports for cleaner implementation.
- Added `@dnd-kit` packages (`core`, `modifiers`, `react`, `sortable`, `utilities`) and related dependencies. - Introduced new peer dependencies and updates for better compatibility and functionality.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/components/ui/Sortable.stories.tsx (1)
38-38: HoistDemocomponents out of therendercallbacks to avoid unintended state resets.Every story defines
function Demo()insiderender: () => { ... }. Each invocation ofrenderproduces a new function reference, so React treats it as a different component type and performs a full unmount/remount — discarding all interactive state (sorted order,editingId, switch states). This is triggered by HMR, any args/controls change in the Storybook panel, or any parent re-render.The fix is to hoist each
Democomponent to module scope (or above the story object):♻️ Example fix (applies to all 8 stories)
+function VerticalDemo() { + const [items, setItems] = useState(initialItems); + return ( + <div className="w-80"> + <Sortable items={items} onValueChange={setItems}> + <div className="flex flex-col gap-2"> + {items.map((item) => ( + <SortableItem key={item.id} id={item.id}> + <div className="flex items-center justify-between rounded-md border bg-card p-4 shadow-sm"> + <span>{item.content}</span> + </div> + </SortableItem> + ))} + </div> + </Sortable> + </div> + ); +} export const Vertical: Story = { - render: () => { - function Demo() { - const [items, setItems] = useState(initialItems); - return ( - <div className="w-80"> - <Sortable items={items} onValueChange={setItems}> - <div className="flex flex-col gap-2"> - {items.map((item) => ( - <SortableItem key={item.id} id={item.id}> - <div className="flex items-center justify-between rounded-md border bg-card p-4 shadow-sm"> - <span>{item.content}</span> - </div> - </SortableItem> - ))} - </div> - </Sortable> - </div> - ); - } - return <Demo />; - }, + render: () => <VerticalDemo />, };Also applies to: 135-135, 159-159, 189-189, 217-217, 251-251, 281-281, 317-317
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/Sortable.stories.tsx` at line 38, Each story currently defines a local function Demo inside its render: () => { ... } callback which creates a new component type on every render (e.g., Demo in the render of each story), causing React to unmount/remount and lose interactive state; hoist each Demo component to module scope (move the Demo function out of the render callback and place it above the story definitions) and update the story render to return the hoisted Demo component (keep any props passed via story args), ensuring the same component reference is used across renders and preserving state for stories that use Demo, editingId, sorted order, and switches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ui/Sortable.stories.tsx`:
- Around line 85-90: The icon-only <button> that calls setEditingId(null) and
renders the X component lacks an accessible name; update that button to include
an appropriate aria-label (e.g., aria-label="Cancel editing" or "Close") so
screen readers get meaningful context for the action triggered by the X icon,
and ensure the X icon itself remains decorative (aria-hidden) if you add the
label.
- Around line 173-175: In the WithDragHandle story in Sortable.stories.tsx the
remove/action Button (the X icon) is missing an onClick handler; either wire
that Button up to call the story's remove function (e.g., call an existing
handleRemove or onRemove item callback used by the Sortable list) or add a local
handler that updates the story's items state to remove the corresponding item id
when clicked; if removal isn't supported in this story, replace the interactive
Button with a non-interactive element (e.g., a span) to avoid implying
interactivity. Ensure you modify the Button inside the WithDragHandle render so
it receives onClick={() => removeItem(id)} (or similar) and that removeItem is
defined and wired to the same state/props managing the list.
- Around line 78-106: The edit UI is currently uncontrolled and never persists
changes: the Input uses defaultValue and the save/cancel handlers (setEditingId
in the X and Check button handlers) only close edit mode without updating the
items state. Change to a controlled edit flow by adding a temporary edit value
(e.g., editingValue) when entering edit mode for the specific item (set
editingValue = item.label in the handler that sets editingId), bind that
editingValue to the Input (onChange updates editingValue), and on the Check
button click update the items state (use setItems to map the item with matching
item.id and replace its label with editingValue) then setEditingId(null); keep
the X button to discard changes by just clearing editingId (and optionally
clearing editingValue).
---
Duplicate comments:
In `@src/components/ui/Sortable.stories.tsx`:
- Around line 316-359: The story MultipleLists currently renders two separate
Sortable components (inside Demo), creating two independent DndContexts so
cross-list dragging is impossible; either (A) make one shared
Sortable/DndContext by moving the <Sortable> wrapper to encompass both list
containers and update Sortable/SortableItem usage and onValueChange to handle
item moves between itemsA and itemsB (use item ids and a single state handler in
Demo), or (B) if independent lists are intended, rename the story and UI text
from MultipleLists to IndependentLists (and update the story description) so it
doesn't promise cross-list drag behavior. Target symbols: MultipleLists, Demo,
Sortable, SortableItem, itemsA, itemsB, onValueChange.
---
Nitpick comments:
In `@src/components/ui/Sortable.stories.tsx`:
- Line 38: Each story currently defines a local function Demo inside its render:
() => { ... } callback which creates a new component type on every render (e.g.,
Demo in the render of each story), causing React to unmount/remount and lose
interactive state; hoist each Demo component to module scope (move the Demo
function out of the render callback and place it above the story definitions)
and update the story render to return the hoisted Demo component (keep any props
passed via story args), ensuring the same component reference is used across
renders and preserving state for stories that use Demo, editingId, sorted order,
and switches.
| {editingId === item.id ? ( | ||
| <div className="relative flex-1"> | ||
| <Input | ||
| defaultValue={item.label} | ||
| className="h-9 border-primary/50 ring-primary/20 focus-visible:border-primary focus-visible:ring-primary/30" | ||
| autoFocus | ||
| /> | ||
| <button | ||
| onClick={() => setEditingId(null)} | ||
| className="absolute right-2 top-1/2 -translate-y-1/2 text-muted-foreground hover:text-foreground" | ||
| > | ||
| <X className="size-4" /> | ||
| </button> | ||
| </div> | ||
| ) : ( | ||
| <span className="text-sm font-medium">{item.label}</span> | ||
| )} | ||
| </div> | ||
|
|
||
| <div className="flex items-center gap-3"> | ||
| {editingId === item.id ? ( | ||
| <Button | ||
| variant="outline" | ||
| size="icon-sm" | ||
| className="h-8 w-8 border-muted" | ||
| onClick={() => setEditingId(null)} | ||
| > | ||
| <Check className="size-4 text-muted-foreground" /> | ||
| </Button> |
There was a problem hiding this comment.
Edits in AdvancedCustomUI are silently discarded — demo implies saving but never updates state.
The Input uses defaultValue (uncontrolled), so its value is never read back. The "save" action (Check button, line 99 and the X button, line 86) only calls setEditingId(null) without committing the new label to items state. Users interacting with the demo will type a label change, click save, and see it revert — which misrepresents the editing capability.
Either make it a read-only view-mode toggle (remove the Input entirely), or wire up a controlled flow:
🐛 Proposed fix to persist edits
+ const [editValues, setEditValues] = useState<Record<string, string>>(
+ Object.fromEntries(items.map((i) => [i.id, i.label]))
+ );
+
+ const saveEdit = (id: string) => {
+ setItems((prev) =>
+ prev.map((item) =>
+ item.id === id ? { ...item, label: editValues[id] } : item
+ )
+ );
+ setEditingId(null);
+ };
{editingId === item.id ? (
<div className="relative flex-1">
<Input
- defaultValue={item.label}
+ value={editValues[item.id]}
+ onChange={(e) =>
+ setEditValues((prev) => ({ ...prev, [item.id]: e.target.value }))
+ }
className="..."
autoFocus
/>
<button
- onClick={() => setEditingId(null)}
+ onClick={() => saveEdit(item.id)}
...
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/Sortable.stories.tsx` around lines 78 - 106, The edit UI is
currently uncontrolled and never persists changes: the Input uses defaultValue
and the save/cancel handlers (setEditingId in the X and Check button handlers)
only close edit mode without updating the items state. Change to a controlled
edit flow by adding a temporary edit value (e.g., editingValue) when entering
edit mode for the specific item (set editingValue = item.label in the handler
that sets editingId), bind that editingValue to the Input (onChange updates
editingValue), and on the Check button click update the items state (use
setItems to map the item with matching item.id and replace its label with
editingValue) then setEditingId(null); keep the X button to discard changes by
just clearing editingId (and optionally clearing editingValue).
| <button | ||
| onClick={() => setEditingId(null)} | ||
| className="absolute right-2 top-1/2 -translate-y-1/2 text-muted-foreground hover:text-foreground" | ||
| > | ||
| <X className="size-4" /> | ||
| </button> |
There was a problem hiding this comment.
Icon-only <button> is missing an aria-label.
The native <button> wrapping the X icon (cancel editing) has no accessible name. Screen readers will announce it without meaningful context.
♿ Proposed fix
- <button
- onClick={() => setEditingId(null)}
- className="absolute right-2 top-1/2 -translate-y-1/2 text-muted-foreground hover:text-foreground"
- >
+ <button
+ onClick={() => setEditingId(null)}
+ aria-label="Cancel editing"
+ className="absolute right-2 top-1/2 -translate-y-1/2 text-muted-foreground hover:text-foreground"
+ >📝 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.
| <button | |
| onClick={() => setEditingId(null)} | |
| className="absolute right-2 top-1/2 -translate-y-1/2 text-muted-foreground hover:text-foreground" | |
| > | |
| <X className="size-4" /> | |
| </button> | |
| <button | |
| onClick={() => setEditingId(null)} | |
| aria-label="Cancel editing" | |
| className="absolute right-2 top-1/2 -translate-y-1/2 text-muted-foreground hover:text-foreground" | |
| > | |
| <X className="size-4" /> | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/Sortable.stories.tsx` around lines 85 - 90, The icon-only
<button> that calls setEditingId(null) and renders the X component lacks an
accessible name; update that button to include an appropriate aria-label (e.g.,
aria-label="Cancel editing" or "Close") so screen readers get meaningful context
for the action triggered by the X icon, and ensure the X icon itself remains
decorative (aria-hidden) if you add the label.
| <Button variant="ghost" size="icon-sm"> | ||
| <X className="size-4" /> | ||
| </Button> |
There was a problem hiding this comment.
Remove/action Button in WithDragHandle has no onClick handler.
The X icon button appears interactive but does nothing when clicked. In a demo, this creates confusion about whether removal is supported. Either wire up a remove handler or replace it with a non-interactive element.
🐛 Proposed fix to add a remove handler
- <Button variant="ghost" size="icon-sm">
+ <Button
+ variant="ghost"
+ size="icon-sm"
+ aria-label="Remove item"
+ onClick={() =>
+ setItems((prev) => prev.filter((i) => i.id !== item.id))
+ }
+ >
<X className="size-4" />
</Button>📝 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.
| <Button variant="ghost" size="icon-sm"> | |
| <X className="size-4" /> | |
| </Button> | |
| <Button | |
| variant="ghost" | |
| size="icon-sm" | |
| aria-label="Remove item" | |
| onClick={() => | |
| setItems((prev) => prev.filter((i) => i.id !== item.id)) | |
| } | |
| > | |
| <X className="size-4" /> | |
| </Button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/Sortable.stories.tsx` around lines 173 - 175, In the
WithDragHandle story in Sortable.stories.tsx the remove/action Button (the X
icon) is missing an onClick handler; either wire that Button up to call the
story's remove function (e.g., call an existing handleRemove or onRemove item
callback used by the Sortable list) or add a local handler that updates the
story's items state to remove the corresponding item id when clicked; if removal
isn't supported in this story, replace the interactive Button with a
non-interactive element (e.g., a span) to avoid implying interactivity. Ensure
you modify the Button inside the WithDragHandle render so it receives
onClick={() => removeItem(id)} (or similar) and that removeItem is defined and
wired to the same state/props managing the list.
1 participant
Summary by CodeRabbit