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

Support tree view drag and drop. #12065

Merged
merged 4 commits into from
Jan 20, 2023

Conversation

tsmaeder
Copy link
Contributor

Signed-off-by: Thomas Mäder t.s.maeder@gmail.com

What it does

Adds support for contributing drag and drop controller in the tree view plugin API. The code tries to mimick the behaviour described in https://code.visualstudio.com/api/references/vscode-api#TreeDragAndDropController<T> as well as possible. I have also refactored some of the "drag into editor area" code to work with the new code (and fixed dragging external files into electron on windows as a "drive-by")

Contributed on behalf of ST Microelectronics

Fixes #11778, Fixes #11776

How to test

I have adapted the VS Code tree view sample to exercise the code added in the PR. The extension & source is attached to this PR.
The extension contributes a view called "Test View Drag and Drop", which you need to open.

The example implement multiple features:

  1. Drag and drop within the tree
    The dropped element should be moved under the element where it was dropped.
    Note that there seems to be a bug in our tree-view refresh code where the UI goes into an endless loop when you reparent the elements of the tree: It seems the parent field of the tree items is not properly updated when an item is reparented. I am pretty convinced this is not related to the changes in this PR.
  2. Drag an element to the editor area:
    This should open a *.js file from the extension itself: note that the cursor will be placed at line 10, column 6.
  3. Exercise the current drag and drop to the editor area. Drag from the file navigator and from the OS file explorer
  4. Exercise dropping files to the example view: the extension will show the first couple of bytes of the file content.

Review checklist

Reminder for reviewers

@tsmaeder
Copy link
Contributor Author

Here's the source:

tree-view-sample.zip

and here's the built extension

custom-view-samples-0.0.1.zip

Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

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

Some initial thoughts on the code

@vince-fugnitto vince-fugnitto added tree issues related to the tree (ex: tree widget) vscode issues related to VSCode compatibility dnd issues related to drag & drop labels Jan 12, 2023
@tsmaeder tsmaeder force-pushed the 11778_tree_view_dnd branch from 6789039 to 9ad9347 Compare January 13, 2023 07:01
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

  1. Exercise the current drag and drop to the editor area. Drag from the file navigator and from the OS file explorer
  2. Exercise dropping files to the example view: the extension will show the first couple of bytes of the file content.

I noticed that these (i.e. dragging from the OS file explorer) only work on Electron. Is it out of scope to make these work in the browser version as well?

Also, when dropping a file into an already open editor, even though it does open the file, it also pastes the file path into the existing editor. Can we change that somehow?

packages/plugin-ext/src/plugin/tree/tree-views.ts Outdated Show resolved Hide resolved
packages/core/src/browser/shell/application-shell.ts Outdated Show resolved Hide resolved
@tsmaeder
Copy link
Contributor Author

tsmaeder commented Jan 18, 2023

  1. Exercise the current drag and drop to the editor area. Drag from the file navigator and from the OS file explorer
  2. Exercise dropping files to the example view: the extension will show the first couple of bytes of the file content.

I noticed that these (i.e. dragging from the OS file explorer) only work on Electron. Is it out of scope to make these work in the browser version as well?¨

No, the drop to the editor works with URI's, and we don't have the URI of the file being dropped. Fixing editor DnD is not in scope for this PR (beyond making it work with tree views)

Also, when dropping a file into an already open editor, even though it does open the file, it also pastes the file path into the existing editor. Can we change that somehow?

I believe this is pre-existing behavior and as such also out of scope for this PR.

…ipse-theia#11776

Adds support for contributing drag and drop controller in the tree view
plugin API

Contributed on behalf of STMicroelectronics

Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>
Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>
Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>
@tsmaeder tsmaeder force-pushed the 11778_tree_view_dnd branch from 89a1b45 to ede1983 Compare January 19, 2023 14:23
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

I'm happy with the changes, just a few very minor comment below.

  • Dragging a file from the file system explorer works and opens the file in the editor area (on Electron)
  • The extension behaves as indicated
  • The API is marked as supported

packages/plugin-ext/src/plugin/types-impl.ts Outdated Show resolved Hide resolved
packages/plugin-ext/src/plugin/types-impl.ts Outdated Show resolved Hide resolved
Co-authored-by: Mark Sujew <mark.sujew@typefox.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dnd issues related to drag & drop tree issues related to the tree (ex: tree widget) vscode issues related to VSCode compatibility
Projects
None yet
6 participants