notebook: reload backLayerWebView when kernels change #122965
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reloads the webview when a kernel with a preload changes. There were two larger reworks in here: - The existing reload handler didn't work, since it didn't wait for the initialization message and would send render requests before the listener was ready. Instead, always run the reload "restore" when initialization happens. This means we also don't need to wait for the "loaded" promise, which is a good thing since that would have been a false indicator around reloads. - Cache more data around markdown cells so they can be restored after reload. Namely, their complete content rather than just a hash. I assume this was avoided for memory usage -- if it's a concern, one alternative would be to keep a reference to the ICellViewModel so that `getText` can be called on-demand. Let me know your thoughts. Fixes #120747
mjbvz
approved these changes
May 5, 2021
mjbvz
reviewed
May 5, 2021
| this.markdownPreviewMapping.clear(); | ||
| for (const cell of mdCells) { | ||
| if (cell.visible) { | ||
| this.createMarkdownPreview(cell); |
There was a problem hiding this comment.
Can you use initializeMarkdownPreview instead? This will create the cells one a time, which is more likely to cause flickering
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reloads the webview when a kernel with a preload changes. There
were two larger reworks in here:
The existing reload handler didn't work, since it didn't wait for
the initialization message and would send render requests before the
listener was ready.
Instead, always run the reload "restore" when initialization happens.
This means we also don't need to wait for the "loaded" promise, which
is a good thing since that would have been a false indicator around
reloads.
Cache more data around markdown cells so they can be restored after
reload. Namely, their complete content rather than just a hash.
I assume this was avoided for memory usage -- if it's a concern, one
alternative would be to keep a reference to the ICellViewModel so
that
getTextcan be called on-demand. Let me know your thoughts.I would appreciate 馃憖 in general here since I am less
familiar with the markdown code.
Fixes #120747