Skip to content
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.

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

Redux for experiments table drag and drop #2097

Merged
merged 10 commits into from
Jul 26, 2022
9 changes: 3 additions & 6 deletions webview/src/experiments/components/Experiments.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import buildDynamicColumns from '../util/buildDynamicColumns'
import { sendMessage } from '../../shared/vscode'
import { WebviewWrapper } from '../../shared/components/webviewWrapper/WebviewWrapper'
import { GetStarted } from '../../shared/components/getStarted/GetStarted'
import { DragDropProvider } from '../../shared/components/dragDrop/DragDropContext'
import { EmptyState } from '../../shared/components/emptyState/EmptyState'

const DEFAULT_COLUMN_WIDTH = 90
Expand Down Expand Up @@ -224,11 +223,9 @@ export const ExperimentsTable: React.FC<{
}

return (
<DragDropProvider>
<RowSelectionProvider>
<Table instance={instance} tableData={tableData} />
</RowSelectionProvider>
</DragDropProvider>
<RowSelectionProvider>
<Table instance={instance} tableData={tableData} />
</RowSelectionProvider>
)
}

Expand Down
27 changes: 21 additions & 6 deletions webview/src/experiments/components/table/Table.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
*/
/* eslint jest/expect-expect: ["error", { "assertFunctionNames": ["expect", "expectHeaders"] }] */
import '@testing-library/jest-dom/extend-expect'
import { configureStore } from '@reduxjs/toolkit'
import {
cleanup,
fireEvent,
queries,
render,
screen
} from '@testing-library/react'
import { Provider } from 'react-redux'
import { Experiment, TableData } from 'dvc/src/experiments/webview/contract'
import { MessageFromWebviewType } from 'dvc/src/webview/contract'
import React from 'react'
Expand All @@ -20,7 +22,6 @@ import { Table } from './Table'
import styles from './styles.module.scss'
import { ExperimentsTable } from '../Experiments'
import * as ColumnOrder from '../../hooks/useColumnOrder'

import { vsCodeApi } from '../../../shared/api'
import {
expectHeaders,
Expand All @@ -29,6 +30,7 @@ import {
} from '../../../test/sort'
import { dragAndDrop } from '../../../test/dragDrop'
import { DragEnterDirection } from '../../../shared/components/dragDrop/util'
import { experimentsReducers } from '../../store'
import { customQueries } from '../../../test/queries'

jest.mock('../../../shared/api')
Expand Down Expand Up @@ -123,14 +125,23 @@ describe('Table', () => {
}
const renderTable = (testData = {}, tableInstance = instance) => {
const tableData = { ...dummyTableData, ...testData }
return render(<Table instance={tableInstance} tableData={tableData} />)
return render(
<Provider store={configureStore({ reducer: experimentsReducers })}>
<Table instance={tableInstance} tableData={tableData} />
</Provider>
)
}
const renderExperimentsTable = (
data: TableData = sortingTableDataFixture
) => {
return render(<ExperimentsTable tableData={data} />, {
queries: { ...queries, ...customQueries }
})
return render(
<Provider store={configureStore({ reducer: experimentsReducers })}>
<ExperimentsTable tableData={data} />
</Provider>,
{
queries: { ...queries, ...customQueries }
}
)
}

beforeAll(() => {
Expand Down Expand Up @@ -335,7 +346,11 @@ describe('Table', () => {
...sortingTableDataFixture,
columnWidths
}
render(<ExperimentsTable tableData={tableDataWithColumnSetting} />)
render(
<Provider store={configureStore({ reducer: experimentsReducers })}>
<ExperimentsTable tableData={tableDataWithColumnSetting} />
</Provider>
)
const [experimentColumnResizeHandle] = await screen.findAllByRole(
'separator'
)
Expand Down
8 changes: 7 additions & 1 deletion webview/src/experiments/index.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import React from 'react'
import ReactDOM from 'react-dom/client'
import { Provider } from 'react-redux'
import '../shared/style.scss'
import { App } from './components/App'
import { experimentsStore } from './store'

const root = ReactDOM.createRoot(document.querySelector('#root') as HTMLElement)
root.render(<App />)
root.render(
<Provider store={experimentsStore}>
<App />
</Provider>
)
13 changes: 13 additions & 0 deletions webview/src/experiments/store.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { configureStore } from '@reduxjs/toolkit'
import dragAndDropReducer from '../shared/components/dragDrop/dragDropSlice'

export const experimentsReducers = {
dragAndDrop: dragAndDropReducer
}

export const experimentsStore = configureStore({
reducer: experimentsReducers
})

export type ExperimentsState = ReturnType<typeof experimentsStore.getState>
export type ExperimentsDispatch = typeof experimentsStore.dispatch
76 changes: 0 additions & 76 deletions webview/src/shared/components/dragDrop/DragDropContext.tsx

This file was deleted.

65 changes: 41 additions & 24 deletions webview/src/shared/components/dragDrop/DragDropWorkbench.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import React, { DragEvent, useContext } from 'react'
import React, { DragEvent } from 'react'
import { useDispatch, useSelector } from 'react-redux'
import { makeTarget } from './DragDropContainer'
import { DragDropContext, DragDropContextValue } from './DragDropContext'
import { setGroup } from './dragDropSlice'
import { ExperimentsState } from '../../../experiments/store'

export type OnDrop = (draggedId: string, draggedOverId: string) => void
export type OnDragStart = (draggedId: string) => void
Expand All @@ -26,21 +28,33 @@ export const Draggable: React.FC<DraggableProps> = ({
onDrop,
onDragOver,
onDragStart
// eslint-disable-next-line sonarjs/cognitive-complexity
}) => {
const { groupStates, setGroupState } =
useContext<DragDropContextValue>(DragDropContext)
const groupStates = useSelector(
(state: ExperimentsState) => state.dragAndDrop.groups
)
const dispatch = useDispatch()

const groupState = groupStates?.[group] || {}
const groupState = groupStates[group] || {}
const { draggedOverId, draggedId } = groupState

const modifyGroup = (id: string) => {
dispatch(
setGroup({
group: {
...groupState,
draggedId: id
},
id: group
})
)
}

const handleDragStart = (e: DragEvent<HTMLElement>) => {
const { id } = e.currentTarget
e.dataTransfer.effectAllowed = 'move'
e.dataTransfer.dropEffect = 'move'
setGroupState?.(group, {
...groupState,
draggedId: id
})
modifyGroup(id)
onDragStart?.(id)
}

Expand All @@ -52,28 +66,31 @@ export const Draggable: React.FC<DraggableProps> = ({
}

const handleDragEnter = (e: DragEvent<HTMLElement>) => {
const { id } = e.currentTarget
!disabled &&
draggedId &&
id !== draggedId &&
id !== draggedOverId &&
(setGroupState?.(group, {
...groupState,
draggedOverId: id
}) ||
onDragOver?.(draggedId, id))
if (!disabled && draggedId) {
const { id } = e.currentTarget

if (id !== draggedId && id !== draggedOverId) {
modifyGroup(id)
onDragOver?.(draggedId, id)
}
}
}

const handleDragOver = (e: DragEvent<HTMLElement>) => {
e.preventDefault()
}

const handleDragEnd = () => {
setGroupState?.(group, {
...groupState,
draggedId: undefined,
draggedOverId: undefined
})
dispatch(
setGroup({
group: {
...groupState,
draggedId: undefined,
draggedOverId: undefined
},
id: group
})
)
}

const item = (
Expand Down
23 changes: 21 additions & 2 deletions webview/src/shared/components/dragDrop/dragDropSlice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,22 @@ export type DraggedInfo =
group?: string
}
| undefined
export interface DragDropGroupState {
draggedId?: string
draggedOverId?: string
}

export type GroupStates = {
[group: string]: DragDropGroupState | undefined
}
export interface DragDropState {
draggedRef: DraggedInfo
groups: GroupStates
}

export const dragDropInitialState: DragDropState = {
draggedRef: undefined
draggedRef: undefined,
groups: {}
}

export const dragDropSlice = createSlice({
Expand All @@ -24,10 +34,19 @@ export const dragDropSlice = createSlice({
...state,
draggedRef: action.payload
}
},
setGroup: (
state,
action: PayloadAction<{ id: string; group: DragDropGroupState }>
) => {
return {
...state,
groups: { ...state.groups, [action.payload.id]: action.payload.group }
}
}
}
})

export const { changeRef } = dragDropSlice.actions
export const { changeRef, setGroup } = dragDropSlice.actions

export default dragDropSlice.reducer
17 changes: 13 additions & 4 deletions webview/src/stories/Table.stories.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { configureStore } from '@reduxjs/toolkit'
import React from 'react'
import { Provider } from 'react-redux'
import { ComponentStory } from '@storybook/react'
import { Meta } from '@storybook/react/types-6-0'
import rowsFixture from 'dvc/src/test/fixtures/expShow/rows'
Expand All @@ -22,6 +24,7 @@ import {
setExperimentsAsSelected,
setExperimentsAsStarred
} from '../test/tableDataFixture'
import { experimentsReducers } from '../experiments/store'

const tableData: TableData = {
changes: workspaceChangesFixture,
Expand Down Expand Up @@ -100,7 +103,11 @@ export default {
} as Meta

const Template: ComponentStory<typeof Experiments> = ({ tableData }) => {
return <Experiments tableData={tableData} />
return (
<Provider store={configureStore({ reducer: experimentsReducers })}>
<Experiments tableData={tableData} />
</Provider>
)
}

export const WithData = Template.bind({})
Expand Down Expand Up @@ -196,9 +203,11 @@ WithNoSortsOrFilters.args = {

export const Scrolled: ComponentStory<typeof Experiments> = ({ tableData }) => {
return (
<div style={{ height: 400, width: 600 }}>
<Experiments tableData={tableData} />
</div>
<Provider store={configureStore({ reducer: experimentsReducers })}>
<div style={{ height: 400, width: 600 }}>
<Experiments tableData={tableData} />
</div>
</Provider>
Comment on lines +206 to +210
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe Provider could be added with a decorator?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good idea, I'll add this in my follow up PR

)
}
Scrolled.play = async ({ canvasElement }) => {
Expand Down
Loading