Drag and drop feedback for comparison table #1551
Conversation
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| ) { | ||
| return singleUseListener.dispose() | ||
| } | ||
| const singleUseListener: Disposable = window.onDidChangeActiveTextEditor( |
There was a problem hiding this comment.
Just typed singleUseListener because the linter wasn't happy.
There was a problem hiding this comment.
This is also the reason for the PR coverage not being 100%
Was not easy to figure what was happening, but I see it now. There are 4 options on drop (drop on next, drop on previous, drop on next placeholder and drop on previous placeholder) and they should all result in different order change. Weirdly enough I cared about one case one way and another on the other. I'll fix and add tests. |
…com/iterative/vscode-dvc into comparison-table-drag-drop-feedback
|
Code Climate has analyzed commit 5e44d84 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 95.9% (85% is the threshold). This pull request will bring the total coverage in the repository to 96.7% (0.0% change). View more on Code Climate. |
| setTimeout(() => setSectionOrder([newSection, ...updatedSections]), 0) | ||
| } | ||
| return | ||
| } | ||
| if (lastSection.group !== group) { | ||
| setSectionOrder([...updatedSections, newSection]) | ||
| setTimeout(() => setSectionOrder([...updatedSections, newSection]), 0) |
There was a problem hiding this comment.
What happens if these aren't wrapped in setTimeout? Some sort of race condition?
There was a problem hiding this comment.
This section of the code is called onDrop and the drop event happens before the dragend event. onDragEnd is where the cleanup happens. If we call setSectionOrder before, the item is in a new section and has no access to the state of its previous section. We also cannot defer the whole function because group and draggedId would be cleared before being used.
|
[N] Drop highlight shows over the pinned column (final behaviour is expected) Screen.Recording.2022-04-14.at.8.22.21.am.mov |
Thanks will add to my list and fix soon |
Part of #1451
Screen.Recording.2022-04-11.at.11.07.10.AM.mov