Closed huyuhuster closed 7 months ago
+1 for this issue when using ipydatagrid and Voila, only I do need the context popup and would like to control its position.
The misplaced context menu (but not the ability to show/hide it) is fixed by the change proposed here: https://github.com/voila-dashboards/voila/issues/930
@chuckmandu the fix proposed in the issue you mention was applied almost 2 years ago in this commit, yet the issue persists (or perhaps was fixed and has resurfaced).
I don't believe the original committer @ibdafna is still active in this repository, hopefully one of the current maintainers will pick this issue up (the bug fix, that is, not the feature request to hide the menu).
I'll try to have a look
I had a poke around the code and the browser element inspector, because I have this issue too and would very much like to not have it, and there are some ipywidget challenges that can be solved with clever injection of CSS style rules into empty HTML widgets. No such luck here I don't think, but I think I do see what the problem is.
The first time this bug emerged, the problem was that window positioning wasn't being correctly accounted for. This time, however, the problem is that at least in the voila template I'm using, the context menu is added as a div at the end of the page, and because positioning is absolute, that is therefore measured relative to the end of the page's content.
The Y coordinate being computed (event.clientY+window.scrollY
) is correct when measured from the top of the page. But if we're measuring from the bottom of the page, then we need to subtract the length of the page and get a negative coordinate. You can verify this in the browser element inspector by setting the Y component of the menu's translate
property to the appropriate negative number.
I don't know if this is true for all voila templates. If previously the context menu was added as a div at the top of the page code instead of at the bottom, though, then that would explain why it worked previously, with coordinates measured from top left.
I haven't set up a development version of the repo yet with which to play around with the JavaScript (I've just been using a pip-installed version), but I can try to test this hypothesis and work towards a PR if nobody else has time.
This seems very likely to be a consequence of the Lumino changes proposed in https://github.com/jupyterlab/lumino/issues/329#issuecomment-1266200387. The problem is that when absolute positioning is used, the y-coordinate for the menu needs to be negative, since the menu is now appended to the document as its own div, instead of added to document.body as before, so the origin is at the bottom of the document in absolute positioning. But Lumino forces the Menu object's y-coordinate to be >=0. The solution (as tested in my local dev installation) appears to be instantiating the menu in gridContextMenu.ts, and then changing the 'transform' member of its style property to a translate
operation that has the correct coordinates. Possible there are slicker workarounds, but since this isn't necessarily a bug with Lumino as much as a consequence of a new feature, I think a workaround is definitely what's needed, rather than a bug report over at Lumino.
I'll create a fork and submit a PR.
Would it possible to re-open this issue? The problem still persists following it being reverted in this PR: https://github.com/bloomberg/ipydatagrid/pull/468
I have diagnosed the cause of the problem here. The context menu div
is created with absolute
positioning and an (x, y) translation with respect to the top left of the body
, taking into account scrolling, and then it is added to the DOM as the last child of the body
. The rules for CSS absolute
positioning are that the element is positioned relative to its closest positioned ancestor element, walking the DOM checking its previous siblings and parents.
This current implementation gives the correct behaviour in Jupyter Lab, but is incorrect using Voila, NbClassic and Notebook < 7 as they have different DOM structure.
The solution is to be more careful about where we add the menu in the DOM, not necessarily as the final child of the body
. We could carefully identify an appropriate <div>
to be the anchoring element, but the simplest solution is to add the menu as the first child of the body
as this works in all situations.
The @lumino/widgets
code to open a context menu at
does not currently support the option to specify an HTMLElement
to insert the menu before, but this functionality is available in Widget.attach
which is used by Menu.open
. So it will be relatively easy to expose this option in Menu.open
in the long term.
I propose the following two-stage fix:
Write a fix for ipydatagrid
as soon as possible. The fix will, just after calling Menu.open
, detach the menu from being the last child of the body
in the DOM and reattach it as the first child. In my experiments this works well without any flicker. ipydatagrid
can be released quickly so that the fix is available to end users quickly.
Expose the option in @lumino/widgets
to specify the HTMLElement
to attach the menu before, falling back to the current behaviour is no such element is specified. This will take a while to become available as @lumino
will have to be released and the new version adopted by Jupyter Lab, etc. Once it is fully adopted there will need to be an extra ipydatagrid
PR to pass the new argument to Menu.open
.
Closing as fixed, we may want to open an issue in lumino to track the changes required there
Description Hey team, we seem to be experiencing an issue where in some specific cases, clicking on the filter icon in an ipydatagrid datagrid, opens the context menu in a really far off position - scrolling to the bottom of the page. This behaviour is only exhibited when the same notebook is rendered in voila. I don't need the context menu, so is there any method that can hide the context menu?
Context voila version: 0.4.0 ipydatagrid: 1.1.16 Operating System and version: Windows 11 or MAC OS Browser and version: Chrome or Microsoft edge or Safari