Fixes #182449 : Pressing Shift re-enables webview during Drag and Drop Events #209211
Conversation
| this._register(DOM.addDisposableListener(targetWindow, DOM.EventType.DRAG, (event) => { | ||
| if (event.shiftKey) { | ||
| onDragEnd(); | ||
| } |
There was a problem hiding this comment.
we could change this to re-disable the webview when the user releases shift.
if (event.shiftKey) {
onDragEnd();
} else {
onDragStart();
}
There was a problem hiding this comment.
Yes I think that makes sense. Otherwise you have to restart dragging if you want to open the dragged resource in an editor instead of dropping it into the webview
There was a problem hiding this comment.
@mjbvz I've made some updates to address the feature that was requested. I noticed you tagged this with the April 2024 milestone and I would love to address any change requests/concerns prior to that deadline.
And thank you for your hard work!
|
@microsoft-github-policy-service agree |
|
Unfortunately, while working on this I learned that after pressing shift the events are captured by the iframe so, releasing shift was unable to be captured by the WebViewDragMonitor. So, I added a new custom event listener that allows events to bubble up from the iframe (I copied the logic from the key events) to enable the requested functionality. I ran into some issues because I used To discover and resolve this issue I stepped through the git history and reverted each change 1 by 1 to figure out when the build started failing. As a result the git history got very cluttered and I assume it would have been difficult to review. So I squashed all my commits into 2. Anyways, here's a video of these changes working with @maxbublik 's test extension. Recording.2024-04-06.114621.mp4 |
| @@ -17,6 +17,10 @@ type KeyEvent = { | |||
| repeat: boolean; | |||
| } | |||
|
|
|||
| type WebViewDragEvent = { | |||
There was a problem hiding this comment.
I tried to type this DragEvent instead of WebViewDragEvent but, the tests failed. I can only assume this is due to a collision with the standard DOM DragEvent type. I would have expected a typescript error though, this was hard to figure out.
updating html hash adding shift release listener to drag and drag over events adding new custom drag event handler to allow event bubbling adding new event types correcting comment: removing drag over listener from drag monitor since we are converting this to drag events making webview drag monitor stateful and adding missing events to iframe dom html file Revert "Merge remote-tracking branch 'microsoft/main' into swordensen" This reverts commit 47e3877, reversing changes made to 8f71927. Revert "making webview drag monitor stateful and adding missing events to iframe dom html file" This reverts commit 8f71927. Revert "removing drag over listener from drag monitor since we are converting this to drag events" This reverts commit bfae5f1. Revert "correcting comment:" This reverts commit 696facb. Revert "Merge branch 'main' into main" This reverts commit 78ed36c, reversing changes made to 7b96d0e. Revert "adding new event types" This reverts commit 7b96d0e. Revert "adding new custom drag event handler to allow event bubbling" This reverts commit 39971e0. Revert "adding shift release listener to drag and drag over events" This reverts commit 4f5d9e8. Revert "updating html hash" This reverts commit 6a474cb. Revert "adding drag and drag over listeners to webview iframe(s)" This reverts commit 6688ec6. adding harmless change to see if this causes smoketests and integration tests to fail adding sha hash Revert "adding sha hash" This reverts commit 830baa8. Revert "adding harmless change to see if this causes smoketests and integration tests to fail" This reverts commit ceb55f9. Revert "Revert "adding drag and drag over listeners to webview iframe(s)"" This reverts commit 1baf483. Revert "Revert "adding shift release listener to drag and drag over events"" This reverts commit 40bc29c. Revert "Revert "adding new custom drag event handler to allow event bubbling"" This reverts commit 44f4fda. Revert "Revert "adding new event types"" This reverts commit 67f1aaa. removing internal drag listener removing types. Maybe it's causing build issues? renaming drag event to WebviewDragEvent to avoid collision with native DragEvent type resolving conflict with correcting comment Revert "Revert "Merge branch 'main' into main"" This reverts commit 1a4d711. Revert "Revert "Merge branch 'main' into main"" This reverts commit 8520787. resolving conflict Revert "resolving conflict" This reverts commit c5b3f7f. Revert "Merge remote-tracking branch 'microsoft/main' into swordensen" somehow i got assigned these changes in the process of unreverting my merges fix: serialization of newline characters removing console log forgot the security hash when removing console log
|
mmmmmmmmmmm I don't think I wanted all of these changes. I was just trying to catch up my branch with the main one and resolve my conflict locally. Please give me some time to figure this out. |
|
@jrieken I am sorry I don't know how I requested your review. I think that happened when I was attempting to handle rebasing my local branch manually. I don't know what your guys' process is for reviewing PRs. I am not trying to be annoying! |
|
I have no idea how github assigned this PR to my resolve merge conflict branch instead of my main branch... |
|
and TIL how to request a review on github! |
…g and Drop Events (microsoft#209211) * added shift key listener to re-enable drag events for webview microsoft#182449 * adding drag and drag over listeners to webview iframe(s) updating html hash adding shift release listener to drag and drag over events adding new custom drag event handler to allow event bubbling adding new event types correcting comment: removing drag over listener from drag monitor since we are converting this to drag events making webview drag monitor stateful and adding missing events to iframe dom html file Revert "Merge remote-tracking branch 'microsoft/main' into swordensen" This reverts commit 47e3877, reversing changes made to 8f71927. Revert "making webview drag monitor stateful and adding missing events to iframe dom html file" This reverts commit 8f71927. Revert "removing drag over listener from drag monitor since we are converting this to drag events" This reverts commit bfae5f1. Revert "correcting comment:" This reverts commit 696facb. Revert "Merge branch 'main' into main" This reverts commit 78ed36c, reversing changes made to 7b96d0e. Revert "adding new event types" This reverts commit 7b96d0e. Revert "adding new custom drag event handler to allow event bubbling" This reverts commit 39971e0. Revert "adding shift release listener to drag and drag over events" This reverts commit 4f5d9e8. Revert "updating html hash" This reverts commit 6a474cb. Revert "adding drag and drag over listeners to webview iframe(s)" This reverts commit 6688ec6. adding harmless change to see if this causes smoketests and integration tests to fail adding sha hash Revert "adding sha hash" This reverts commit 830baa8. Revert "adding harmless change to see if this causes smoketests and integration tests to fail" This reverts commit ceb55f9. Revert "Revert "adding drag and drag over listeners to webview iframe(s)"" This reverts commit 1baf483. Revert "Revert "adding shift release listener to drag and drag over events"" This reverts commit 40bc29c. Revert "Revert "adding new custom drag event handler to allow event bubbling"" This reverts commit 44f4fda. Revert "Revert "adding new event types"" This reverts commit 67f1aaa. removing internal drag listener removing types. Maybe it's causing build issues? renaming drag event to WebviewDragEvent to avoid collision with native DragEvent type resolving conflict with correcting comment Revert "Revert "Merge branch 'main' into main"" This reverts commit 1a4d711. Revert "Revert "Merge branch 'main' into main"" This reverts commit 8520787. resolving conflict Revert "resolving conflict" This reverts commit c5b3f7f. Revert "Merge remote-tracking branch 'microsoft/main' into swordensen" somehow i got assigned these changes in the process of unreverting my merges fix: serialization of newline characters removing console log forgot the security hash when removing console log * updating sha
| @@ -310,6 +310,10 @@ export class WebviewElement extends Disposable implements IWebview, WebviewFindD | |||
| this._startBlockingIframeDragEvents(); | |||
| })); | |||
|
|
|||
| this._register(this.on('drag', (event) => { | |||
| this.handleDragEvent('drag', event); | |||
There was a problem hiding this comment.
we likely need to prevent default after we pass this to our custom drag event listener
|
This operation will affect dragging within the webview |
This is primarily to resolve #182449
Basically, this adds a new disposable listener to the entire drag event and if the shiftkey is pressed at anytime pointer-events are re-enabled for the Webview Window so you can drop files to the webview.