Use hook for drag and drop #5062
Conversation
| renderAppWithOptionalDataInDragAndDropMode( | ||
| { | ||
| custom: customPlotsFixture, | ||
| template: templatePlotsFixture |
There was a problem hiding this comment.
[Q] From the test title it sounds like these plots are needed. Do the statements need to be updated?
There was a problem hiding this comment.
It was never testing the other part. I guess we need to update the statement.
| .includes(endingNode.id) | ||
| ).toBe(true) | ||
| within(headerWrapper).getByTestId('comparison-drop-target') | ||
| ).toBeInTheDocument() |
There was a problem hiding this comment.
[Q] How do these assertions differ/relate to one another?
There was a problem hiding this comment.
It's not wrapped the exact same way, but the test checks the same thing, it was just not written in a clear way before.
| onLayoutChange: () => void | ||
| setOrder: (order: string[]) => void | ||
| order: string[] | ||
| bodyRef?: React.RefObject<HTMLTableSectionElement> |
There was a problem hiding this comment.
[Q] Should this be RefObject? (I am asking because that is what I've been using)
There was a problem hiding this comment.
Copied and pasted without verifying. I'll fix it.
webview/src/test/dragDrop.ts
Outdated
|
|
||
| act(() => { | ||
| jest.advanceTimersByTime(1) | ||
| jest.advanceTimersByTime(501) |
There was a problem hiding this comment.
[Q] Why is this needed?
There was a problem hiding this comment.
It isn't. I added dragLeave in there to test something and the 500 was needed then. Turns out that wasn't how to solve the test and I forgot to revert this.
| | null | ||
|
|
||
| export type Any = BaseType | BaseType[] | ||
| type Any = BaseType | BaseType[] |
| pascalCase: true | ||
| }, | ||
| ignore: [/.*\.stories.tsx$/, /.*\.test\.tsx$/] | ||
| ignore: [/.*\.stories.tsx$/, /.*\.test\.tsx$/, /.*use.*\.tsx$/] |
There was a problem hiding this comment.
What's the reasoning for adding this? I'm not seeing any new files with this syntax 🤔
There was a problem hiding this comment.
useDragAndDrop.tsx there is some jsx in that file
![qlty-cloud-legacy[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
|
|
||
| const { immediateDragLeave, immediateDragEnter, deferredDragLeave } = | ||
| useDeferredDragLeave() | ||
| const { |
There was a problem hiding this comment.
Similar blocks of code found in 5 locations. Consider refactoring.
| } | ||
| } | ||
|
|
||
| export const useDragAndDrop = ({ |
There was a problem hiding this comment.
Function useDragAndDrop has a Cognitive Complexity of 36 (exceeds 6 allowed). Consider refactoring.
|
|
||
| export const { changeRef, setGroup, setDraggedOverGroup } = | ||
| dragDropSlice.actions | ||
| export const { |
There was a problem hiding this comment.
Similar blocks of code found in 2 locations. Consider refactoring.
|
Code Climate has analyzed commit 7ce4da3 and detected 4 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 96.6% (85% is the threshold). This pull request will bring the total coverage in the repository to 95.2% (0.0% change). View more on Code Climate. |
3 participants
Part of #4867
Demo
Screen.Recording.2023-12-12.at.10.08.36.PM.mov
Things should be as before, but faster