Skip to content
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

Fix context menu position on Voila, NbClassic and Notebook<7 #509

Merged

Conversation

ianthomas23
Copy link
Contributor

@ianthomas23 ianthomas23 commented Apr 24, 2024

Fixes issue #422.

The positioning of context menus using Voila, NbClassic and Notebook<7 is incorrect as they are added to the DOM as the last child of the body and so the reference element that they are positioned with respect to is not necessarily correct, although it is fine for Jupyter Lab (the most tested option). The solution is make them the first child of the body instead.

Longer term there will be an addition to lumino so that menus can be attached to the correct node straight away rather than this approach of having to detach and reattach them. This code can remain then as the menu will already be the body's first child at the point of testing so the detach and reattach will be avoided.

Screenshots of this PR working in the presence of scrolling for lab, voila, notebook 6, notebook 7 and nbclassic respectively:

lab voila notebook6 notebook7 nbclassic

Copy link
Member

@martinRenou martinRenou left a comment

Choose a reason for hiding this comment

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

🚀 Thanks a lot for this!

@martinRenou
Copy link
Member

Fix #422 and fix voila-dashboards/voila#930

@martinRenou martinRenou force-pushed the 422_context_menu_position branch from 69fbb3c to 93667f2 Compare April 25, 2024 07:15
Copy link
Member

@martinRenou martinRenou left a comment

Choose a reason for hiding this comment

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

I just rebased, let's merge when green!

@martinRenou martinRenou merged commit a939121 into jupyter-widgets:main Apr 25, 2024
15 checks passed
@ianthomas23 ianthomas23 deleted the 422_context_menu_position branch April 25, 2024 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants