Skip to content

Commit

Permalink
feat: #417 drag outside behavior (#420)
Browse files Browse the repository at this point in the history
* fix: improve drag behavior outside of tree boundaries or legal drop positions (#417)

* chore: use volta in core package for react workflow

* chore: fix packagejson field

* chore: change verify workflow to not use yarn on nested packages
  • Loading branch information
lukasbach authored Nov 18, 2024
1 parent 59ffd2b commit feedb5c
Show file tree
Hide file tree
Showing 11 changed files with 85 additions and 21 deletions.
7 changes: 2 additions & 5 deletions .github/workflows/verify.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,8 @@ jobs:
run: yarn
- name: Custom React version
run: |
echo "enableImmutableInstalls: false" > ./.yarnrc.yml
yarn add react@${{ matrix.react }} -D
cd packages/core
yarn add react@${{ matrix.react }} -D
cd ../..
echo -e "nodeLinker: node-modules\nenableImmutableInstalls: false" > ./.yarnrc.yml
npm pkg set devDependencies.react=${{matrix.react}} --ws
cat ./.yarnrc.yml
yarn
- name: Build
Expand Down
10 changes: 2 additions & 8 deletions next-release-notes.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
<!--
### Breaking Changes
### Features
### Bug Fixes and Improvements
### Other Changes
-->
- Don't show drag line when dragging outside of the tree container (#417)
- Fix a bug where items where dropped on the last valid position when dragging items on an invalid position and then dropping (#417)
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"repository": {
"type": "git",
"url": "[email protected]:lukasbach/react-complex-tree.git",
"directory": "packages/docs"
"directory": "."
},
"author": "Lukas Bach",
"license": "MIT",
Expand Down
4 changes: 4 additions & 0 deletions packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,5 +66,9 @@
"setupFiles": [
"./test/helpers/setup.ts"
]
},
"volta": {
"node": "18.12.1",
"yarn": "3.3.0"
}
}
8 changes: 4 additions & 4 deletions packages/core/src/controlledEnvironment/layoutUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export const computeItemHeight = (treeId: string) => {
};

export const isOutsideOfContainer = (e: DragEvent, treeBb: DOMRect) =>
e.clientX < treeBb.left ||
e.clientX > treeBb.right ||
e.clientY < treeBb.top ||
e.clientY > treeBb.bottom;
e.clientX <= treeBb.left ||
e.clientX >= treeBb.right ||
e.clientY <= treeBb.top ||
e.clientY >= treeBb.bottom;
17 changes: 17 additions & 0 deletions packages/core/src/drag/DragAndDropProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { useCallSoon } from '../useCallSoon';
import { useStableHandler } from '../useStableHandler';
import { useGetOriginalItemOrder } from '../useGetOriginalItemOrder';
import { useDraggingPosition } from './useDraggingPosition';
import { isOutsideOfContainer } from '../controlledEnvironment/layoutUtils';

const DragAndDropContext = React.createContext<DragAndDropContextProps>(
null as any
Expand Down Expand Up @@ -177,6 +178,20 @@ export const DragAndDropProvider: React.FC<React.PropsWithChildren> = ({
}
);

const onDragLeaveContainerHandler = useStableHandler(
(
e: DragEvent,
containerRef: React.MutableRefObject<HTMLElement | undefined>
) => {
if (!containerRef.current) return;
if (
isOutsideOfContainer(e, containerRef.current.getBoundingClientRect())
) {
setDraggingPosition(undefined);
}
}
);

const onDropHandler = useStableHandler(() => {
if (!draggingItems || !draggingPosition || !environment.onDrop) {
return;
Expand Down Expand Up @@ -287,6 +302,7 @@ export const DragAndDropProvider: React.FC<React.PropsWithChildren> = ({
itemHeight: itemHeight.current,
isProgrammaticallyDragging,
onDragOverTreeHandler,
onDragLeaveContainerHandler,
viableDragPositions,
}),
[
Expand All @@ -297,6 +313,7 @@ export const DragAndDropProvider: React.FC<React.PropsWithChildren> = ({
isProgrammaticallyDragging,
itemHeight,
onDragOverTreeHandler,
onDragLeaveContainerHandler,
onStartDraggingItems,
programmaticDragDown,
programmaticDragUp,
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/drag/DraggingPositionEvaluation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ export class DraggingPositionEvaluation {
return true;
}

getDraggingPosition(): DraggingPosition | undefined {
getDraggingPosition(): DraggingPosition | 'invalid' | undefined {
if (this.env.linearItems[this.treeId].length === 0) {
return this.getEmptyTreeDragPosition();
}
Expand All @@ -251,11 +251,11 @@ export class DraggingPositionEvaluation {
}

if (this.areDraggingItemsDescendantOfTarget()) {
return undefined;
return 'invalid';
}

if (!this.canDropAtCurrentTarget()) {
return undefined;
return 'invalid';
}

const { parent } = this.getParentOfLinearItem(
Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/tree/TreeManager.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ export const TreeManager = (): JSX.Element => {
e.preventDefault(); // Allow drop. Also implicitly set by items, but needed here as well for dropping on empty space
dnd.onDragOverTreeHandler(e as any, treeId, containerRef);
},
onDragLeave: e => {
dnd.onDragLeaveContainerHandler(e as any, containerRef);
},
onMouseDown: () => dnd.abortProgrammaticDrag(),
ref: containerRef,
style: { position: 'relative' },
Expand Down
4 changes: 4 additions & 0 deletions packages/core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,10 @@ export interface DragAndDropContextProps<T = any> {
treeId: string,
containerRef: React.MutableRefObject<HTMLElement | undefined>
) => void;
onDragLeaveContainerHandler: (
e: DragEvent,
containerRef: React.MutableRefObject<HTMLElement | undefined>
) => void;
}

export type DraggingPosition =
Expand Down
31 changes: 31 additions & 0 deletions packages/core/test/dnd-basics.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -654,4 +654,35 @@ describe('dnd basics', () => {
});
});
});

it('doesnt drop on last valid location when moving drop to invalid location', async () => {
const test = await new TestUtil().renderOpenTree({});
await test.startDrag('aab');
await test.dragOver('aa');
await test.dragOver('aab');
await test.drop();
await test.expectItemContentsUnchanged('aa');
});

it('sets drop position correctly', async () => {
const test = await new TestUtil().renderOpenTree({});
await test.startDrag('target');
await test.dragOver('aa');
expect(test.treeRef?.dragAndDropContext?.draggingPosition).toStrictEqual({
depth: 1,
linearIndex: 5,
parentItem: 'a',
targetItem: 'aa',
targetType: 'item',
treeId: 'tree-1',
});
});

it('unsets drop position when dragging out', async () => {
const test = await new TestUtil().renderOpenTree({});
await test.startDrag('target');
await test.dragOver('aa');
await test.dragLeave();
expect(test.treeRef?.dragAndDropContext?.draggingPosition).toBe(undefined);
});
});
14 changes: 14 additions & 0 deletions packages/core/test/helpers/TestUtil.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,20 @@ export class TestUtil {
});
}

public async dragLeave() {
(isOutsideOfContainer as jest.Mock).mockReturnValue(true);
await act(async () => {
this.environmentRef?.dragAndDropContext.onDragLeaveContainerHandler(
{
clientX: 9999,
clientY: 9999,
} as any,
{ current: this.containerRef ?? undefined }
);
});
(isOutsideOfContainer as jest.Mock).mockReturnValue(false);
}

public async drop() {
await act(async () => {
fireEvent.drop(window);
Expand Down

0 comments on commit feedb5c

Please sign in to comment.