-
Notifications
You must be signed in to change notification settings - Fork 511
Fix CustomViews by switching to WebViews #1919
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
Fix CustomViews by switching to WebViews #1919
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.
Don't know much about this, but the code looks good
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.
LGTM.
cae4fb4
to
0185ff6
Compare
Ok rewire is back but with it, comes 3 tests! |
Implementation has changed a bit - re-requested reviews |
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.
LGTM. Simpler. cleaner and now with tests!
// Setup types that are not exported. | ||
const customViews = rewire("../../src/features/CustomViews"); | ||
const htmlContentViewClass = customViews.__get__("HtmlContentView"); | ||
const HtmlContentView: typeof htmlContentViewClass = htmlContentViewClass; |
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.
Wow, this is cool!
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.
It's like reflection but much more convoluted lol
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.
LGTM ❤️
PR Summary
I'm going to try to see if I can add a test or two... but this fixes the *-VSCodeHtmlContentView cmdlets. You are still able to pass in stylesheets and javascript and it will inline them.
Then, in order to refresh your view to get any js or css updates, just run the
Show-VSCodeHtmlContentView
cmdlet again and your changes will be reflected.This also enables the persistent pages so if you click away from a view and come back, the state remains!
P.S. there seems to be some weird behavior when you close out of a view - we must not be cleaning up properly. This bug existed before this change... I might try to tackle it now.
PR Checklist
Note: Tick the boxes below that apply to this pull request by putting an
x
between the square brackets.Please mark anything not applicable to this PR
NA
.WIP:
to the beginning of the title and remove the prefix when the PR is ready