Skip to content

Commit e668911

Browse files
igarashitmigarashitm
committed
fix(DataMapper): Add DnD drop validation with visual feedback and error toasts
Fixes: #2898 Introduces mapping validation when user performs drag and drop to create a data mapping. Introduces `MappingValidationService` to centralise mapping pair validation rules, replacing `VisualizationService testNodePair()`. `DnDHandler.handleDragEnd()` now returns a `DnDResult` to propagate error message if there is. - Invalid cross-side drops (unselected xs:choice, container/terminal mismatch) show an error toast - Hovering over an invalid target shows a disabled border and field name style - Hovering over a same-side droppable (source→source, target→target) is disabled at DNDKit level, i.e. no visual feedback on hover, silently ignored if dropped
1 parent 31d010d commit e668911

12 files changed

+789
-19
lines changed

‎packages/ui/src/components/Document/NodeContainer.scss‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,16 @@
1010
box-sizing: border-box;
1111
}
1212

13+
.droppable-invalid {
14+
color: var(--pf-t--global--text--color--disabled);
15+
border-style: dashed;
16+
border-color:var(--pf-t--global--text--color--disabled);
17+
border-width: 1px;
18+
cursor: not-allowed;
19+
border-radius: var(--pf-v6-c-droppable--m-dragging--after--BorderRadius);
20+
box-sizing: border-box;
21+
}
22+
1323
.draggable-container {
1424
font-weight: bold;
1525
color: var(--pf-t--global--icon--color--brand--default);

‎packages/ui/src/components/Document/NodeContainer.test.tsx‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
import { useDroppable } from '@dnd-kit/core';
2+
import { render } from '@testing-library/react';
3+
4+
import { NodeData } from '../../models/datamapper/visualization';
5+
import { DataMapperDndContext } from '../../providers/datamapper-dnd.provider';
6+
import { MappingValidationService } from '../../services/mapping-validation.service';
7+
import { DroppableContainer } from './NodeContainer';
8+
9+
jest.mock('@dnd-kit/core', () => ({
10+
useDroppable: jest.fn().mockReturnValue({ isOver: false, setNodeRef: jest.fn() }),
11+
useDraggable: jest.fn().mockReturnValue({
12+
isDragging: false,
13+
attributes: {},
14+
listeners: {},
15+
setNodeRef: jest.fn(),
16+
transform: null,
17+
}),
18+
}));
19+
20+
jest.mock('../../services/mapping-validation.service', () => ({
21+
MappingValidationService: {
22+
validateMappingPair: jest.fn().mockReturnValue({ isValid: true }),
23+
},
24+
}));
25+
26+
describe('DroppableContainer', () => {
27+
const mockNodeData = {
28+
id: 'test-node',
29+
title: 'Test Node',
30+
isSource: false,
31+
isPrimitive: false,
32+
path: {},
33+
} as unknown as NodeData;
34+
35+
const activeSourceNode = {
36+
id: 'active-source',
37+
title: 'Active Source',
38+
isSource: true,
39+
isPrimitive: false,
40+
path: {},
41+
} as unknown as NodeData;
42+
43+
beforeEach(() => {
44+
(useDroppable as jest.Mock).mockReturnValue({ isOver: false, setNodeRef: jest.fn() });
45+
(MappingValidationService.validateMappingPair as jest.Mock).mockReturnValue({ isValid: true });
46+
});
47+
48+
it('should call useDroppable with disabled: false when there is no active node', () => {
49+
render(
50+
<DataMapperDndContext.Provider value={{}}>
51+
<DroppableContainer id="test" nodeData={mockNodeData}>
52+
content
53+
</DroppableContainer>
54+
</DataMapperDndContext.Provider>,
55+
);
56+
expect(useDroppable).toHaveBeenCalledWith(expect.objectContaining({ disabled: false }));
57+
});
58+
59+
it('should call useDroppable with disabled: true when the active node is on the same side', () => {
60+
const sameSideNode = { ...mockNodeData, id: 'same-side', isSource: false } as unknown as NodeData;
61+
render(
62+
<DataMapperDndContext.Provider value={{ activeNode: sameSideNode }}>
63+
<DroppableContainer id="test" nodeData={mockNodeData}>
64+
content
65+
</DroppableContainer>
66+
</DataMapperDndContext.Provider>,
67+
);
68+
expect(useDroppable).toHaveBeenCalledWith(expect.objectContaining({ disabled: true }));
69+
});
70+
71+
it('should apply droppable-invalid class when hovering over an invalid cross-side drop', () => {
72+
(MappingValidationService.validateMappingPair as jest.Mock).mockReturnValue({ isValid: false });
73+
(useDroppable as jest.Mock).mockReturnValue({ isOver: true, setNodeRef: jest.fn() });
74+
75+
const { container } = render(
76+
<DataMapperDndContext.Provider value={{ activeNode: activeSourceNode }}>
77+
<DroppableContainer id="test" nodeData={mockNodeData}>
78+
content
79+
</DroppableContainer>
80+
</DataMapperDndContext.Provider>,
81+
);
82+
83+
expect(container.querySelector('.droppable-invalid')).toBeTruthy();
84+
expect(container.querySelector('.droppable-container')).toBeFalsy();
85+
});
86+
87+
it('should apply droppable-container class when hovering over a valid cross-side drop', () => {
88+
(MappingValidationService.validateMappingPair as jest.Mock).mockReturnValue({ isValid: true });
89+
(useDroppable as jest.Mock).mockReturnValue({ isOver: true, setNodeRef: jest.fn() });
90+
91+
const { container } = render(
92+
<DataMapperDndContext.Provider value={{ activeNode: activeSourceNode }}>
93+
<DroppableContainer id="test" nodeData={mockNodeData}>
94+
content
95+
</DroppableContainer>
96+
</DataMapperDndContext.Provider>,
97+
);
98+
99+
expect(container.querySelector('.droppable-container')).toBeTruthy();
100+
expect(container.querySelector('.droppable-invalid')).toBeFalsy();
101+
});
102+
});

‎packages/ui/src/components/Document/NodeContainer.tsx‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@ import './NodeContainer.scss';
33
import { useDraggable, useDroppable } from '@dnd-kit/core';
44
import { isDefined } from '@kaoto/forms';
55
import clsx from 'clsx';
6-
import { forwardRef, FunctionComponent, PropsWithChildren } from 'react';
6+
import { forwardRef, FunctionComponent, PropsWithChildren, useContext, useMemo } from 'react';
77

88
import { DocumentNodeData, NodeData } from '../../models/datamapper/visualization';
9+
import { DataMapperDndContext } from '../../providers/datamapper-dnd.provider';
10+
import { MappingValidationService } from '../../services/mapping-validation.service';
911
import { VisualizationService } from '../../services/visualization.service';
1012

1113
type DnDContainerProps = PropsWithChildren & {
@@ -19,16 +21,35 @@ type BaseContainerProps = PropsWithChildren & {
1921
};
2022

2123
export const DroppableContainer: FunctionComponent<BaseContainerProps> = ({ className, id, nodeData, children }) => {
24+
const { activeNode } = useContext(DataMapperDndContext);
25+
26+
const isInvalidDrop = useMemo(() => {
27+
if (!activeNode) return false;
28+
if (activeNode.isSource === nodeData.isSource) return false;
29+
return !MappingValidationService.validateMappingPair(activeNode, nodeData).isValid;
30+
}, [activeNode, nodeData]);
31+
32+
// unlike `isInvalidDrop` which shows disabled hover effect and show an error toast if it's dropped,
33+
// if it's same side (source<->source / target<->target), disable DnDKit to show no hover effect
34+
// nor error, but just silently ignore it
35+
const isSameSide = !!activeNode && activeNode.isSource === nodeData.isSource;
36+
2237
const { isOver, setNodeRef: setDroppableNodeRef } = useDroppable({
2338
id: `droppable-${id}`,
2439
data: nodeData,
40+
disabled: isSameSide,
2541
});
2642

2743
return (
2844
<div
2945
id={`droppable-${id}`}
3046
ref={setDroppableNodeRef}
31-
className={clsx(className, { 'droppable-container': isOver }, 'pf-v6-c-droppable')}
47+
className={clsx(
48+
className,
49+
{ 'droppable-container': isOver && !isInvalidDrop },
50+
{ 'droppable-invalid': isOver && isInvalidDrop },
51+
'pf-v6-c-droppable',
52+
)}
3253
data-dnd-droppable={isOver ? `${id}` : undefined}
3354
>
3455
{children}

‎packages/ui/src/providers/datamapper-dnd.provider.test.tsx‎

Lines changed: 91 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,11 @@
1-
import { canScrollPanel, scrollAwareCollision } from './datamapper-dnd.provider';
1+
import { DndContext, DragEndEvent } from '@dnd-kit/core';
2+
import { AlertVariant } from '@patternfly/react-core';
3+
import { act, render } from '@testing-library/react';
4+
import { ReactNode } from 'react';
5+
6+
import { useDataMapper } from '../hooks/useDataMapper';
7+
import { MappingTree } from '../models/datamapper/mapping';
8+
import { canScrollPanel, DatamapperDndProvider, scrollAwareCollision } from './datamapper-dnd.provider';
29

310
// Mock rectIntersection to control collision detection in tests
411
jest.mock('@dnd-kit/core', () => {
@@ -13,9 +20,17 @@ jest.mock('@dnd-kit/core', () => {
1320
}));
1421
}),
1522
pointerWithin: jest.fn(() => []),
23+
DndContext: jest.fn(),
24+
DragOverlay: jest.fn(() => null),
25+
useSensor: jest.fn(),
26+
useSensors: jest.fn().mockReturnValue([]),
1627
};
1728
});
1829

30+
jest.mock('../hooks/useDataMapper', () => ({
31+
useDataMapper: jest.fn(),
32+
}));
33+
1934
// Helper to create mock rect object
2035
const createMockRect = (top: number, bottom: number, left: number, right: number) => ({
2136
top,
@@ -336,3 +351,78 @@ describe('datamapper-dnd.provider', () => {
336351
});
337352
});
338353
});
354+
355+
describe('DatamapperDndProvider', () => {
356+
const mockSendAlert = jest.fn();
357+
const mockRefreshMappingTree = jest.fn();
358+
const mockMappingTree = {} as MappingTree;
359+
360+
const mockDragEndEvent = {
361+
active: { id: 'a', data: { current: null } },
362+
over: null,
363+
} as unknown as DragEndEvent;
364+
365+
beforeEach(() => {
366+
jest.clearAllMocks();
367+
(DndContext as unknown as jest.Mock).mockImplementation(({ children }: { children: ReactNode }) => children);
368+
(useDataMapper as jest.Mock).mockReturnValue({
369+
mappingTree: mockMappingTree,
370+
refreshMappingTree: mockRefreshMappingTree,
371+
sendAlert: mockSendAlert,
372+
});
373+
});
374+
375+
const invokeOnDragEnd = async (event: DragEndEvent) => {
376+
const onDragEnd = (DndContext as unknown as jest.Mock).mock.calls.at(-1)[0].onDragEnd as (e: DragEndEvent) => void;
377+
await act(async () => {
378+
onDragEnd(event);
379+
});
380+
};
381+
382+
it('should call sendAlert with danger variant when handler returns failure with error message', async () => {
383+
const mockHandler = {
384+
handleDragEnd: jest.fn().mockReturnValue({ success: false, errorMessage: 'error msg' }),
385+
handleDragStart: jest.fn(),
386+
handleDragOver: jest.fn(),
387+
};
388+
389+
render(
390+
<DatamapperDndProvider handler={mockHandler}>
391+
<div />
392+
</DatamapperDndProvider>,
393+
);
394+
395+
await invokeOnDragEnd(mockDragEndEvent);
396+
397+
expect(mockSendAlert).toHaveBeenCalledWith({ variant: AlertVariant.danger, title: 'error msg' });
398+
});
399+
400+
it('should not call sendAlert when handler returns success', async () => {
401+
const mockHandler = {
402+
handleDragEnd: jest.fn().mockReturnValue({ success: true }),
403+
handleDragStart: jest.fn(),
404+
handleDragOver: jest.fn(),
405+
};
406+
407+
render(
408+
<DatamapperDndProvider handler={mockHandler}>
409+
<div />
410+
</DatamapperDndProvider>,
411+
);
412+
413+
await invokeOnDragEnd(mockDragEndEvent);
414+
415+
expect(mockSendAlert).not.toHaveBeenCalled();
416+
});
417+
418+
it('should not crash when no handler is provided', async () => {
419+
render(
420+
<DatamapperDndProvider handler={undefined}>
421+
<div />
422+
</DatamapperDndProvider>,
423+
);
424+
425+
await expect(invokeOnDragEnd(mockDragEndEvent)).resolves.not.toThrow();
426+
expect(mockSendAlert).not.toHaveBeenCalled();
427+
});
428+
});

‎packages/ui/src/providers/datamapper-dnd.provider.tsx‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
useSensor,
1717
useSensors,
1818
} from '@dnd-kit/core';
19-
import { Label } from '@patternfly/react-core';
19+
import { AlertVariant, Label } from '@patternfly/react-core';
2020
import { createContext, FunctionComponent, PropsWithChildren, useCallback, useMemo, useRef, useState } from 'react';
2121

2222
import { useDataMapper } from '../hooks/useDataMapper';
@@ -116,6 +116,7 @@ export const canScrollPanel = (element: Element, activeDragSideRef: { current: D
116116

117117
export interface IDataMapperDndContext {
118118
handler?: DnDHandler;
119+
activeNode?: NodeData;
119120
}
120121

121122
export const DataMapperDndContext = createContext<IDataMapperDndContext>({});
@@ -125,7 +126,7 @@ type DataMapperDndContextProps = PropsWithChildren & {
125126
};
126127

127128
export const DatamapperDndProvider: FunctionComponent<DataMapperDndContextProps> = (props) => {
128-
const { mappingTree, refreshMappingTree } = useDataMapper();
129+
const { mappingTree, refreshMappingTree, sendAlert } = useDataMapper();
129130
const [activeData, setActiveData] = useState<DataRef<NodeData> | null>(null);
130131
const activeDragSideRef = useRef<'source' | 'target' | null>(null);
131132

@@ -166,18 +167,24 @@ export const DatamapperDndProvider: FunctionComponent<DataMapperDndContextProps>
166167

167168
const handleDragEnd = useCallback(
168169
(event: DragEndEvent) => {
169-
props.handler && props.handler.handleDragEnd(event, mappingTree, refreshMappingTree);
170+
if (props.handler) {
171+
const result = props.handler.handleDragEnd(event, mappingTree, refreshMappingTree);
172+
if (!result.success && result.errorMessage) {
173+
sendAlert({ variant: AlertVariant.danger, title: result.errorMessage });
174+
}
175+
}
170176
setActiveData(null);
171177
activeDragSideRef.current = null;
172178
},
173-
[mappingTree, props.handler, refreshMappingTree],
179+
[mappingTree, props.handler, refreshMappingTree, sendAlert],
174180
);
175181

176182
const handler = useMemo(() => {
177183
return {
178184
handler: props.handler,
185+
activeNode: activeData?.current,
179186
};
180-
}, [props.handler]);
187+
}, [props.handler, activeData]);
181188

182189
const draggingLabel = activeData?.current?.title ? activeData.current.title : 'dragging...';
183190
return (
@@ -197,7 +204,11 @@ export const DatamapperDndProvider: FunctionComponent<DataMapperDndContextProps>
197204
>
198205
{props.children}
199206
<DragOverlay dropAnimation={null}>
200-
<div className={'pf-v6-c-draggable node__row dragging-container'} data-dnd-dragging={draggingLabel}>
207+
<div
208+
className={'pf-v6-c-draggable node__row dragging-container'}
209+
data-dnd-dragging={draggingLabel}
210+
style={{ pointerEvents: 'none' }}
211+
>
201212
<Label>{draggingLabel}</Label>
202213
</div>
203214
</DragOverlay>

‎packages/ui/src/providers/dnd/DnDHandler.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,13 @@ import { DragEndEvent, DragOverEvent, DragStartEvent } from '@dnd-kit/core';
22

33
import { MappingTree } from '../../models/datamapper/mapping';
44

5+
export interface DnDResult {
6+
success: boolean;
7+
errorMessage?: string;
8+
}
9+
510
export interface DnDHandler {
611
handleDragStart(event: DragStartEvent): void;
712
handleDragOver(event: DragOverEvent): void;
8-
handleDragEnd(event: DragEndEvent, mappingTree: MappingTree, onUpdate: () => void): void;
13+
handleDragEnd(event: DragEndEvent, mappingTree: MappingTree, onUpdate: () => void): DnDResult;
914
}

0 commit comments

Comments
 (0)