Skip to content

Commit 960b3e3

Browse files
LFDanLuLFDanLu
authored
Fix useListData crash when dropping on empty list (#3769)
* Fix useListData crash when dropping on empty list * replace placeholder check with collection length check the placeholder key name could be used by the user, so use the collection size instead to default to root if empty
1 parent e5d6994 commit 960b3e3

File tree

6 files changed

+195
-3
lines changed

6 files changed

+195
-3
lines changed

‎packages/@react-spectrum/list/docs/ListView.mdx‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -798,6 +798,15 @@ function BidirectionalDnDListView(props) {
798798
list.moveAfter(target.key, [...keys]);
799799
}
800800
},
801+
onRootDrop: async (e) => {
802+
let {
803+
items
804+
} = e;
805+
let processedItems = await Promise.all(
806+
items.map(async item => JSON.parse(await item.getText('custom-app-type-bidirectional')))
807+
);
808+
list.append(...processedItems);
809+
},
801810
/*- begin highlight -*/
802811
onDragEnd: (e) => {
803812
let {

‎packages/@react-spectrum/list/test/ListViewDnd.test.js‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,15 @@ import {DataTransfer, DataTransferItem, DragEvent, FileSystemDirectoryEntry, Fil
1717
import {DIRECTORY_DRAG_TYPE} from '@react-aria/dnd';
1818
import {DragBetweenListsComplex, DragBetweenListsExample, DragBetweenListsRootOnlyExample, DragExample, DragIntoItemExample, ReorderExample} from '../stories/ListView.stories';
1919
import {Droppable} from '@react-aria/dnd/test/examples';
20+
import {Flex} from '@react-spectrum/layout';
2021
import {globalDndState} from '@react-aria/dnd/src/utils';
22+
import {Item, ListView} from '../';
2123
import {Provider} from '@react-spectrum/provider';
2224
import React from 'react';
25+
import {Text} from '@react-spectrum/text';
2326
import {theme} from '@react-spectrum/theme-default';
27+
import {useDragAndDrop} from '@react-spectrum/dnd';
28+
import {useListData} from '@react-stately/data';
2429
import userEvent from '@testing-library/user-event';
2530

2631
let isReact18 = parseInt(React.version, 10) >= 18;
@@ -920,6 +925,109 @@ describe('ListView', function () {
920925
});
921926
});
922927

928+
it('should call onRootDrop when dropping on a empty list', async function () {
929+
function Example() {
930+
let list1 = useListData({
931+
initialItems: []
932+
});
933+
934+
let list2 = useListData({
935+
initialItems: [
936+
{id: '7', type: 'folder', name: 'Pictures'},
937+
{id: '8', type: 'file', name: 'Adobe Fresco'},
938+
{id: '9', type: 'folder', name: 'Apps'},
939+
{id: '10', type: 'file', name: 'Adobe Illustrator'},
940+
{id: '11', type: 'file', name: 'Adobe Lightroom'},
941+
{id: '12', type: 'file', name: 'Adobe Dreamweaver'}
942+
]
943+
});
944+
945+
let {dragAndDropHooks: list1Hooks} = useDragAndDrop({
946+
...mockUtilityOptions,
947+
acceptedDragTypes: 'all'
948+
});
949+
950+
let {dragAndDropHooks: list2Hooks} = useDragAndDrop({
951+
getItems: (keys) => [...keys].map(key => {
952+
let item = list2.getItem(key);
953+
return {
954+
[`${item.type}`]: JSON.stringify(item),
955+
'text/plain': JSON.stringify(item)
956+
};
957+
})
958+
});
959+
960+
return (
961+
<Flex wrap gap="size-300">
962+
<ListView
963+
aria-label="Droppable listview"
964+
width="size-3600"
965+
height="size-3600"
966+
items={list1.items}
967+
dragAndDropHooks={list1Hooks}>
968+
{(item) => (
969+
<Item textValue={item.name}>
970+
<Text>{item.name}</Text>
971+
</Item>
972+
)}
973+
</ListView>
974+
<ListView
975+
aria-label="Draggable ListView"
976+
selectionMode="multiple"
977+
width="size-3600"
978+
height="size-3600"
979+
items={list2.items}
980+
dragAndDropHooks={list2Hooks}>
981+
{(item) => (
982+
<Item textValue={item.name}>
983+
<Text>{item.name}</Text>
984+
</Item>
985+
)}
986+
</ListView>
987+
</Flex>
988+
);
989+
}
990+
991+
let {getAllByRole} = render(
992+
<Example />
993+
);
994+
let grids = getAllByRole('grid');
995+
expect(grids).toHaveLength(2);
996+
997+
let dropTarget = grids[0];
998+
let list2Rows = within(grids[1]).getAllByRole('row');
999+
dragBetweenLists(list2Rows, dropTarget);
1000+
expect(onReorder).toHaveBeenCalledTimes(0);
1001+
expect(onItemDrop).toHaveBeenCalledTimes(0);
1002+
expect(onRootDrop).toHaveBeenCalledTimes(1);
1003+
expect(onRootDrop).toHaveBeenCalledWith({
1004+
dropOperation: 'move',
1005+
items: [
1006+
{
1007+
kind: 'text',
1008+
types: new Set(['text/plain', 'folder']),
1009+
getText: expect.any(Function)
1010+
},
1011+
{
1012+
kind: 'text',
1013+
types: new Set(['text/plain', 'file']),
1014+
getText: expect.any(Function)
1015+
}
1016+
]
1017+
});
1018+
let items = await Promise.all(onRootDrop.mock.calls[0][0].items.map(async (item) => JSON.parse(await item.getText('text/plain'))));
1019+
expect(items).toContainObject({
1020+
id: '7',
1021+
type: 'folder',
1022+
name: 'Pictures'
1023+
});
1024+
expect(items).toContainObject({
1025+
id: '8',
1026+
type: 'file',
1027+
name: 'Adobe Fresco'
1028+
});
1029+
});
1030+
9231031
it('should call onItemDrop when dropping on a folder in the list', async function () {
9241032
let {getAllByRole} = render(
9251033
<DragBetweenListsComplex firstListDnDOptions={mockUtilityOptions} secondListDnDOptions={{onDragEnd}} />

‎packages/@react-stately/data/src/useListData.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,11 @@ export function createListActions<T, C>(opts: CreateListOptions<T, C>, dispatch:
188188
dispatch(state => {
189189
let index = state.items.findIndex(item => getKey(item) === key);
190190
if (index === -1) {
191-
return;
191+
if (state.items.length === 0) {
192+
index = 0;
193+
} else {
194+
return state;
195+
}
192196
}
193197

194198
return insert(state, index, ...values);
@@ -198,7 +202,11 @@ export function createListActions<T, C>(opts: CreateListOptions<T, C>, dispatch:
198202
dispatch(state => {
199203
let index = state.items.findIndex(item => getKey(item) === key);
200204
if (index === -1) {
201-
return;
205+
if (state.items.length === 0) {
206+
index = 0;
207+
} else {
208+
return state;
209+
}
202210
}
203211

204212
return insert(state, index + 1, ...values);

‎packages/@react-stately/data/test/useListData.test.js‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,64 @@ describe('useListData', function () {
209209
expect(result.current.items[4]).toEqual({name: 'Danni'});
210210
});
211211

212+
it('should not wipe the list state when inserting before with a target key that doesn\'t exist', function () {
213+
let {result} = renderHook(() => useListData({initialItems: initial, getKey, initialSelectedKeys: ['Sam', 'Julia'], initialFilterText: 'test'}));
214+
let initialResult = result.current;
215+
expect(initialResult.items).toBe(initial);
216+
expect(initialResult.selectedKeys).toEqual(new Set(['Sam', 'Julia']));
217+
expect(initialResult.filterText).toBe('test');
218+
219+
act(() => {
220+
result.current.insertBefore('fakeKey', {name: 'Devon'});
221+
});
222+
223+
expect(result.current.items).toBe(initialResult.items);
224+
expect(result.current.selectedKeys).toBe(initialResult.selectedKeys);
225+
expect(result.current.filterText).toBe(initialResult.filterText);
226+
});
227+
228+
it('should not wipe the list state when inserting before with a target key that doesn\'t exist', function () {
229+
let {result} = renderHook(() => useListData({initialItems: initial, getKey, initialSelectedKeys: ['Sam', 'Julia'], initialFilterText: 'test'}));
230+
let initialResult = result.current;
231+
expect(initialResult.items).toBe(initial);
232+
expect(initialResult.selectedKeys).toEqual(new Set(['Sam', 'Julia']));
233+
expect(initialResult.filterText).toBe('test');
234+
235+
act(() => {
236+
result.current.insertAfter('fakeKey', {name: 'Devon'});
237+
});
238+
239+
expect(result.current.items).toBe(initialResult.items);
240+
expect(result.current.selectedKeys).toBe(initialResult.selectedKeys);
241+
expect(result.current.filterText).toBe(initialResult.filterText);
242+
});
243+
244+
it('should insert items into a empty list regardless of the target key provided (insertBefore)', function () {
245+
let {result} = renderHook(() => useListData({initialItems: [], getKey}));
246+
let initialResult = result.current;
247+
248+
act(() => {
249+
result.current.insertBefore('fakeKey', {name: 'Devon'});
250+
});
251+
252+
expect(result.current.items).not.toBe(initialResult.items);
253+
expect(result.current.items).toHaveLength(1);
254+
expect(result.current.items[0]).toEqual({name: 'Devon'});
255+
});
256+
257+
it('should insert items into a empty list regardless of the target key provided (insertAfter)', function () {
258+
let {result} = renderHook(() => useListData({initialItems: [], getKey}));
259+
let initialResult = result.current;
260+
261+
act(() => {
262+
result.current.insertAfter('fakeKey', {name: 'Devon'});
263+
});
264+
265+
expect(result.current.items).not.toBe(initialResult.items);
266+
expect(result.current.items).toHaveLength(1);
267+
expect(result.current.items[0]).toEqual({name: 'Devon'});
268+
});
269+
212270
it('should remove an item', function () {
213271
let {result} = renderHook(() => useListData({initialItems: initial, getKey, initialSelectedKeys: ['Sam', 'Julia']}));
214272
let initialResult = result.current;

‎packages/@react-stately/layout/src/ListLayout.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ export class ListLayout<T> extends Layout<Node<T>> implements KeyboardDelegate,
476476
y += this.virtualizer.visibleRect.y;
477477

478478
let key = this.virtualizer.keyAtPoint(new Point(x, y));
479-
if (key == null) {
479+
if (key == null || this.collection.size === 0) {
480480
return {type: 'root'};
481481
}
482482

‎packages/dev/docs/pages/blog/drag-and-drop.mdx‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,15 @@ function BidirectionalDnDListView(props) {
153153
list.moveAfter(target.key, [...keys]);
154154
}
155155
},
156+
onRootDrop: async (e) => {
157+
let {
158+
items
159+
} = e;
160+
let processedItems = await Promise.all(
161+
items.map(async item => JSON.parse(await item.getText('custom-app-type-bidirectional')))
162+
);
163+
list.append(...processedItems);
164+
},
156165
/*- begin highlight -*/
157166
onDragEnd: (e) => {
158167
let {

0 commit comments

Comments
 (0)