diff --git a/packages/react/src/experimental/OneDataCollection/hooks/useDataCollectionData/useDataCollectionLanesData.tsx b/packages/react/src/experimental/OneDataCollection/hooks/useDataCollectionData/useDataCollectionLanesData.tsx index a677edc3ce..52ec7d3182 100644 --- a/packages/react/src/experimental/OneDataCollection/hooks/useDataCollectionData/useDataCollectionLanesData.tsx +++ b/packages/react/src/experimental/OneDataCollection/hooks/useDataCollectionData/useDataCollectionLanesData.tsx @@ -6,7 +6,7 @@ import { SortingsDefinition, UseDataOptions, } from "@/hooks/datasource" -import { useCallback, useEffect, useMemo, useState } from "react" +import { useCallback, useEffect, useMemo, useRef, useState } from "react" import { ItemActionsDefinition } from "../../item-actions" import { NavigationFiltersDefinition } from "../../navigationFilters/types" import { SummariesDefinition } from "../../summary" @@ -79,10 +79,28 @@ const LaneProvider = < onError, }) + const signature = useMemo(() => { + return JSON.stringify({ + dataRef: hook.data, + isInitialLoading: hook.isInitialLoading, + isLoading: hook.isLoading, + isLoadingMore: hook.isLoadingMore, + pagination: hook.paginationInfo, + error: hook.error?.message ?? null, + }) + }, [ + hook.data, + hook.isInitialLoading, + hook.isLoading, + hook.isLoadingMore, + hook.paginationInfo, + hook.error?.message, + ]) + useEffect(() => { onHookUpdate?.(hook) // eslint-disable-next-line react-hooks/exhaustive-deps - }, [hook]) + }, [signature]) return null } @@ -120,70 +138,81 @@ export function useDataCollectionLanesData< const handleHookUpdate = useCallback( (laneId: string | symbol, value: UseDataCollectionData) => { - setLanesHooks((prev) => ({ ...prev, [laneId]: value })) + setLanesHooks((prev) => { + const curr = prev[laneId] + if ( + curr && + curr.data === value.data && + curr.paginationInfo === value.paginationInfo && + curr.isInitialLoading === value.isInitialLoading && + curr.isLoading === value.isLoading && + curr.isLoadingMore === value.isLoadingMore && + curr.error?.message === value.error?.message + ) { + return prev + } + if (curr === value) return prev + return { ...prev, [laneId]: value } + }) }, [] ) const lanesSignature = useMemo(() => JSON.stringify(lanes), [lanes]) - const currentFiltersSignature = useMemo( - () => JSON.stringify(source.currentFilters), - [source.currentFilters] - ) - const currentNavigationFiltersSignature = useMemo( - () => JSON.stringify(source.currentNavigationFilters), - [source.currentNavigationFilters] - ) - const currentSortingsSignature = useMemo( - () => JSON.stringify(source.currentSortings), - [source.currentSortings] - ) - const currentGroupingSignature = useMemo( - () => JSON.stringify(source.currentGrouping), - [source.currentGrouping] - ) - const currentSearchSignature = useMemo( - () => JSON.stringify(source.currentSearch), - [source.currentSearch] - ) - const groupingSignature = useMemo( - () => JSON.stringify(source.grouping), - [source.grouping] - ) - - const lanesProvider = useMemo( - () => { - return (lanes || []).map((lane) => ( - - key={lane.id} - lane={lane} - onError={options.onError} - source={source} - onHookUpdate={(value) => handleHookUpdate(lane.id, value)} - > - )) - }, - // eslint-disable-next-line react-hooks/exhaustive-deps + const sourceDepsSignature = useMemo( + () => + JSON.stringify({ + f: source.currentFilters, + nf: source.currentNavigationFilters, + s: source.currentSortings, + g: source.currentGrouping, + q: source.currentSearch, + grp: source.grouping, + }), [ - // TODO check why source ref is updated always - lanesSignature, - handleHookUpdate, - currentFiltersSignature, - currentNavigationFiltersSignature, - currentSortingsSignature, - currentGroupingSignature, - currentSearchSignature, - groupingSignature, + source.currentFilters, + source.currentNavigationFilters, + source.currentSortings, + source.currentGrouping, + source.currentSearch, + source.grouping, ] ) + const stableSourceRef = useRef(source) + useEffect(() => { + stableSourceRef.current = source + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [sourceDepsSignature]) + + const stableLanesRef = useRef(lanes) + useEffect(() => { + stableLanesRef.current = lanes + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [lanesSignature]) + + const lanesProvider = useMemo(() => { + const currentLanes = stableLanesRef.current || [] + const currentSource = stableSourceRef.current + return currentLanes.map((lane) => ( + + key={lane.id} + lane={lane} + onError={options.onError} + source={currentSource} + onHookUpdate={(value) => handleHookUpdate(lane.id, value)} + > + )) + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [lanesSignature, sourceDepsSignature, handleHookUpdate, options.onError]) + return { lanesProvider, lanesHooks, diff --git a/packages/react/src/experimental/OneDataCollection/hooks/useDataCollectionSource/__tests__/useDataCollectionSource.component.spec.tsx b/packages/react/src/experimental/OneDataCollection/hooks/useDataCollectionSource/__tests__/useDataCollectionSource.component.spec.tsx new file mode 100644 index 0000000000..407461a00d --- /dev/null +++ b/packages/react/src/experimental/OneDataCollection/hooks/useDataCollectionSource/__tests__/useDataCollectionSource.component.spec.tsx @@ -0,0 +1,92 @@ +import { + FiltersDefinition, + GroupingDefinition, + SortingsDefinition, +} from "@/hooks/datasource" +import { defaultTranslations, I18nProvider } from "@/lib/providers/i18n" +import { render, screen } from "@testing-library/react" +import userEvent from "@testing-library/user-event" +import { memo, useState } from "react" +import { describe, expect, it, vi } from "vitest" +import { useDataCollectionSource } from ".." +import type { ItemActionsDefinition } from "../../../item-actions" +import type { NavigationFiltersDefinition } from "../../../navigationFilters/types" +import type { SummariesDefinition } from "../../../summary" +import type { DataCollectionSource } from "../types" + +const Wrapper = ({ children }: { children: React.ReactNode }) => ( + {children} +) + +type Item = { id: number; name: string } + +type SourceT = DataCollectionSource< + Item, + FiltersDefinition, + SortingsDefinition, + SummariesDefinition, + ItemActionsDefinition, + NavigationFiltersDefinition, + GroupingDefinition +> + +const Child = memo( + ({ + source, + onRender, + }: { + source: SourceT + onRender: (s: SourceT) => void + }) => { + onRender(source) + return
+ } +) +Child.displayName = "Child" + +function Parent({ onRender }: { onRender: (s: SourceT) => void }) { + const [tick, setTick] = useState(0) + const source = useDataCollectionSource< + Item, + FiltersDefinition, + SortingsDefinition, + SummariesDefinition, + ItemActionsDefinition, + NavigationFiltersDefinition, + GroupingDefinition + >({ + dataAdapter: { fetchData: async () => ({ records: [] }) }, + filters: { name: { type: "search", label: "Name" } }, + lanes: [{ id: "1", filters: { name: "a" } }], + }) + + return ( +
+ + +
+ ) +} + +describe("useDataCollectionSource (component integration)", () => { + it("does not re-render child when hook inputs remain unchanged", async () => { + const onRender = vi.fn() + const user = userEvent.setup() + + render( + + + + ) + + // Initial render of Child + expect(onRender).toHaveBeenCalledTimes(1) + + // Trigger a parent re-render without changing hook inputs + const button = screen.getByRole("button", { name: /rerender/i }) + await user.click(button) + + // Desired behavior: source reference should be stable, child should not re-render + expect(onRender).toHaveBeenCalledTimes(1) + }) +}) diff --git a/packages/react/src/experimental/OneDataCollection/hooks/useDataCollectionSource/__tests__/useDataCollectionSource.spec.tsx b/packages/react/src/experimental/OneDataCollection/hooks/useDataCollectionSource/__tests__/useDataCollectionSource.spec.tsx new file mode 100644 index 0000000000..5ff94a6dd4 --- /dev/null +++ b/packages/react/src/experimental/OneDataCollection/hooks/useDataCollectionSource/__tests__/useDataCollectionSource.spec.tsx @@ -0,0 +1,137 @@ +import { defaultTranslations, I18nProvider } from "@/lib/providers/i18n" +import { renderHook } from "@testing-library/react" +import { describe, expect, it, vi } from "vitest" +import { useDataCollectionSource } from ".." + +const Wrapper = ({ children }: { children: React.ReactNode }) => ( + {children} +) + +describe("useDataCollectionSource", () => { + it("returns stable references when source and deps are stable", () => { + const dataAdapter = { fetchData: vi.fn(async () => ({ records: [] })) } + + const { result, rerender } = renderHook( + ({ dep }: { dep: number }) => + useDataCollectionSource( + { + dataAdapter, + filters: { name: { type: "search" as const, label: "Name" } }, + navigationFilters: undefined, + }, + [dep] + ), + { initialProps: { dep: 1 }, wrapper: Wrapper } + ) + + const first = result.current + + // Rerender without changing props or deps + rerender({ dep: 1 }) + + const second = result.current + + // Core references should be stable (note: the returned object itself is new on rerender) + expect(second.filters).toBe(first.filters) + expect(second.dataAdapter).toBe(first.dataAdapter) + // setCurrentFilters is a wrapper function, identity is not guaranteed to be stable + expect(second.setCurrentSortings).toBe(first.setCurrentSortings) + expect(second.setCurrentSearch).toBe(first.setCurrentSearch) + expect(second.setIsLoading).toBe(first.setIsLoading) + expect(second.setCurrentNavigationFilters).toBe( + first.setCurrentNavigationFilters + ) + + // Invoke setters with identical values to ensure no type errors and stable behavior + second.setCurrentFilters(second.currentFilters) + second.setCurrentSearch(second.currentSearch) + second.setIsLoading(second.isLoading) + + // Rerender again with same deps + rerender({ dep: 1 }) + + const third = result.current + + // Still stable references for memoized and state handlers + expect(third.filters).toBe(first.filters) + expect(third.dataAdapter).toBe(first.dataAdapter) + }) + + it("updates memoized fields only when deps change", () => { + const dataAdapter = { fetchData: vi.fn(async () => ({ records: [] })) } + + const summaries = { total: { type: "sum" as const } } + + const { result, rerender } = renderHook( + ({ dep, sums }: { dep: number; sums: typeof summaries }) => + useDataCollectionSource( + { + dataAdapter, + summaries: sums, + }, + [dep] + ), + { initialProps: { dep: 1, sums: summaries }, wrapper: Wrapper } + ) + + const first = result.current + + // Change non-dependency prop reference (new object with same content) + const newSummaries = { total: { type: "sum" as const } } + rerender({ dep: 1, sums: newSummaries }) + + const second = result.current + + // summaries is memoized with deps; with same dep it should remain referentially equal + expect(second.summaries).toBe(first.summaries) + + // Now change dependency so memoized values can update + rerender({ dep: 2, sums: newSummaries }) + + const third = result.current + expect(third.summaries).not.toBe(first.summaries) + }) + + it("exposes the provided filters definition on source.filters", () => { + const dataAdapter = { fetchData: vi.fn(async () => ({ records: [] })) } + const filters = { name: { type: "search" as const, label: "Name" } } + + const { result } = renderHook( + () => + useDataCollectionSource({ + dataAdapter, + filters, + }), + { wrapper: Wrapper } + ) + + expect(result.current.filters).toBe(filters) + }) + + it("preserves lanes configuration in the returned source", () => { + const dataAdapter = { fetchData: vi.fn(async () => ({ records: [] })) } + const filters = { + status: { + type: "in" as const, + label: "Status", + options: { options: [{ value: "open", label: "Open" }] }, + }, + } + const lanes = [ + { id: "open", filters: { status: ["open"] } }, + { id: "all", filters: {} }, + ] + + const { result } = renderHook( + () => + useDataCollectionSource({ + dataAdapter, + filters, + lanes, + }), + { wrapper: Wrapper } + ) + + expect(result.current.lanes).toBe(lanes) + }) +}) diff --git a/packages/react/src/experimental/OneDataCollection/hooks/useDataCollectionSource/useDataCollectionSource.ts b/packages/react/src/experimental/OneDataCollection/hooks/useDataCollectionSource/useDataCollectionSource.ts index d940bb9a53..7a90dfe3ce 100644 --- a/packages/react/src/experimental/OneDataCollection/hooks/useDataCollectionSource/useDataCollectionSource.ts +++ b/packages/react/src/experimental/OneDataCollection/hooks/useDataCollectionSource/useDataCollectionSource.ts @@ -85,19 +85,31 @@ export const useDataCollectionSource = < // eslint-disable-next-line react-hooks/exhaustive-deps const memoizedSummaries = useMemo(() => summaries, deps) - return { - ...datasource, - summaries: memoizedSummaries, - navigationFilters, - currentNavigationFilters, - setCurrentNavigationFilters, - } as DataCollectionSource< - R, - FiltersSchema, - Sortings, - Summaries, - ItemActions, - NavigationFilters, - Grouping - > + const stableSource = useMemo( + () => + ({ + ...datasource, + summaries: memoizedSummaries, + navigationFilters, + currentNavigationFilters, + setCurrentNavigationFilters, + }) as DataCollectionSource< + R, + FiltersSchema, + Sortings, + Summaries, + ItemActions, + NavigationFilters, + Grouping + >, + [ + datasource, + memoizedSummaries, + navigationFilters, + currentNavigationFilters, + setCurrentNavigationFilters, + ] + ) + + return stableSource } diff --git a/packages/react/src/experimental/OneDataCollection/visualizations/collection/Kanban/Kanban.tsx b/packages/react/src/experimental/OneDataCollection/visualizations/collection/Kanban/Kanban.tsx index 87f585f217..ef99ea070c 100644 --- a/packages/react/src/experimental/OneDataCollection/visualizations/collection/Kanban/Kanban.tsx +++ b/packages/react/src/experimental/OneDataCollection/visualizations/collection/Kanban/Kanban.tsx @@ -103,6 +103,73 @@ export const KanbanCollection = < return Boolean(paginationInfo && paginationInfo.type === "infinite-scroll") } + const getStableId = (item: R, index?: number) => { + const explicitId = (item as unknown as { id?: string | number })?.id + if (explicitId !== undefined && explicitId !== null) + return String(explicitId) + if (idProvider) return String(idProvider(item, index ?? 0)) + return String(index ?? 0) + } + + const optimisticMoveIntoData = async ( + fromLaneId: string, + toLaneId: string, + sourceRecord: R, + destiny: { record: R; position: "above" | "below" } | null + ) => { + if (fromLaneId === toLaneId) { + const laneHook = lanesHooks[fromLaneId] + if (!laneHook) return + const srcKey = getStableId(sourceRecord) + laneHook.updateRecords((prev) => { + const items = [...prev] + const fromIdx = items.findIndex( + (item, index) => getStableId(item, index) === srcKey + ) + if (fromIdx === -1) return prev + const [moved] = items.splice(fromIdx, 1) + // Recompute target relative to post-removal array + let targetIdx = 0 + if (destiny && destiny.record) { + const destKey = getStableId(destiny.record) + const idx = items.findIndex( + (item, index) => getStableId(item, index) === destKey + ) + targetIdx = Math.max(0, idx) + } + const insertAt = + destiny?.position === "below" ? targetIdx + 1 : targetIdx + const bounded = Math.max(0, Math.min(insertAt, items.length)) + items.splice(bounded, 0, moved) + return items + }) + return + } + + const fromHook = lanesHooks[fromLaneId] + const toHook = lanesHooks[toLaneId] + if (!fromHook || !toHook) return + const srcKey = getStableId(sourceRecord) + fromHook.updateRecords((prev) => + prev.filter((item, index) => getStableId(item, index) !== srcKey) + ) + toHook.updateRecords((prev) => { + const items = [...prev] + let insertIndex = 0 + if (destiny && destiny.record) { + const destKey = getStableId(destiny.record) + const targetIdx = items.findIndex( + (item, index) => getStableId(item, index) === destKey + ) + if (targetIdx !== -1) + insertIndex = targetIdx + (destiny.position === "below" ? 1 : 0) + } + const bounded = Math.max(0, Math.min(insertIndex, items.length)) + items.splice(bounded, 0, sourceRecord) + return items + }) + } + const kanbanProps: KanbanProps = { lanes: laneItems.map((l) => { const laneData = lanesHooks[l.id] @@ -126,20 +193,12 @@ export const KanbanCollection = < loading: Object.values(lanesHooks).some( (laneHook) => laneHook.isInitialLoading ), - getKey: (item, index) => { - if (idProvider) return String(idProvider(item, index)) - const fallbackId = (item as unknown as { id?: string | number })?.id - return fallbackId !== undefined && fallbackId !== null - ? String(fallbackId) - : String(index) - }, + getKey: (item, index) => getStableId(item, index), renderCard: (item, index, total, laneId) => { - const dragId = String( - idProvider - ? idProvider(item, index) - : ((item as unknown as { id?: string | number })?.id ?? index) - ) - const itemId = source.selectable ? source.selectable(item) : item.id + const dragId = getStableId(item, index) + const itemId = source.selectable + ? source.selectable(item) + : (item as unknown as { id?: string | number })?.id // Gets the lane useSelectable hook const useSelectable = @@ -160,8 +219,8 @@ export const KanbanCollection = < return ( key={dragId} - drag={{ id: dragId, type: "list-card", data: { ...item, laneId } }} - id={String(item.id)} + drag={{ id: dragId, type: "list-card", data: { ...item } }} + id={dragId} index={index} total={total} laneId={laneId} @@ -245,7 +304,36 @@ export const KanbanCollection = < const idx = laneIndexMaps.get(laneId)?.get(id) ?? -1 return allowReorder ? idx : -1 }, - onMove: onMove, + onMove: async (fromLaneId, toLaneId, sourceRecord, destiny) => { + // Optimistic into useData + await optimisticMoveIntoData(fromLaneId, toLaneId, sourceRecord, destiny) + // Delegate to external onMove, then replace if backend returned updated record + if (onMove) { + const result = await onMove(fromLaneId, toLaneId, sourceRecord, destiny) + if (result) { + const destHook = lanesHooks[toLaneId] + const srcHook = lanesHooks[fromLaneId] + const srcKey = getStableId(sourceRecord) + // Replace in destination (same-lane: this is the only operation needed) + destHook?.updateRecords((prev) => { + const items = [...prev] + const idx = items.findIndex( + (item, index) => getStableId(item, index) === srcKey + ) + if (idx !== -1) items.splice(idx, 1, result) + return items + }) + // Cross-lane: ensure cleanup in origin in case of race + if (fromLaneId !== toLaneId) { + srcHook?.updateRecords((prev) => + prev.filter((item, index) => getStableId(item, index) !== srcKey) + ) + } + } + return result + } + return sourceRecord + }, } /** diff --git a/packages/react/src/hooks/datasource/useData.ts b/packages/react/src/hooks/datasource/useData.ts index 9c05533cdd..1d94be1201 100644 --- a/packages/react/src/hooks/datasource/useData.ts +++ b/packages/react/src/hooks/datasource/useData.ts @@ -99,6 +99,8 @@ export interface UseDataReturn { // Merged filters (default values and current values) mergedFilters: FiltersState + // Optimistic update API + updateRecords: (updater: (prev: R[]) => R[]) => void } type DataType = PromiseState @@ -746,6 +748,9 @@ export function useData< loadMore, mergedFilters, totalItems, + updateRecords: (updater: (prev: R[]) => R[]) => { + setRawData((prev) => updater(prev)) + }, } } diff --git a/packages/react/src/hooks/datasource/useDataSource.ts b/packages/react/src/hooks/datasource/useDataSource.ts index 464afa60bc..f3abdb91e9 100644 --- a/packages/react/src/hooks/datasource/useDataSource.ts +++ b/packages/react/src/hooks/datasource/useDataSource.ts @@ -1,5 +1,5 @@ import type { Dispatch, SetStateAction } from "react" -import { useEffect, useMemo, useState } from "react" +import { useCallback, useEffect, useMemo, useState } from "react" import { useDebounceValue } from "usehooks-ts" import { DataSource, @@ -107,23 +107,25 @@ export function useDataSource< const setCurrentFilters: Dispatch< SetStateAction> - > = (value) => { - if (typeof value === "function") { - _setCurrentFilters((prev) => { - const next = ( - value as ( - prevState: FiltersState - ) => FiltersState - )(prev) - return JSON.stringify(next) === JSON.stringify(prev) ? prev : next - }) - } else { - if (JSON.stringify(currentFilters) === JSON.stringify(value)) { - return + > = useCallback( + (value) => { + if (typeof value === "function") { + _setCurrentFilters((prev) => { + const next = ( + value as ( + prevState: FiltersState + ) => FiltersState + )(prev) + return JSON.stringify(next) === JSON.stringify(prev) ? prev : next + }) + } else { + _setCurrentFilters((prev) => + JSON.stringify(prev) === JSON.stringify(value) ? prev : value + ) } - _setCurrentFilters(value) - } - } + }, + [_setCurrentFilters] + ) const [currentSortings, setCurrentSortings] = useState | null>(defaultSorting || null) @@ -171,33 +173,90 @@ export function useDataSource< throw new Error("Grouping is mandatory but no grouping state is set") } - return { - ...rest, - // Filters - filters: memoizedFilters, - currentFilters, - setCurrentFilters, + const { + sortings, + presets, + presetsLoading, + selectable, + defaultSelectedItems, + } = rest - // Sortings - currentSortings, - setCurrentSortings, + const stableDataSource = useMemo( + () => ({ + // Unknown passthrough (e.g., extensions from higher-level sources) + ...rest, + // Definition passthrough + sortings, + presets, + presetsLoading, + selectable, + defaultSelectedItems, + defaultSorting, - // Search - search, - currentSearch, - setCurrentSearch, - debouncedCurrentSearch, + // Filters + filters: memoizedFilters, + currentFilters, + setCurrentFilters, - // Loading - isLoading, - setIsLoading, + // Sortings + currentSortings, + setCurrentSortings, - // Data adapter - dataAdapter: memoizedDataAdapter, + // Search + search, + currentSearch, + setCurrentSearch, + debouncedCurrentSearch, - // Grouping - setCurrentGrouping, - currentGrouping, - grouping, - } + // Loading + isLoading, + setIsLoading, + + // Data adapter + dataAdapter: memoizedDataAdapter, + + // Grouping + setCurrentGrouping, + currentGrouping, + grouping, + }), + [ + // Definition + sortings, + presets, + presetsLoading, + selectable, + defaultSelectedItems, + defaultSorting, + + // Filters + memoizedFilters, + currentFilters, + setCurrentFilters, + + // Sortings + currentSortings, + setCurrentSortings, + + // Search + search, + currentSearch, + setCurrentSearch, + debouncedCurrentSearch, + + // Loading + isLoading, + setIsLoading, + + // Data adapter + memoizedDataAdapter, + + // Grouping + setCurrentGrouping, + currentGrouping, + grouping, + ] + ) + + return stableDataSource } diff --git a/packages/react/src/ui/Kanban/Kanban.tsx b/packages/react/src/ui/Kanban/Kanban.tsx index b19f887464..be9d9bf695 100644 --- a/packages/react/src/ui/Kanban/Kanban.tsx +++ b/packages/react/src/ui/Kanban/Kanban.tsx @@ -26,13 +26,7 @@ export function Kanban( maxHeight = KANBAN_LANE_HEIGHT, } = props - // Local source-of-truth for lanes to orchestrate moves centrally - const [localLanes, setLocalLanes] = useState( - () => lanes as KanbanProps["lanes"] - ) - useEffect(() => { - setLocalLanes(lanes) - }, [lanes]) + // No local lanes state. Parent owns the data; this component is pure/derivative. // Horizontal edge zones for board autoscroll (debug visibility + logs) const [isDragging, setIsDragging] = useState(false) @@ -117,7 +111,7 @@ export function Kanban( }, [isDragging]) const getIndexById = (laneId: string, id: string): number => { - const lane = localLanes.find((l) => l.id === laneId) + const lane = lanes.find((l) => l.id === laneId) if (!lane) return -1 return lane.items.findIndex((item, index) => { const key = String(getKey(item as TRecord, index, laneId)) @@ -127,132 +121,43 @@ export function Kanban( const onMove = async (params: KanbanOnMoveParam) => { const { fromLaneId, toLaneId, sourceId, indexOfTarget, position } = params - - // Snapshot - const prev = localLanes - - // Find source record and indices in snapshot (robust to mis-reported fromLaneId) - let fromLaneIdx = prev.findIndex((l) => l.id === fromLaneId) - const toLaneIdx = prev.findIndex((l) => l.id === toLaneId) - if (toLaneIdx === -1) return Promise.reject(new Error("Lane not found")) - let sourceIndex = -1 - if (fromLaneIdx !== -1) { - sourceIndex = prev[fromLaneIdx].items.findIndex((item, index) => { - const key = String(getKey(item as TRecord, index, fromLaneId)) - return key === String(sourceId) - }) - } - if (sourceIndex === -1) { - for (let i = 0; i < prev.length; i++) { - const laneId = prev[i].id as string - const idx = prev[i].items.findIndex((item, index) => { - const key = String(getKey(item as TRecord, index, laneId)) - return key === String(sourceId) - }) - if (idx !== -1) { - fromLaneIdx = i - sourceIndex = idx - break - } - } - } - if (fromLaneIdx === -1 || sourceIndex === -1) { + // Resolve source record from current props + const allLanes = lanes + const fromLane = allLanes.find((l) => l.id === fromLaneId) + const toLane = allLanes.find((l) => l.id === toLaneId) + if (!fromLane || !toLane) return Promise.reject(new Error("Lane not found")) + const sourceIndex = fromLane.items.findIndex( + (item, idx) => + String(getKey(item as TRecord, idx, fromLaneId)) === String(sourceId) + ) + if (sourceIndex === -1) return Promise.resolve(undefined as unknown as TRecord) - } - const sourceRecord = prev[fromLaneIdx].items[sourceIndex] as TRecord - - // Compute insertion index - let insertIndex = 0 - if (indexOfTarget == null) { - insertIndex = 0 - } else { - insertIndex = indexOfTarget + (position === "below" ? 1 : 0) - } - - // Build next lanes state (also adjust totals if provided by lanes) - const isSameLane = fromLaneId === toLaneId - const next = prev.map((lane, idx) => { - if (idx === fromLaneIdx && isSameLane) { - // Same lane reorder - const items = [...lane.items] - items.splice(sourceIndex, 1) - // Adjust index after removal - const adjustedIndex = - sourceIndex < insertIndex ? insertIndex - 1 : insertIndex - items.splice(adjustedIndex, 0, sourceRecord) - return { ...lane, items } - } - if (idx === fromLaneIdx) { - const items = [...lane.items] - items.splice(sourceIndex, 1) - const nextTotal = - typeof lane.total === "number" && !isSameLane - ? Math.max(0, lane.total - 1) - : lane.total - return { ...lane, items, total: nextTotal } - } - if (idx === toLaneIdx) { - const items = [...lane.items] - const boundedIndex = Math.max(0, Math.min(insertIndex, items.length)) - items.splice(boundedIndex, 0, sourceRecord) - const nextTotal = - typeof lane.total === "number" && !isSameLane - ? lane.total + 1 - : lane.total - return { ...lane, items, total: nextTotal } - } - return lane - }) - - // Optimistic apply - setLocalLanes(next) - - try { - // Call external move if provided - const destinyRecord = - indexOfTarget == null - ? null - : (prev[toLaneIdx].items[indexOfTarget] as TRecord | undefined) - const result = await dnd?.onMove?.( - fromLaneId, - toLaneId, - sourceRecord, - destinyRecord - ? { - record: destinyRecord, - position: (position as "above" | "below") ?? "above", - } - : null - ) - - if (result) { - // Replace record by id with backend version - setLocalLanes((curr) => - curr.map((lane) => { - if (lane.id !== toLaneId) return lane - const items = [...lane.items] - const idx = items.findIndex((item, index) => { - const key = String(getKey(item as TRecord, index, toLaneId)) - return key === String(sourceId) - }) - if (idx !== -1) items.splice(idx, 1, result) - return { ...lane, items } - }) - ) - } - return result as TRecord - } catch (e) { - // Rollback - setLocalLanes(prev) - throw e - } + const sourceRecord = fromLane.items[sourceIndex] as TRecord + + const destinyRecord = + indexOfTarget == null + ? null + : (toLane.items[indexOfTarget] as TRecord | undefined) + + const result = await dnd?.onMove?.( + fromLaneId, + toLaneId, + sourceRecord, + destinyRecord + ? { + record: destinyRecord, + position: (position as "above" | "below") ?? "above", + } + : null + ) + return result as TRecord } return (
- {localLanes.map( + {lanes.map( (lane: KanbanLaneAttributes, laneIndex: number) => { const total = lane.total ?? lane.items.length return ( diff --git a/packages/react/src/ui/Kanban/__stories__/Kanban.stories.tsx b/packages/react/src/ui/Kanban/__stories__/Kanban.stories.tsx index 5883cbf2a3..f7b556558c 100644 --- a/packages/react/src/ui/Kanban/__stories__/Kanban.stories.tsx +++ b/packages/react/src/ui/Kanban/__stories__/Kanban.stories.tsx @@ -460,3 +460,137 @@ export const SimpleOnMoveTest: Story = { }) }, } + +export const SameLaneReorderDebug: Story = { + args: { + lanes: [], + renderCard: () => null, + getKey: () => "", + }, + render: function Render() { + const [instanceId] = useState(() => Symbol("kanban-instance")) + + type T = { id: string; title: string } + const laneId = "A" + const [lanes, setLanes] = useState["lanes"]>([ + { + id: laneId, + title: "Lane A", + items: [ + { id: "1", title: "One" }, + { id: "2", title: "Two" }, + { id: "3", title: "Three" }, + ], + variant: "neutral", + }, + ]) + + const getIndexById = (lid: string, id: string): number => { + const lane = lanes.find((l) => l.id === lid) + return lane?.items.findIndex((it) => it.id === id) ?? -1 + } + + const onMove: KanbanProps["dnd"]["onMove"] = async ( + _from, + _to, + source, + _dest + ) => { + // Simulate backend OK + await new Promise((r) => setTimeout(r, 10)) + return { ...source } + } + + const dispatchMove = (detail: { + fromLaneId: string + toLaneId: string + sourceId: string + indexOfTarget: number | null + position: "above" | "below" | null + }) => { + window.dispatchEvent(new CustomEvent("kanban-test-move", { detail })) + } + + return ( +
+
+ + + +
+ + + + lanes={lanes} + getKey={(i) => i.id} + dnd={{ instanceId, getIndexById, onMove }} + renderCard={(item: T, index: number, total: number) => ( + + drag={{ id: item.id, type: "list-card", data: { ...item } }} + id={item.id} + index={index} + total={total} + title={`${item.id} - ${item.title}`} + /> + )} + /> + +
+ ) + }, +} diff --git a/packages/react/src/ui/Kanban/components/KanbanLane.tsx b/packages/react/src/ui/Kanban/components/KanbanLane.tsx index 1e073aa465..84227b6743 100644 --- a/packages/react/src/ui/Kanban/components/KanbanLane.tsx +++ b/packages/react/src/ui/Kanban/components/KanbanLane.tsx @@ -27,17 +27,11 @@ export function KanbanLane({ allowReorder?: boolean } & LaneProps) { const laneRef = useRef(null) - // const coordinator = useMoveCoordinator() const [isOver, setIsOver] = useState(false) const hasFullDnD = Boolean(id && getLaneResourceIndexById) // Autoscroll state - const overlayRef = useRef(null) const viewportRef = useRef(null) - const rafRef = useRef(null) - const speedPxPerSecRef = useRef(0) - const lastTimeRef = useRef(null) - const [isDragging, setIsDragging] = useState(false) useDroppableList( hasFullDnD @@ -49,7 +43,13 @@ export function KanbanLane({ : undefined ) - // Time-based scroll loop + // Minimal DnD wiring: only compute params and forward to parent; no local state mutation + const rafRef = useRef(null) + const speedPxPerSecRef = useRef(0) + const lastTimeRef = useRef(null) + const [isDragging, setIsDragging] = useState(false) + + // Time-based vertical scroll during drag useEffect(() => { const step = () => { const now = performance.now() @@ -254,12 +254,18 @@ export function KanbanLane({ return } - // Single upward call. Parent/Story updates state; coordinator not needed here. + // Forward to parent await onMove?.(onMoveParams) }, }) }, [id, getLaneResourceIndexById, onMove, isDragging, laneProps.items]) + // Track drag lifecycle via DnD driver + useDndEvents(({ phase }) => { + if (phase === "start") setIsDragging(true) + if (phase === "drop" || phase === "cancel") setIsDragging(false) + }) + // Resolve viewport and observe DOM changes to re-resolve useEffect(() => { const resolve = () => { @@ -281,11 +287,7 @@ export function KanbanLane({ return () => observer.disconnect() }, [id]) - // Track drag lifecycle via DnD driver - useDndEvents(({ phase }) => { - if (phase === "start") setIsDragging(true) - if (phase === "drop" || phase === "cancel") setIsDragging(false) - }) + // Drag lifecycle handled by parent Kanban // Test hook: allow stories to trigger onMove without real DnD useEffect(() => { @@ -333,10 +335,9 @@ export function KanbanLane({ > {/* Debug overlay (non-interactive) */}