Skip to content

[Lens] Drag and drop performance improvements #91641

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Feb 21, 2021
Merged

[Lens] Drag and drop performance improvements #91641

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b6c1db6
don't run expensive function twice
mbondyra Feb 17, 2021
2b3a4ac
don't create new function on every dnd context change for changeIndex…
mbondyra Feb 17, 2021
7217907
pass the context directly to the buttons to avoid rerendering the pan…
mbondyra Feb 17, 2021
4c6062e
memoize ChartSwitch
mbondyra Feb 17, 2021
34af0e7
no functional changes, just moving things around
mbondyra Feb 17, 2021
1058db3
flattening structure of activeDropTarget
mbondyra Feb 17, 2021
e9a7736
only register drops if keyboardMode
mbondyra Feb 17, 2021
2e67967
add pointer-none for all the drops that are currently available so we…
mbondyra Feb 18, 2021
8c6e429
not spreading
mbondyra Feb 19, 2021
673019b
removing memoizing order
mbondyra Feb 19, 2021
0b40493
removing some unnecessary memoizations
mbondyra Feb 19, 2021
750e0ac
Merge branch 'master' into lens/perf-dnd
kibanamachine Feb 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions x-pack/plugins/lens/public/drag_drop/drag_drop.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@
.lnsDragDrop-isDropTarget {
@include lnsDroppable;
@include lnsDroppableActive;
> * {
pointer-events: none;
}
}

.lnsDragDrop-isActiveGroup {
Expand Down
82 changes: 35 additions & 47 deletions x-pack/plugins/lens/public/drag_drop/drag_drop.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
ReorderProvider,
DragDropIdentifier,
DraggingIdentifier,
DropTargets,
DropIdentifier,
} from './providers';
import { act } from 'react-dom/test-utils';
import { DropType } from '../types';
Expand All @@ -32,6 +32,7 @@ describe('DragDrop', () => {
setDragging: jest.fn(),
setActiveDropTarget: jest.fn(),
activeDropTarget: undefined,
dropTargetsByOrder: undefined,
keyboardMode: false,
setKeyboardMode: () => {},
setA11yMessage: jest.fn(),
Expand Down Expand Up @@ -255,11 +256,10 @@ describe('DragDrop', () => {
dragging = { id: '1', humanData: { label: 'Label1' } };
}}
setActiveDropTarget={setActiveDropTarget}
activeDropTarget={
({ activeDropTarget: value } as unknown) as DragContextState['activeDropTarget']
}
activeDropTarget={value as DragContextState['activeDropTarget']}
keyboardMode={false}
setKeyboardMode={(keyboardMode) => true}
dropTargetsByOrder={undefined}
registerDropTarget={jest.fn()}
>
<DragDrop
Expand Down Expand Up @@ -349,12 +349,10 @@ describe('DragDrop', () => {
dragging: { ...items[0].value, ghost: { children: <div />, style: {} } },
setActiveDropTarget,
setA11yMessage,
activeDropTarget: {
activeDropTarget: { ...items[1].value, onDrop, dropType: 'move_compatible' },
dropTargetsByOrder: {
'2,0,1,0': { ...items[1].value, onDrop, dropType: 'move_compatible' },
'2,0,2,0': { ...items[2].value, onDrop, dropType: 'replace_compatible' },
},
activeDropTarget: { ...items[1].value, onDrop, dropType: 'move_compatible' },
dropTargetsByOrder: {
'2,0,1,0': { ...items[1].value, onDrop, dropType: 'move_compatible' },
'2,0,2,0': { ...items[2].value, onDrop, dropType: 'replace_compatible' },
},
keyboardMode: true,
}}
Expand Down Expand Up @@ -463,11 +461,9 @@ describe('DragDrop', () => {
dragging: { ...items[0].value, ghost: { children: <div>Hello</div>, style: {} } },
setActiveDropTarget,
setA11yMessage,
activeDropTarget: {
activeDropTarget: { ...items[1].value, onDrop, dropType: 'move_compatible' },
dropTargetsByOrder: {
'2,0,1,0': { ...items[1].value, onDrop, dropType: 'move_compatible' },
},
activeDropTarget: { ...items[1].value, onDrop, dropType: 'move_compatible' },
dropTargetsByOrder: {
'2,0,1,0': { ...items[1].value, onDrop, dropType: 'move_compatible' },
},
keyboardMode: true,
}}
Expand Down Expand Up @@ -525,11 +521,12 @@ describe('DragDrop', () => {
keyboardMode = mode;
}),
setActiveDropTarget: (target?: DragDropIdentifier) => {
activeDropTarget = { activeDropTarget: target } as DropTargets;
activeDropTarget = target as DropIdentifier;
},
activeDropTarget,
setA11yMessage,
registerDropTarget,
dropTargetsByOrder: undefined,
};

const dragDropSharedProps = {
Expand Down Expand Up @@ -665,13 +662,11 @@ describe('DragDrop', () => {
const component = mountComponent({
dragging: { ...items[0] },
keyboardMode: true,
activeDropTarget: {
activeDropTarget: undefined,
dropTargetsByOrder: {
'2,0,0': undefined,
'2,0,1': { ...items[1], onDrop, dropType: 'reorder' },
'2,0,2': { ...items[2], onDrop, dropType: 'reorder' },
},
activeDropTarget: undefined,
dropTargetsByOrder: {
'2,0,0': undefined,
'2,0,1': { ...items[1], onDrop, dropType: 'reorder' },
'2,0,2': { ...items[2], onDrop, dropType: 'reorder' },
},
setActiveDropTarget,
setA11yMessage,
Expand All @@ -693,15 +688,12 @@ describe('DragDrop', () => {
test(`Keyboard navigation: user can drop element to an activeDropTarget`, () => {
const component = mountComponent({
dragging: { ...items[0] },
activeDropTarget: {
activeDropTarget: { ...items[2], dropType: 'reorder', onDrop },
dropTargetsByOrder: {
'2,0,0': { ...items[0], onDrop, dropType: 'reorder' },
'2,0,1': { ...items[1], onDrop, dropType: 'reorder' },
'2,0,2': { ...items[2], onDrop, dropType: 'reorder' },
},
activeDropTarget: { ...items[2], dropType: 'reorder', onDrop },
dropTargetsByOrder: {
'2,0,0': { ...items[0], onDrop, dropType: 'reorder' },
'2,0,1': { ...items[1], onDrop, dropType: 'reorder' },
'2,0,2': { ...items[2], onDrop, dropType: 'reorder' },
},

keyboardMode: true,
});
const keyboardHandler = component
Expand Down Expand Up @@ -747,13 +739,11 @@ describe('DragDrop', () => {
const component = mountComponent({
dragging: { ...items[0] },
keyboardMode: true,
activeDropTarget: {
activeDropTarget: undefined,
dropTargetsByOrder: {
'2,0,0': undefined,
'2,0,1': { ...items[1], onDrop, dropType: 'reorder' },
'2,0,2': { ...items[2], onDrop, dropType: 'reorder' },
},
activeDropTarget: undefined,
dropTargetsByOrder: {
'2,0,0': undefined,
'2,0,1': { ...items[1], onDrop, dropType: 'reorder' },
'2,0,2': { ...items[2], onDrop, dropType: 'reorder' },
},
setA11yMessage,
});
Expand Down Expand Up @@ -799,15 +789,13 @@ describe('DragDrop', () => {
{...defaultContext}
keyboardMode={true}
activeDropTarget={{
activeDropTarget: {
...items[1],
onDrop,
dropType: 'reorder',
},
dropTargetsByOrder: {
'2,0,1,0': undefined,
'2,0,1,1': { ...items[1], onDrop, dropType: 'reorder' },
},
...items[1],
onDrop,
dropType: 'reorder',
}}
dropTargetsByOrder={{
'2,0,1,0': undefined,
'2,0,1,1': { ...items[1], onDrop, dropType: 'reorder' },
}}
dragging={{ ...items[0] }}
setActiveDropTarget={setActiveDropTarget}
Expand Down
Loading