-
-
Notifications
You must be signed in to change notification settings - Fork 126
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
Remove the overflow rule on widgets #715
Remove the overflow rule on widgets #715
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.
Applying this leads to a couple of subtle visual regressions in JupyterLab (these are just things I noticed immediately, there are probably more)
Before | After |
---|---|
Resizing the sidebar panels with accordion gets jumpy:
Would it still work if we made the change here but restored this rule in JupyterLab codebase?
Thanks for the review @krassowski
Do you mean restoring it where we see changes or overall? |
I was thinking about something more general like |
Unfortunately that will indeed make this PR not useful. The main issue is when integrating a library that does not support mechanism like react portal to attach out of the flow elements outside of the local dom tree. |
That can be fixed by removing the following rule in JupyterLab: .jp-SidePanel {
/* overflow-y: auto; */
} I saw also it will require some changes in the filebrowser styling: .jp-FileBrowser .jp-SidePanel-content {
overflow: hidden;
}
.jp-FileBrowser-Panel {
overflow: hidden;
}
.jp-DirListing {
overflow: hidden;
} Do you think this would be acceptable? |
These would be good, but I guess we should also test with as many extensions as possible to see what else might be affected. What about more granular scoping |
👍
We could add something like that - it could indeed mitigate issues. |
if it is ok for you - I can merge and release lumino to open a PR on Lab |
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 with the caveat that this needs more testing downstream and if we find any breakages which cannot be fixed before 4.3 is out we would want to (temporarily) revert this (possibly on lab side).
I agree with you - and yes it will be better to revert it only on lab if required. |
This removes the
overflow: hidden
CSS rule on widget.The rationale behind this change is that
It breaks the ability to have sub items overflow for example one having custom list picker or context menus in cell outputs.