-
-
Notifications
You must be signed in to change notification settings - Fork 46
Conversation
cc @davidbrochart the build seems to be passing on CI |
I tried
|
Strange. This seems to be building fine locally and also on Binder, which installs from source as follows: Lines 1 to 8 in da09750
|
There is now a So we can move forward with this PR and start targeting JupyterLab 4 on |
Still left to do: show the notebook toolbar items, which might be related to this upstream PR: jupyterlab/jupyterlab#10469 |
I was trying to set the option to not close terminals on exit, but adding |
@davidbrochart I wonder if this should be set here instead:
iirc the other plugin handles opening in a new browser tab. |
This one looks like a race condition. Adding an artificial delay such as following seems lets the toolbar be correctly populated: diff --git a/packages/application-extension/src/index.ts b/packages/application-extension/src/index.ts
index 109fd19..866570b 100644
--- a/packages/application-extension/src/index.ts
+++ b/packages/application-extension/src/index.ts
@@ -181,9 +181,11 @@ const opener: JupyterFrontEndPlugin<void> = {
app.restored.then(() => {
// TODO: get factory from file type instead?
if (ext === '.ipynb') {
- docManager.open(file, NOTEBOOK_FACTORY, undefined, {
- ref: '_noref'
- });
+ setTimeout(() => {
+ docManager.open(file, NOTEBOOK_FACTORY, undefined, {
+ ref: '_noref'
+ });
+ }, 1000);
} else {
docManager.open(file, EDITOR_FACTORY, undefined, {
ref: '_noref' toolbar-notebook-delay.mp4 |
Thanks, that was it! |
Nice thanks @davidbrochart! |
cc @fcollonval who might know if this is related to the logic in jupyterlab/jupyterlab#10469 |
Using the following trick like in the await settingRegistry?.load('@jupyterlab/notebook-extension:panel');
await Promise.resolve(); |
// TODO: fix upstream? | ||
await settingRegistry?.load('@jupyterlab/notebook-extension:panel'); | ||
await Promise.resolve(); |
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.
We should investigate if it is possible to fix this upstream as this is very brittle.
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.
A toolbar definition may indeed not be fully known by the time the open is triggered as some plugins may not yet be loaded or the treatment of the settings may not be complete yet. Hence adding some sort of delay does the trick.
Merging that one since CI is now green. Will make a |
|
Fixes #303
Fixes #299
To start checking for breaking changes, and the new packages / plugins to add to the build.
@jupyterlab
packages tonext
@lumino
packages tolatest
Keeping as a draft for now.