-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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 the latest Lumino #16804
Update to the latest Lumino #16804
Conversation
Thanks for making a pull request to jupyterlab! |
What do you think about releasing another beta before merging this one? I know it can be annoying as it can cause merge conflicts, but my reasoning is that this will allow to narrow down any regressions by installing different versions? |
Yeah it would be great to have this Lumino update out soon so it can be tested with extensions and downstream in Notebook 7. But then that would mean releasing now, merging this PR, and making another right after (to not wait too long), which can also be tedious. |
When I reviewed jupyterlab/lumino#715 I noted that there is a regression with sidebar resizing behaviour, was this addressed? I do not see the changes mentioned in jupyterlab/lumino#715 (comment) in the diff but maybe it was addressed differently?
Ok, I will do that - I will take care of resolving conflicts here and making two releases. |
Ah maybe this is not covered by this PR then, would need to double check. The menu overflow issues were caught by the UI tests and should now be fixed though. |
Just checked from this PR and there does seem be an issue when resizing the sidebar: jupyterlab-lumino-sidebar-resize-issue.webm |
b509b0d
to
4a6c176
Compare
@jtpio rebased to resolve conflicts after release - do you want to take it from here to fix the sidebar styling? |
Thanks @krassowski for helping with the rebase and the conflicts! I just pushed a commit to include the suggestions from jupyterlab/lumino#715 (comment), which seems to be fixing the sidebar issues. Let's see if CI still passes. |
Should we also add an entry about this in the extension migration guide, ideally with instructions how to restore old behaviour in extensions by adding |
Thanks for the suggestion 👍 Added in 39fd022. |
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, just tiny suggestion on docs - I think we can continue testing this as beta3 :)
Co-authored-by: Michał Krassowski <[email protected]>
Thanks @krassowski for the review 👍 Addressed the last point in 80304be. Waiting for CI to complete to get this one in. |
References
Extract the Lumino update from #16794 to another PR, to avoid noise in #16794 and fix potential issues with this update not necessarily related to #16794.
Closes #16674
Code changes
@lumino
packages_plugins
in the UI testsoverflow: hidden
CSS rule for menusUser-facing changes
None?
Backwards-incompatible changes
None