Add drag and drop mode to plots #4934
Conversation
| @@ -151,10 +151,10 @@ const getCustomPlotData = ( | |||
| const updatedSpec = truncateVegaSpecTitles(spec, nbItemsPerRow, height) | |||
|
|
|||
| return { | |||
| content: updatedSpec, | |||
There was a problem hiding this comment.
Making this consistent with template plots
|
|
||
| if (addPlotsButton) { |
There was a problem hiding this comment.
There was never a addPlotsButton.
|
|
||
| if (menu) { |
There was a problem hiding this comment.
We never passed a menu prop
There was a problem hiding this comment.
Was only used by the list filter here
| @@ -36,9 +36,6 @@ export const customPlotsSlice = createSlice({ | |||
| initialState: customPlotsInitialState, | |||
| name: 'custom', | |||
| reducers: { | |||
| changeDisabledDragIds: (state, action: PayloadAction<string[]>) => { | |||
| state.disabledDragPlotIds = action.payload | |||
| }, | |||
There was a problem hiding this comment.
Not necessary anymore as it was used to disable the drag and drop when using the smooth plot slider.
| @@ -37,9 +37,6 @@ export const templatePlotsSlice = createSlice({ | |||
| initialState: templatePlotsInitialState, | |||
| name: 'template', | |||
| reducers: { | |||
| changeDisabledDragIds: (state, action: PayloadAction<string[]>) => { | |||
There was a problem hiding this comment.
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| parentDraggedOver?: boolean | ||
| } | ||
|
|
||
| export const Grid: React.FC<GridProps> = ({ |
There was a problem hiding this comment.
Similar blocks of code found in 4 locations. Consider refactoring.
| : undefined | ||
| } | ||
| parentDraggedOver={parentDraggedOver} | ||
| disabledDropIds={[]} |
There was a problem hiding this comment.
Since disabledDropIds is optional, why not remove it entirely?
There was a problem hiding this comment.
It's used by the comparison table
There was a problem hiding this comment.
Apologies, I meant remove this line like this:
| disabledDropIds={[]} |
If I'm understanding things correctly, disabledDropIds defaults to an empty array anyway.
There was a problem hiding this comment.
You are right, sorry. I'll remove this.
| > | ||
| <GripIcon className={styles.plotGripIcon} /> | ||
|
|
||
| <h2>{plot}</h2> |
There was a problem hiding this comment.
The custom plots id looks strange on the drag and drop table, especially if the metrics/params are lengthy:
If I'm remembering correctly, the placeholder design isn't finalized but we may want to look into using a formatted title for custom plots like we show in custom plot quick picks:
There was a problem hiding this comment.
It's the first todo in the list (still need to get to it)
* Make toggling drag and drop mode seamless * Add tests * Fix pplot actions * Fix drag end event * Add style to plots in drag and drop mode (#4960) * Add style to drag and drop mode plots * Format custom plots titles in drag and drop mode * Self review * Add tests * Fix bug and style
| sectionKey: PlotsSection | ||
| } | ||
|
|
||
| export const DragAndDropGrid: React.FC<DragAndDropGridProps> = ({ |
There was a problem hiding this comment.
Similar blocks of code found in 5 locations. Consider refactoring.
| parentDraggedOver?: boolean | ||
| } | ||
|
|
||
| export const Grid: React.FC<GridProps> = ({ |
There was a problem hiding this comment.
Similar blocks of code found in 5 locations. Consider refactoring.
|
Code Climate has analyzed commit 92d0eca and detected 7 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 98.2% (85% is the threshold). This pull request will bring the total coverage in the repository to 95.2% (0.1% change). View more on Code Climate. |
1/3 main <- this <- #4942 <- #4960
Part of #4867
Demo
Screen.Recording.2023-10-31.at.3.21.51.PM.mov
I'll iterate on this PR before merging it. This is big enough as is without adding more functionality. To be added in future PRs merging back into this one: