-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
monaco preview specification #10136
monaco preview specification #10136
Conversation
We've been experimenting with WebView2 for Monaco in our Monaco Editor repo. It'd be great if we could just support a WPF version of that as well vs. having you re-implement all the work to host Monaco in a WebView? It's been on my list of things to investigate, I've just been pretty swamped with the Toolkit, but we should be trying to do more with it in April. We've also been waiting for WebView2 to have better support for AddHostObjectInScript from the WinUI 3 side. I don't know if that works in WPF yet or not? |
It's WPF using a liabary with WebView2 |
Great job Aaron on this and way to drive clarity. My feedback is all around driving clarity |
added a bit more information up front. Added in new requirement at the bottom that i think we should discuss.
doc/specs/preview-monaco.md
Outdated
|
||
### 1.1.2 Why WebView2 | ||
|
||
Experiments with a POC showed that only WebView2 is displaying Monaco the right way. WebView and WebBrowser elements are using too old versions of IE/Edge who don't support JS functions like `require()`, which are needed for monaco to work. The other option would be to import many JS liabarys, but they don't guarantee 100% coverage with JS and they take more space. |
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.
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.
my theory is this was added in well before Monaco has this dependencies and why this is like it is
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.
just saw your question: Yes, we added monaco to WTS way before it had this dependency. We're using WPF and WebBrowser.
I think one item we need to figure out in the POC is what happens when someone doesn't have the runtime installed and safely fall back to a "heyyy, go install the webview". I would say for the installer we'd leverage the bootstrapper installer but that needs to be online. |
how we test is on VM and self-contained .net application. |
@Aaron-Junker add this bit in about when the webview2 runtime isn't actually installed and lets call it good |
@Aaron-Junker any eta? what can i do to help wrap this up |
@crutkas Do you mean the specification or the project? So the spec can get merged. You're the one who wanted to wait |
|Name|Description|Priority| | ||
|----|-----------|--------| | ||
| Working Preview pane | It's simply working. | P0 | | ||
| Settings can install filetypes | When the user activates MonacoPreview in settings it registers the preview handlers. | P0 | | ||
| Style code | Monaco recognizes the file extensions and highlights the syntax the right way. | P0 | | ||
| User can choose file previews | Users can attach custom filetypes to preview. For example `.phptest` files to the `.php` handler. | P2 | | ||
| OOBE | Description for the OOBE. | P1 | | ||
| On/Off in settings | The user can turn it on and off. | P0 | | ||
| Settings: selection for file extensions | User can choose in the settings which File extensions should get registered. | P1 | | ||
| Preview pane Handler registration logic in app, not installer | Logic is migrated out so we deduplicate logic now we're adding 125 files. The uninstaller needs to be run a script of some sort to remove the registration. | P1 | |
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.
- Let's order table by priority from P0 as first to P2 as last.
- Let's move Priority in the first line.
- Let's add the item "Docs - Adding feature to docs. - P1".
Summary of the Pull Request
What is this about:
Adds the spec for the monaco preview pane
What is include in the PR:
The spec file
How does someone test / validate:
Read through and check for mistakes
Quality Checklist
Contributor License Agreement (CLA)
A CLA must be signed. If not, go over here and sign the CLA.