Skip to content

Commit

Permalink
[Lens] Refactor drag and drop (#161257)
Browse files Browse the repository at this point in the history
## Summary

When I created drag and drop for Lens, the API I went for was not the
most readable one. It was designed this way because I wanted to gain
some performance, but it was very hard to maintain the performance gain
with a lot of changes in the drag and drop area because all the pieces
of the code needed to memoized in a tricky way and it wasn't
communicated well.
In the end it works even without these tricks so I decided to simplify
it in this PR.

The main changes include:

1. Instead of multiple `useState` per parameter, we keep all the state
in reducer both for `ReorderProvider` and `RootDragDropProvider`. Thanks
to that we get multiple improvements:
2. The code in `DragDrop` component becomes more descriptive as we don't
run multiple state updates when user executes an action but one state
update describing what actually happens (eg. `dispatchDnd({type:
'selectDropTarget' ....})`. The internal logic of the update lives in
the reducer.
3. We don't have to pass `trackUiCounterEvents` as another prop to
`DragDrop` and run it wherever we need - instead we pass it as a
middleware to the context and run before dispatching (and it's very easy
to add more middlewares if we need extra integrations at some point!)
4. We also run a11y announcements as a middleware instead of inside
`DragDrop` component
5. The `ChildDragDropProvider` props look much cleaner:
    before:
```
      <ChildDragDropProvider
        keyboardMode={keyboardModeState}
        setKeyboardMode={setKeyboardModeState}
        dragging={draggingState.dragging}
        setA11yMessage={setA11yMessage}
        setDragging={setDragging}
        activeDropTarget={activeDropTargetState}
        setActiveDropTarget={setActiveDropTarget}
        registerDropTarget={registerDropTarget}
        dropTargetsByOrder={dropTargetsByOrderState}
        dataTestSubjPrefix={dataTestSubj}
        onTrackUICounterEvent={onTrackUICounterEvent}
      >
        {children}
      </ChildDragDropProvider>
```
after:
```
<ChildDragDropProvider value={[state, dispatch]}>{children}</ChildDragDropProvider>
```
6. Created custom hook `useDragDropContext` instead of using
`useContext(DragContext)` and making DragContext private. This way we
will avoid potential problems with using context outside of root.
7. Bonus thing - if we ever decide to move to redux, the structure is
there already



What I am still not happy with is that the tests are very
domain-dependant instead of user-driven - instead of checking the store
actions, I should check the interface from the user perspective. I will
try to work on it once I find some time between more important tasks
though.
  • Loading branch information
mbondyra authored Jul 11, 2023
1 parent 203c9b0 commit 91a0d2f
Show file tree
Hide file tree
Showing 33 changed files with 1,209 additions and 1,110 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
* Side Public License, v 1.
*/

import React, { useContext, useMemo } from 'react';
import { DragContext, DragDrop, DropOverlayWrapper, DropType } from '@kbn/dom-drag-drop';
import React, { useMemo } from 'react';
import { DragDrop, DropOverlayWrapper, DropType, useDragDropContext } from '@kbn/dom-drag-drop';
import { EuiEmptyPrompt, EuiPanel } from '@elastic/eui';

const DROP_PROPS = {
Expand All @@ -34,8 +34,8 @@ export interface ExampleDropZoneProps {
}

export const ExampleDropZone: React.FC<ExampleDropZoneProps> = ({ onDropField }) => {
const dragDropContext = useContext(DragContext);
const draggingFieldName = dragDropContext.dragging?.id;
const [{ dragging }] = useDragDropContext();
const draggingFieldName = dragging?.id;

const onDroppingField = useMemo(() => {
if (!draggingFieldName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
* Side Public License, v 1.
*/

import React, { useCallback, useContext, useMemo, useRef } from 'react';
import React, { useCallback, useMemo, useRef } from 'react';
import type { DataView } from '@kbn/data-views-plugin/public';
import { generateFilters } from '@kbn/data-plugin/public';
import { ChildDragDropProvider, DragContext } from '@kbn/dom-drag-drop';
import { ChildDragDropProvider, useDragDropContext } from '@kbn/dom-drag-drop';
import {
UnifiedFieldListSidebarContainer,
type UnifiedFieldListSidebarContainerProps,
Expand Down Expand Up @@ -54,7 +54,7 @@ export const FieldListSidebar: React.FC<FieldListSidebarProps> = ({
onAddFieldToWorkspace,
onRemoveFieldFromWorkspace,
}) => {
const dragDropContext = useContext(DragContext);
const dragDropContext = useDragDropContext();
const unifiedFieldListContainerRef = useRef<UnifiedFieldListSidebarContainerApi>(null);
const filterManager = services.data?.query?.filterManager;

Expand All @@ -80,7 +80,7 @@ export const FieldListSidebar: React.FC<FieldListSidebarProps> = ({
}, [unifiedFieldListContainerRef]);

return (
<ChildDragDropProvider {...dragDropContext}>
<ChildDragDropProvider value={dragDropContext}>
<UnifiedFieldListSidebarContainer
ref={unifiedFieldListContainerRef}
variant="responsive"
Expand Down
14 changes: 7 additions & 7 deletions packages/kbn-dom-drag-drop/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,21 @@ We aren't using EUI or another library, due to the fact that Lens visualizations
First, place a RootDragDropProvider at the root of your application.

```js
<RootDragDropProvider onTrackUICounterEvent={...}>
<RootDragDropProvider customMiddleware={...}>
... your app here ...
</RootDragDropProvider>
```

If you have a child React application (e.g. a visualization), you will need to pass the drag / drop context down into it. This can be obtained like so:

```js
const context = useContext(DragContext);
const context = useDragDropContext();
```

In your child application, place a `ChildDragDropProvider` at the root of that, and spread the context into it:
In your child application, place a `ChildDragDropProvider` at the root of that, and assign the context into it:

```js
<ChildDragDropProvider {...context}>... your child app here ...</ChildDragDropProvider>
<ChildDragDropProvider value={context}>... your child app here ...</ChildDragDropProvider>
```

This enables your child application to share the same drag / drop context as the root application.
Expand All @@ -49,7 +49,7 @@ To enable dragging an item, use `DragDrop` with both a `draggable` and a `value`
To enable dropping, use `DragDrop` with both a `dropTypes` attribute that should be an array with at least one value and an `onDrop` handler attribute. `dropType` should only be truthy if is an item being dragged, and if a drop of the dragged item is supported.

```js
const { dragging } = useContext(DragContext);
const [ dndState ] = useDragDropContext()

return (
<DragDrop
Expand All @@ -69,13 +69,13 @@ return (
To create a reordering group, surround the elements from the same group with a `ReorderProvider`:

```js
<ReorderProvider id="groupId">... elements from one group here ...</ReorderProvider>
<ReorderProvider>... elements from one group here ...</ReorderProvider>
```

The children `DragDrop` components must have props defined as in the example:

```js
<ReorderProvider id="groupId">
<ReorderProvider>
<div className="field-list">
{fields.map((f) => (
<DragDrop
Expand Down
6 changes: 4 additions & 2 deletions packages/kbn-dom-drag-drop/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,18 @@

export {
type DragDropIdentifier,
type DragContextValue,
type DragContextState,
type DropType,
type DraggingIdentifier,
type DragDropAction,
type DropOverlayWrapperProps,
DragDrop,
DragContext,
useDragDropContext,
RootDragDropProvider,
ChildDragDropProvider,
ReorderProvider,
DropOverlayWrapper,
type DropOverlayWrapperProps,
} from './src';

export { DropTargetSwapDuplicateCombine } from './src/drop_targets';
Loading

0 comments on commit 91a0d2f

Please sign in to comment.