Skip to content

Commit d8d3fa2

Browse files
authored
fix: Limit auto-scroll to the nearest scroll container when dragging sortable list items (#5228)
1 parent 6829e8f commit d8d3fa2

3 files changed

Lines changed: 233 additions & 3 deletions

File tree

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
import React, { useContext, useState } from 'react';
4+
5+
import { Box, List, SegmentedControl } from '~components';
6+
7+
import AppContext, { AppContextType } from '../app/app-context';
8+
import { SimplePage } from '../app/templates';
9+
10+
type Layout = 'drawer-scroller' | 'drawer' | 'page';
11+
12+
type PageContext = React.Context<AppContextType<{ layout: Layout }>>;
13+
14+
interface Item {
15+
id: string;
16+
content: string;
17+
}
18+
19+
const initialItems: Item[] = Array.from({ length: 30 }, (_, index) => ({
20+
id: `item-${index + 1}`,
21+
content: `Item ${index + 1}`,
22+
}));
23+
24+
const outerHeight = 600;
25+
26+
function SortableList() {
27+
const [items, setItems] = useState<readonly Item[]>(initialItems);
28+
return (
29+
<List
30+
ariaLabel="Sortable list"
31+
items={items}
32+
sortable={true}
33+
renderItem={item => ({ id: item.id, content: item.content })}
34+
onSortingChange={event => setItems(event.detail.items)}
35+
/>
36+
);
37+
}
38+
39+
function StickyDrawerLayout({ ownScroller }: { ownScroller: boolean }) {
40+
return (
41+
<div data-testid="outer-scroller" style={{ blockSize: outerHeight, overflow: 'auto', display: 'flex' }}>
42+
<div style={{ flex: 1 }}>
43+
{Array.from({ length: 150 }, (_, index) => (
44+
<Box key={index} padding="xxs">
45+
Main content row {index + 1}
46+
</Box>
47+
))}
48+
</div>
49+
<aside
50+
data-testid="sticky-drawer"
51+
style={{
52+
position: 'sticky',
53+
insetBlockStart: 0,
54+
alignSelf: 'flex-start',
55+
inlineSize: 320,
56+
blockSize: ownScroller ? outerHeight : undefined,
57+
display: 'flex',
58+
flexDirection: 'column',
59+
}}
60+
>
61+
<Box variant="h2" padding="s">
62+
Drawer
63+
</Box>
64+
{ownScroller ? (
65+
<div data-testid="drawer-list-scroller" style={{ flex: 1, overflow: 'auto' }}>
66+
<SortableList />
67+
</div>
68+
) : (
69+
<SortableList />
70+
)}
71+
</aside>
72+
</div>
73+
);
74+
}
75+
76+
export default function SortableListAutoScrollPage() {
77+
const {
78+
urlParams: { layout = 'drawer-scroller' },
79+
setUrlParams,
80+
} = useContext(AppContext as PageContext);
81+
return (
82+
<SimplePage
83+
title="Sortable list auto-scroll"
84+
screenshotArea={{}}
85+
settings={
86+
<SegmentedControl
87+
label="Layout"
88+
selectedId={layout}
89+
onChange={({ detail }) => setUrlParams({ layout: detail.selectedId as Layout })}
90+
options={[
91+
{ id: 'drawer-scroller', text: 'Sticky drawer with own scroller' },
92+
{ id: 'drawer', text: 'Sticky drawer without own scroller' },
93+
{ id: 'page', text: 'Page flow' },
94+
]}
95+
/>
96+
}
97+
>
98+
{layout === 'page' ? <SortableList /> : <StickyDrawerLayout ownScroller={layout === 'drawer-scroller'} />}
99+
</SimplePage>
100+
);
101+
}

‎src/internal/components/sortable-area/index.tsx‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,16 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
import React, { useEffect, useRef } from 'react';
5-
import { DndContext, DragOverlay } from '@dnd-kit/core';
5+
import { AutoScrollOptions, DndContext, DragOverlay, TraversalOrder } from '@dnd-kit/core';
66
import { arrayMove, SortableContext, useSortable, verticalListSortingStrategy } from '@dnd-kit/sortable';
77
import { CSS } from '@dnd-kit/utilities';
88
import clsx from 'clsx';
99

1010
import { Portal } from '@cloudscape-design/component-toolkit/internal';
1111

1212
import { fireNonCancelableEvent } from '../../events';
13+
import { isElement, isHTMLElement } from '../../utils/dom';
14+
import { getFirstScrollableParent } from '../../utils/scrollable-containers';
1315
import { joinStrings } from '../../utils/strings';
1416
import { SortableAreaProps } from './interfaces';
1517
import { EventName } from './keyboard-sensor/utilities/events';
@@ -37,10 +39,12 @@ export default function SortableArea<Item>({
3739
const isDragging = activeItemId !== null;
3840
const announcements = useLiveAnnouncements({ items, itemDefinition, isDragging, ...i18nStrings });
3941
const portalContainer = usePortalContainer();
42+
const { autoScroll, setAutoScrollBoundary } = useScopedAutoScroll();
4043
return (
4144
<DndContext
4245
sensors={sensors}
4346
collisionDetection={collisionDetection}
47+
autoScroll={autoScroll}
4448
accessibility={{
4549
announcements,
4650
restoreFocus: false,
@@ -49,8 +53,12 @@ export default function SortableArea<Item>({
4953
: undefined,
5054
container: portalContainer ?? undefined,
5155
}}
52-
onDragStart={({ active }) => setActiveItemId(active.id)}
56+
onDragStart={({ active, activatorEvent }) => {
57+
setAutoScrollBoundary(activatorEvent.target);
58+
setActiveItemId(active.id);
59+
}}
5360
onDragEnd={event => {
61+
setAutoScrollBoundary(null);
5462
setActiveItemId(null);
5563
const { active, over } = event;
5664
if (over && active.id !== over.id) {
@@ -60,7 +68,10 @@ export default function SortableArea<Item>({
6068
fireNonCancelableEvent(onItemsChange, { items: arrayMove([...items], oldIndex, newIndex), movedItem });
6169
}
6270
}}
63-
onDragCancel={() => setActiveItemId(null)}
71+
onDragCancel={() => {
72+
setAutoScrollBoundary(null);
73+
setActiveItemId(null);
74+
}}
6475
>
6576
<SortableContext
6677
disabled={disableReorder}
@@ -110,6 +121,26 @@ export default function SortableArea<Item>({
110121
);
111122
}
112123

124+
// dnd-kit auto-scrolls every scrollable ancestor up to the first fixed-position element, outermost first by default
125+
// (https://github.com/clauderic/dnd-kit/issues/1893). Restrict it to the sortable area's nearest scroll container so
126+
// that dragging inside e.g. a sticky drawer does not scroll unrelated outer containers. Without a scrollable ancestor,
127+
// all containers (including the window) remain eligible.
128+
function useScopedAutoScroll() {
129+
const boundaryRef = useRef<HTMLElement | null>(null);
130+
const autoScroll: AutoScrollOptions = {
131+
order: TraversalOrder.ReversedTreeOrder,
132+
canScroll: element => !boundaryRef.current || boundaryRef.current.contains(element),
133+
};
134+
const setAutoScrollBoundary = (dragStartTarget: EventTarget | null) => {
135+
let element = isElement(dragStartTarget) ? dragStartTarget : null;
136+
while (element && !isHTMLElement(element)) {
137+
element = element.parentElement;
138+
}
139+
boundaryRef.current = element ? (getFirstScrollableParent(element) ?? null) : null;
140+
};
141+
return { autoScroll, setAutoScrollBoundary };
142+
}
143+
113144
function usePortalContainer() {
114145
const portalContainerRef = useRef(typeof document !== 'undefined' ? document.createElement('div') : null);
115146
useEffect(() => {

‎src/list/__tests__/list-sortable.test.tsx‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,28 @@
11
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
22
// SPDX-License-Identifier: Apache-2.0
33
import React from 'react';
4+
import { AutoScrollOptions, TraversalOrder } from '@dnd-kit/core';
45
import { act, fireEvent, render } from '@testing-library/react';
56

67
import TestI18nProvider from '../../../lib/components/i18n/testing';
78
import List, { ListProps } from '../../../lib/components/list';
89
import createWrapper from '../../../lib/components/test-utils/dom';
910
import InternalDragHandleWrapper from '../../../lib/components/test-utils/dom/internal/drag-handle';
1011

12+
const mockDndContextAutoScroll: { current: AutoScrollOptions | undefined } = { current: undefined };
13+
14+
jest.mock('@dnd-kit/core', () => {
15+
const actual = jest.requireActual('@dnd-kit/core');
16+
const { createElement } = jest.requireActual('react');
17+
return {
18+
...actual,
19+
DndContext: (props: { autoScroll?: AutoScrollOptions }) => {
20+
mockDndContextAutoScroll.current = props.autoScroll;
21+
return createElement(actual.DndContext, props);
22+
},
23+
};
24+
});
25+
1126
interface Item {
1227
id: string;
1328
content: string;
@@ -246,4 +261,87 @@ describe('List - Sortable', () => {
246261
dragHandle.click({ button: 1 });
247262
expect(dragWrapper.findVisibleDirectionButtonBlockEnd()).toBeFalsy();
248263
});
264+
265+
describe('auto-scroll', () => {
266+
const interact = (action: () => void) =>
267+
act(async () => {
268+
action();
269+
await new Promise(resolve => setTimeout(resolve, 0));
270+
});
271+
const canAutoScroll = (element: Element) => mockDndContextAutoScroll.current!.canScroll!(element);
272+
273+
function renderInScrollContainers({ innerScrolls }: { innerScrolls: boolean }) {
274+
const { container } = render(
275+
<div data-testid="outer" style={{ overflowY: 'auto' }}>
276+
<div data-testid="inner" style={{ overflowY: 'auto' }}>
277+
<ControlledList
278+
items={defaultItems}
279+
renderItem={item => ({ id: item.id, content: item.content })}
280+
sortable={true}
281+
/>
282+
</div>
283+
</div>
284+
);
285+
const outer = container.querySelector<HTMLElement>('[data-testid="outer"]')!;
286+
const inner = container.querySelector<HTMLElement>('[data-testid="inner"]')!;
287+
// jsdom has no layout, so make the containers overflow explicitly.
288+
const overflowing = innerScrolls ? [outer, inner] : [];
289+
for (const element of overflowing) {
290+
Object.defineProperty(element, 'scrollHeight', { configurable: true, value: 1000 });
291+
Object.defineProperty(element, 'clientHeight', { configurable: true, value: 100 });
292+
}
293+
const list = createWrapper(container).findList()!;
294+
const item = list.findItemById('item1')!.getElement();
295+
const handle = list.findItemById('item1')!.findDragHandle()!.getElement();
296+
return { outer, inner, item, handle };
297+
}
298+
299+
test('checks the innermost scroll container first', () => {
300+
renderInScrollContainers({ innerScrolls: true });
301+
expect(mockDndContextAutoScroll.current!.order).toBe(TraversalOrder.ReversedTreeOrder);
302+
});
303+
304+
test('only auto-scrolls the nearest scroll container and its descendants while dragging', async () => {
305+
const { outer, inner, item, handle } = renderInScrollContainers({ innerScrolls: true });
306+
307+
await interact(() => pressKey(handle, 'Space'));
308+
309+
expect(canAutoScroll(inner)).toBe(true);
310+
expect(canAutoScroll(item)).toBe(true);
311+
expect(canAutoScroll(outer)).toBe(false);
312+
expect(canAutoScroll(document.documentElement)).toBe(false);
313+
314+
await interact(() => pressKey(handle, 'Space'));
315+
316+
expect(canAutoScroll(outer)).toBe(true);
317+
expect(canAutoScroll(document.documentElement)).toBe(true);
318+
});
319+
320+
test('keeps every container eligible for auto-scroll when the list has no scrollable ancestor', async () => {
321+
const { outer, inner, handle } = renderInScrollContainers({ innerScrolls: false });
322+
323+
await interact(() => pressKey(handle, 'Space'));
324+
325+
expect(canAutoScroll(inner)).toBe(true);
326+
expect(canAutoScroll(outer)).toBe(true);
327+
expect(canAutoScroll(document.documentElement)).toBe(true);
328+
329+
await interact(() => pressKey(handle, 'Escape'));
330+
});
331+
332+
test('limits auto-scroll when the drag starts on the drag handle icon and stops limiting it on cancel', async () => {
333+
const { outer, inner, handle } = renderInScrollContainers({ innerScrolls: true });
334+
const icon = handle.querySelector('svg')!;
335+
expect(icon).not.toBeNull();
336+
337+
await interact(() => fireEvent.click(icon));
338+
339+
expect(canAutoScroll(inner)).toBe(true);
340+
expect(canAutoScroll(outer)).toBe(false);
341+
342+
await interact(() => pressKey(handle, 'Escape'));
343+
344+
expect(canAutoScroll(outer)).toBe(true);
345+
});
346+
});
249347
});

0 commit comments

Comments
 (0)