Skip to content

Defer loading files until after layout restore #4087

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

Merged
merged 10 commits into from
Mar 9, 2018

Conversation

blink1073
Copy link
Contributor

@blink1073 blink1073 commented Mar 1, 2018

Fixes #3572.

Summary of changes:

API:

  • Adds a new initialize() function to Context (but not IContext) that is used instead of the previous save() and fromStore() (which was only on Context).
  • Adds an optional parameter to the document manager for when to load contents.

Behavioral:

  • We no longer wait for the kernel to be ready before showing the file
  • We no longer wait for the widget to be ready before returning from the docmananger:open command

@blink1073 blink1073 added this to the Beta 2 milestone Mar 1, 2018
*/
fromStore(): Promise<void> {
initialize(isNew: boolean): Promise<void> {
if (isNew) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just verified that this change still works with the modelDB in the Google Realtime stuff. I know that's going away soon, but wanted to make sure that we don't do it accidentally.

@blink1073 blink1073 force-pushed the simple-context branch 2 times, most recently from bae9ccd to c16ca7d Compare March 7, 2018 20:26
@@ -496,13 +425,109 @@ class Context<T extends DocumentRegistry.IModel> implements DocumentRegistry.ICo
name,
language: this._model.defaultKernelLanguage,
};
return this.session.initialize();
this.session.initialize();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we shouldn't even wait on a checkpoint to resolve the ready promise. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me, updated.

Untitled.ipynb Outdated
@@ -0,0 +1,44 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you intend to add this file?

@ian-r-rose
Copy link
Member

Thanks! This is a great improvement.

@ian-r-rose ian-r-rose merged commit 7c4f9e1 into jupyterlab:master Mar 9, 2018
@lock lock bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Aug 9, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pkg:docmanager pkg:docregistry status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Defer loading file contents until after App is restored
2 participants