-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Update to JupyterLab 4.3.0b1 #7423
base: main
Are you sure you want to change the base?
Conversation
bot please update playwright snapshots |
Looks like the following error might be relevant: Also need to check the effects of jupyterlab/jupyterlab#16446 on Notebook. |
Just noticed an upstream change that might be relevant: jupyterlab/jupyterlab#16523 cc @krassowski who may know more about this. Wondering if this is because we had to fetch the docmanager settings manually at some point due to some race conditions: notebook/packages/application-extension/src/index.ts Lines 223 to 246 in 43b8cce
|
Nothing jumps at me here, but some ideas:
|
The error seems to be coming from here in JupyterLab: |
Also seeing this on #7447. I do not think this is 4.3.x specific issue |
The error is coming from here:
In fact all settings accessed in |
And that code is referenced in Notebook codebase here: notebook/packages/application-extension/src/index.ts Lines 223 to 232 in 43b8cce
Maybe there is no race condition but the settings for docmanager plugin are only loaded but not transformed yet. I guess that the issue in JupyterLab is masked by the fact that |
I see the same error on released Jupyter Notebook 7.2.0 using https://gist.github.com/jtpio/befb8ebf2b630b4748e2538f79877022 I now think this should not be a blocker for merging this one as there is no regression in JupyterLab 4.3 nor 4.2.x as this was just overlooked previously. |
Thanks @krassowski for looking into this 👍 I guess we could just ignore this for now then. Spinning back a dev install for this branch, it looks like there is still an issue with the background color though: |
Does |
Done in 40387bb. Although not seeing any change for now: |
Is there a way to reduce the size of the "File size" column? I guess it's related to jupyterlab/jupyterlab#16646 ? |
It looks like
|
packages/application/style/base.css
Outdated
body { | ||
margin: 0; | ||
padding: 0; | ||
} | ||
|
||
/* | ||
Override the default background | ||
See https://github.com/jupyterlab/jupyterlab/pull/16519 for more information | ||
*/ | ||
.jp-ThemedContainer { |
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.
I think this is because of this change. I think the right fix is to just adjust the selector to body.jp-ThemedContainer
:
body { | |
margin: 0; | |
padding: 0; | |
} | |
/* | |
Override the default background | |
See https://github.com/jupyterlab/jupyterlab/pull/16519 for more information | |
*/ | |
.jp-ThemedContainer { | |
body.jp-ThemedContainer { | |
margin: 0; | |
padding: 0; |
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.
This will also make embedding notebook in other websites easier (when using a full javascript rather than iframe) as the styles will not conflict :)
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.
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 like #main.jp-ThemedContainer
also had to be defined.
I guess it's because Notebook 7 does not provide support for workspaces and restoring the file browser? Maybe we should hardcode some default values that match the previous defaults for now? |
@krassowski any suggestion for setting proper defaults for the file browser column sizes? I tried something like the following but it didn't seem to have any effect: stateDB.save('file-browser-filebrowser:columns', {
sizes: {
name: 258.125,
file_size: 60,
is_selected: 18,
last_modified: 778.899513036933,
},
})
.then(async () => {
await browser['listing'].restore('filebrowser');
}); |
Or maybe this will be made easier with jupyterlab/jupyterlab#16751, so we can just specify custom CSS directly instead. |
Update to the latest pre-releases:
Part of #7441