-
Notifications
You must be signed in to change notification settings - Fork 28.8k
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
Move off in Webview #206363
Move off in Webview #206363
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking into this! Since the WebviewElement
is mounted into the Dom, I wonder if we can use this to get the code window in some cases instead of passing it in
That still leaves the call to parentOriginHash(this.codeWindow.origin, ...)
(and maybe others I'm overlooking)
Does the origin change for detached editors? If not, maybe we can just the origin of the main window in this case?
src/vs/workbench/contrib/customEditor/browser/customEditorInputFactory.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool stuff, one thing that worries me is the serialise/deserialise for webviews, I have asked some questions in the code.
src/vs/workbench/contrib/welcomeGettingStarted/browser/gettingStarted.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/welcomeGettingStarted/browser/gettingStarted.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/webviewPanel/browser/webviewEditorInputSerializer.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/customEditor/browser/customEditorInputFactory.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed some changes, I will make sure that this.group
is defined early on, so that we can continue to use it for getting the window.
src/vs/workbench/contrib/customEditor/browser/customEditorInput.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
I've been working on refactoring the webview related code by dropping all $window references. This task turned out to be quite complex, as it involves a deep understanding of the system to decide how to correctly associate the code windows.
Here are the steps I've taken so far:
codeWindow
when initializing webviews. This is because the webview needs to be aware of thecodeWindow
from the start, as it listens to messages and events from bothwindow
anddocument
refWebviewElement
to useCodeWindow
. refNext steps are revising the use of webviews across the workbench. For areas where the
codeWindow
is clearly identified, like in editors (where we havegroup.windowId
), it's straightforward to proceed:CodeWindow
Editor
so I can reading fromgroup.windowId
CodeWindow
containing the editor.For cases where the code window isn't explicitly known, especially for requests initiated from the extension host, I used
getActiveWindow
, but I'm uncertain about its reliability:@bpasero @mjbvz