Conversation
This will move all webviews into a single Webpack bundle. This will make it easier to add new webviews since we don't need to add a new bundle, but just need to add a new directory with an `index.tsx` file. It also moves the CSS processing to Webpack so that we don't need to specify the CSS files to use separately, but can simply do so in the TypeScript files.
aeisenberg
left a comment
aeisenberg
left a comment
There was a problem hiding this comment.
Thanks for taking this on! Looks really good. I have a few questions, but nothing really blocking. I haven't downloaded this and tried it out, which I plan on doing.
| @@ -31,9 +30,7 @@ export const config: webpack.Configuration = { | |||
| { | |||
| test: /\.less$/, | |||
There was a problem hiding this comment.
Minor:
Hmmm...looks like we have a rule for less files, but we're not using any less. This has been in the file since the very beginning. If we're not using less now and have no plans to, perhaps we should remove it. And in the future, use sass if we want.
Not something you need to do for this PR. Just a thought and something to discuss for later.
| Uri.file(ctx.asAbsolutePath('out/webview.css')) | ||
| ]; | ||
|
|
||
| if (includeCodicons) { |
There was a problem hiding this comment.
What is the danger of always including codicons? It would be nice to slowly move our other webviews to use the same UI styles as the MRVA webview.
There was a problem hiding this comment.
As you have noticed in #1478, I think we should always include the codicons and will do it in that PR. However, to include the codicons the path should also be included in the localResourceRoots, so that would require duplicating this line to the other two interface managers. I didn't really think that would be worth it, especially since that part is going to be replaced anyway in #1478.
| allowInlineStyles?: boolean; | ||
| includeCodicons?: boolean; | ||
| } = { | ||
| allowInlineStyles: false, |
There was a problem hiding this comment.
I believe this is only true for the mrva page so that it can use inline styles while loading react components from the primer UI toolkit library. Since we are moving away from primer and using the vscode UI toolkit, is this still necessary?
Again, not related to this PR, but perhaps something to check later.
2 participants
This will move all webviews into a single Webpack bundle. This will make it easier to add new webviews since we don't need to add a new bundle, but just need to add a new directory with an
index.tsxfile.It also moves the CSS processing to Webpack so that we don't need to specify the CSS files to use separately, but can simply do so in the TypeScript files.
The next step will be abstracting/simplifying the interface classes which create the panel.
Checklist
ready-for-doc-reviewlabel there.