Fix drag and dropping of editors for webviews #82813
Conversation
There was a problem hiding this comment.
@mjbvz even though maybe you need to use hacks, the result is sweet 👍
My only concern with the approach is how you depend on editor interna to get the drop target to show. Instead, I would make a method on IEditorGroupsService to draw this target if a component needs it. Since this is an internal workbench service anyways, I see no harm in having it.
mjbvz
force-pushed
the
webview-drag-drop
branch
from
c6dc3cc
to
1602a94
Compare
|
@bpasero Thanks for the feedback and good suggestion. I had to add the new method on the |
| @@ -1046,6 +1046,10 @@ export class EditorPart extends Part implements IEditorGroupsService, IEditorGro | |||
| type: Parts.EDITOR_PART | |||
| }; | |||
| } | |||
|
|
|||
| createDropTargets(container: HTMLElement, delegate: EditorDropTargetDelegate): IDisposable { | |||
There was a problem hiding this comment.
@mjbvz I use folding in that file and would prefer if this new method could move up below private assertGroupView
| @@ -1046,6 +1046,10 @@ export class EditorPart extends Part implements IEditorGroupsService, IEditorGro | |||
| type: Parts.EDITOR_PART | |||
| }; | |||
| } | |||
|
|
|||
| createDropTargets(container: HTMLElement, delegate: EditorDropTargetDelegate): IDisposable { | |||
There was a problem hiding this comment.
@mjbvz maybe a better name would be createEditorDropTarget
| this._webviewVisibleDisposables.clear(); | ||
|
|
||
| // Webviews are not part of the normal editor dom, so we have to register our own drag and drop handler on them. | ||
| this._webviewVisibleDisposables.add((this._editorGroupsService as EditorPart).createDropTargets(input.webview.container, { |
There was a problem hiding this comment.
@mjbvz I think one step further is to make this new method available from IEditorGroupsService to prevent this ugly cast.
There was a problem hiding this comment.
Not sure if this is possible. IEditorGroupsService is in common so it can't reference HTMLElement. I've replaced the cast with an instanceof check which should be safer
Fixes microsoft#25854 This change does the following: - Have the webview editor create an `EditorDropTarget`. This is required because the webview itself is not part of the normal editor dom (it exists as a top level node in the workbench so that we never reparent it). This means that the standard `EditorDropTarget` does not work. - Make webviews set `pointer-events: none` while a drag is happening. When a drag happens on Electron's webviews or a normal iframe, no drag and drop events seem to get generated. The fix is to set `pointer-events: none` while the drag is happening. But it's also difficult to detect when the drag is happening because the `EditorDropTargets` class eats the drop event itself. The only reliable seeming way I could find to determine when a drag starts and ends is looking at global events on the window. This workaround is pretty ugly. I'm not sure if there's some better approach that would work with webviews
Unfortunatly we can't add this method to IEditorGroupsService as it uses the `HTMLElement` type
mjbvz
force-pushed
the
webview-drag-drop
branch
from
1602a94
to
bc544a1
Compare
Fixes #25854
This change does the following:
Have the webview editor create an
EditorDropTarget. This is required because the webview itself is not part of the normal editor dom (it exists as a top level node in the workbench so that we never reparent it). This means that the standardEditorDropTargetdoes not work.Make webviews set
pointer-events: nonewhile a drag is happening. When a drag happens on Electron's webviews or a normal iframe, no drag and drop events seem to get generated. The fix is to setpointer-events: nonewhile the drag is happening. But it's also difficult to detect when the drag is happening because theEditorDropTargetsclass eats the drop event itself. The only reliable seeming way I could find to determine when a drag starts and ends is looking at global events on the window.This workaround is pretty ugly. Unfortunately I haven't been able to find a better way to do this