-
Notifications
You must be signed in to change notification settings - Fork 53
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
Make UI vscode styled and refactor webview js #730
Make UI vscode styled and refactor webview js #730
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.
Minor formatting thing: I looked through the repo and determined this set of Prettier settings is the most consistent with the existing code. I'm considering doing a formatting job, so for consistency, these are the formatting settings you should use.
{
"singleQuote": true,
"endOfLine": "crlf",
"trailingComma": "es5"
}
Can you explain more what you are trying to do with "figure out how to make the deps bundled"? |
node_modules don’t exist in the built vsix, so the way I’m importing vscode-elements currently (uri from there) won’t work. I think I can get webpack to load the dependencies I need using raw-loader and inline as a script element. I’m open to other ideas, though! |
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.
Looks good to me
Needs conflicts resolved. |
Done! |
Build is failing |
innerHTML
to create the content and created eventListeners in another function based on DOM attributes. This refactors to create throughdocument.createElement
, giving access to the elements on creation so eventListeners can be created there (and version info captured). This also means that a malicious (or just broken?) vendordep can't inject any html into the sidebar and break itTODO: figure out how to make the deps bundled (probably requires bundling main.js too?)done