Skip to content

Commit

Permalink
Update default sortable keyboard coordinate getter
Browse files Browse the repository at this point in the history
  • Loading branch information
clauderic committed Mar 17, 2022
1 parent 00d1dbc commit 30bbd12
Show file tree
Hide file tree
Showing 12 changed files with 77 additions and 52 deletions.
12 changes: 12 additions & 0 deletions .changeset/sortable-keyboard-coordinates.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
'@dnd-kit/sortable': major
---

The default `sortableKeyboardCoordinates` function that is exported from the `@dnd-kit/sortable` package has been updated to better handle cases where the collision rectangle is overlapping droppable rectangles. For example, for `down` arrow key, the default function had logic that would only consider collisions against droppables that were below the `bottom` edge of the collision rect. This was problematic when the collision rect was overlapping droppable rects, because it meant that it's bottom edge was below the top edge of the droppable, and that resulted in that droppable being skipped.

```diff
- collisionRect.bottom > droppableRect.top
+ collisionRect.top > droppableRect.top
```

This change should be backwards compatible for most consumers, but may introduce regressions in some use-cases, especially for consumers that may have copied the multiple containers examples. There is now a custom sortable keyboard coordinate getter [optimized for multiple containers that you can refer to](https://github.com/clauderic/dnd-kit/tree/master/stories/2%20-%20Presets/Sortable/multipleContainersKeyboardCoordinates.ts).
3 changes: 2 additions & 1 deletion cypress.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@
"supportFile": "cypress/support/index.ts",
"projectId": "zkn9qu",
"baseUrl": "http://localhost:6006/",
"viewportHeight": 1000
"viewportHeight": 1000,
"scrollBehavior": false
}
2 changes: 1 addition & 1 deletion cypress/integration/sortable_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ describe('Sortable Virtualized List', () => {
});

describe('Stress test', () => {
it.skip('Multiple actions in both directions', () => {
it('Multiple actions in both directions', () => {
const initialIndex = 10;
const id = getIdForIndex(initialIndex);
const delta = 10;
Expand Down
6 changes: 5 additions & 1 deletion cypress/support/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,11 @@ Cypress.Commands.add(
force: true,
})
.wait(150)
.type(Keys.Space, {scrollBehavior: false, log: false, force: true});
.type(Keys.Space, {
force: true,
scrollBehavior: false,
})
.wait(250);
}
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ export const sortableKeyboardCoordinates: KeyboardCoordinateGetter = (
{
context: {
active,
collisionRect,
droppableRects,
droppableContainers,
collisionRect,
over,
scrollableAncestors,
},
}
Expand All @@ -48,22 +49,22 @@ export const sortableKeyboardCoordinates: KeyboardCoordinateGetter = (

switch (event.code) {
case KeyboardCode.Down:
if (collisionRect.top + collisionRect.height <= rect.top) {
if (collisionRect.top < rect.top) {
filteredContainers.push(entry);
}
break;
case KeyboardCode.Up:
if (collisionRect.top >= rect.top + rect.height) {
if (collisionRect.top > rect.top) {
filteredContainers.push(entry);
}
break;
case KeyboardCode.Left:
if (collisionRect.left >= rect.left + rect.width) {
if (collisionRect.left > rect.left) {
filteredContainers.push(entry);
}
break;
case KeyboardCode.Right:
if (collisionRect.left + collisionRect.width <= rect.left) {
if (collisionRect.left < rect.left) {
filteredContainers.push(entry);
}
break;
Expand All @@ -77,7 +78,11 @@ export const sortableKeyboardCoordinates: KeyboardCoordinateGetter = (
droppableContainers: filteredContainers,
pointerCoordinates: null,
});
const closestId = getFirstCollision(collisions, 'id');
let closestId = getFirstCollision(collisions, 'id');

if (closestId === over?.id && collisions.length > 1) {
closestId = collisions[1].id;
}

if (closestId != null) {
const newDroppable = droppableContainers.get(closestId);
Expand Down
2 changes: 2 additions & 0 deletions stories/2 - Presets/Sortable/5-Virtualized.story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ function Sortable({
const sensors = useSensors(
useSensor(PointerSensor),
useSensor(KeyboardSensor, {
// Disable smooth scrolling in Cypress automated tests
scrollBehavior: 'Cypress' in window ? 'auto' : undefined,
coordinateGetter: sortableKeyboardCoordinates,
})
);
Expand Down
4 changes: 2 additions & 2 deletions stories/2 - Presets/Sortable/MultipleContainers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ import {
useSortable,
arrayMove,
defaultAnimateLayoutChanges,
sortableKeyboardCoordinates,
verticalListSortingStrategy,
SortingStrategy,
horizontalListSortingStrategy,
} from '@dnd-kit/sortable';
import {CSS} from '@dnd-kit/utilities';
import {coordinateGetter as multipleContainersCoordinateGetter} from './multipleContainersKeyboardCoordinates';

import {Item, Container, ContainerProps} from '../../components';

Expand Down Expand Up @@ -151,7 +151,7 @@ export function MultipleContainers({
handle = false,
items: initialItems,
containerStyle,
coordinateGetter = sortableKeyboardCoordinates,
coordinateGetter = multipleContainersCoordinateGetter,
getItemStyles = () => ({}),
wrapperStyle = () => ({}),
minimal = false,
Expand Down
2 changes: 2 additions & 0 deletions stories/2 - Presets/Sortable/Sortable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ export function Sortable({
activationConstraint,
}),
useSensor(KeyboardSensor, {
// Disable smooth scrolling in Cypress automated tests
scrollBehavior: 'Cypress' in window ? 'auto' : undefined,
coordinateGetter,
})
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const directions: string[] = [

export const coordinateGetter: KeyboardCoordinateGetter = (
event,
{context: {active, droppableContainers, collisionRect}}
{context: {active, droppableRects, droppableContainers, collisionRect}}
) => {
if (directions.includes(event.code)) {
event.preventDefault();
Expand All @@ -31,7 +31,7 @@ export const coordinateGetter: KeyboardCoordinateGetter = (
return;
}

const rect = entry?.rect.current;
const rect = droppableRects.get(entry.id);

if (!rect) {
return;
Expand All @@ -43,7 +43,9 @@ export const coordinateGetter: KeyboardCoordinateGetter = (
const {type, children} = data;

if (type === 'container' && children?.length > 0) {
return;
if (active.data.current?.type !== 'container') {
return;
}
}
}

Expand Down Expand Up @@ -74,6 +76,7 @@ export const coordinateGetter: KeyboardCoordinateGetter = (
const collisions = closestCorners({
active,
collisionRect: collisionRect,
droppableRects,
droppableContainers: filteredContainers,
pointerCoordinates: null,
});
Expand All @@ -85,6 +88,13 @@ export const coordinateGetter: KeyboardCoordinateGetter = (
const newRect = newDroppable?.rect.current;

if (newNode && newRect) {
if (newDroppable.data.current?.type === 'container') {
return {
x: newRect.left + (newRect.width - collisionRect.width) / 2,
y: newRect.top + (newRect.height - collisionRect.height) / 2,
};
}

return {
x: newRect.left,
y: newRect.top,
Expand Down
9 changes: 3 additions & 6 deletions stories/3 - Examples/Advanced/Pages/Page.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,8 @@
}

&.clone {
margin-top: 10px;
margin-left: 10px;

.Page {
transform: scale(1.025);
transform: translate3d(10px, 10px, 0) scale(1.025);
animation: pop 150ms cubic-bezier(0.18, 0.67, 0.6, 1.22);
box-shadow: 0 0 0 1px rgba(63, 63, 68, 0.05),
0 1px 6px 0 rgba(34, 33, 81, 0.3);
Expand Down Expand Up @@ -207,9 +204,9 @@

@keyframes pop {
0% {
transform: translate3d(-10px, -10px, 0) scale(1);
transform: translate3d(0px, 0px, 0) scale(1);
}
100% {
transform: translate3d(0px, 0px, 0) scale(1.025);
transform: translate3d(10px, 10px, 0) scale(1.025);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
rectSortingStrategy,
} from '@dnd-kit/sortable';

import {coordinateGetter} from './keyboardCoordinates';
import {Sortable} from '../../../2 - Presets/Sortable/Sortable';
import {MultipleContainers} from '../../../2 - Presets/Sortable/MultipleContainers';
import {PlayingCard, getDeckOfCards} from './PlayingCard';
Expand Down Expand Up @@ -50,7 +49,6 @@ export const SingleDeck = () => {
zIndex: isDragging ? deck.length - overIndex : deck.length - index,
opacity: isDragging && !isDragOverlay ? 0.3 : undefined,
})}
coordinateGetter={coordinateGetter}
/>
</div>
);
Expand Down Expand Up @@ -152,7 +150,6 @@ export const MultipleDecks = () => {
: deck.length - index,
};
}}
coordinateGetter={coordinateGetter}
minimal
/>
</>
Expand Down
53 changes: 24 additions & 29 deletions stories/3 - Examples/Tree/keyboardCoordinates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export const sortableTreeKeyboardCoordinates: (
event,
{
currentCoordinates,
context: {active, over, collisionRect, droppableContainers},
context: {active, over, collisionRect, droppableRects, droppableContainers},
}
) => {
if (directions.includes(event.code)) {
Expand Down Expand Up @@ -73,41 +73,36 @@ export const sortableTreeKeyboardCoordinates: (

const containers: DroppableContainer[] = [];

const overRect = over?.id
? droppableContainers.get(over.id)?.rect.current
: undefined;

if (overRect) {
droppableContainers.forEach((container) => {
if (container?.disabled) {
return;
}
droppableContainers.forEach((container) => {
if (container?.disabled || container.id === over?.id) {
return;
}

const rect = container?.rect.current;
const rect = container?.rect.current;

if (!rect) {
return;
}
if (!rect) {
return;
}

switch (event.code) {
case KeyboardCode.Down:
if (overRect.top < rect.top) {
containers.push(container);
}
break;
case KeyboardCode.Up:
if (overRect.top > rect.top) {
containers.push(container);
}
break;
}
});
}
switch (event.code) {
case KeyboardCode.Down:
if (collisionRect.top < rect.top) {
containers.push(container);
}
break;
case KeyboardCode.Up:
if (collisionRect.top > rect.top) {
containers.push(container);
}
break;
}
});

const collisions = closestCorners({
active,
collisionRect: collisionRect,
collisionRect,
pointerCoordinates: null,
droppableRects,
droppableContainers: containers,
});
const closestId = getFirstCollision(collisions, 'id');
Expand Down

0 comments on commit 30bbd12

Please sign in to comment.