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

ipydatagrid context menu opens in wrong position when rendered in voila #930

Closed
ibdafna opened this issue Aug 13, 2021 · 9 comments
Closed
Labels
bug Something isn't working

Comments

@ibdafna
Copy link

ibdafna commented Aug 13, 2021

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 top of the page. This behaviour is only exhibited when the same notebook is rendered in voila.

Reproduce

Checkout the gif file below, which shows how to reproduce the behaviour with the following code snippet:

import pandas as pd
import numpy as np
import ipydatagrid as grid
import ipywidgets as widgets

np.random.seed(104)
rang = 100
df = pd.DataFrame(
    data=[np.random.randint(0, 11, rang) for i in range(rang)],
    index=[f"Row {i}" for i in range(rang)],
    columns=[f"Col {i}" for i in range(rang)],
)

g = grid.DataGrid(
    df, layout={"height": "1200px", "width": "800px"}, selection_mode="cell"
    
)
display(g)
display(g)
  1. Execute the cell
  2. Scroll down so what the second view of the datagrid is covering ~95* of the screen
  3. Click on a filter icon in any one of the columns
  4. Voila will scroll the page to the top, where the context menu will be displayed (overlaid on top of the first grid view)

Expected behavior

The context menu should overlap on top of the second grid view - as it does when rendered in JupyterLab or Jupyter Notebook.

Context

  • voila version: 0.2.10
  • Operating System and version: Windows 10
  • Browser and version: Chrome 92

voila_ipydatagrid_issue

@ibdafna ibdafna added the bug Something isn't working label Aug 13, 2021
@jtpio
Copy link
Member

jtpio commented Aug 31, 2021

Thanks @ibdafna for reporting this.

This is reproducible on Binder with this gist: https://gist.github.com/jtpio/73fdfaa8881f3f68b46173eb8300952f

@jtpio
Copy link
Member

jtpio commented Aug 31, 2021

@ibdafna
Copy link
Author

ibdafna commented Sep 2, 2021

@jtpio that's the code which triggers opening the lumino context menu, but the weird positioning only happens in Voila

@ibdafna
Copy link
Author

ibdafna commented Sep 9, 2021

@jtpio let me know if there's anything else you need from me or if I've missed something! Thank you 😄

@jtpio
Copy link
Member

jtpio commented Sep 9, 2021

We could check whether the issue also happens with this PR, which switches to a lab-based application for the frontend: #846. We might have to make ipydatagrid compatible with ipywidgets 8.

Another guess could be the DOM structure being different between lab and voila.

@ibdafna
Copy link
Author

ibdafna commented Sep 17, 2021

@jtpio anything I can do on my end to help expedite?

@jtpio
Copy link
Member

jtpio commented Sep 21, 2021

@ibdafna I looked into it a bit and here are some findings.

This seems to be because the context menu is positioned as an absolute element on the page. In the case of JupyterLab and the classic notebook, the nearest positioned ancestor is body as absolute. With Voila it might not be the case depending on the template.

According to https://developer.mozilla.org/en-US/docs/Web/CSS/position#absolute_positioning and https://developer.mozilla.org/en-US/docs/Web/CSS/Containing_block#identifying_the_containing_block:

If a positioned ancestor doesn't exist, it is positioned relative to the ICB

Note: The containing block in which the root element () resides is a rectangle called the initial containing block. It has the dimensions of the viewport (for continuous media) or the page area (for paged media).

Which probably explains why it always tries to "scroll back up" when the context menu is opened since the viewport coordinates would be used to position the element, but relative to the top of the html document.


Testing locally with a diff like the following seems to be doing the trick:

diff --git a/js/feathergrid.ts b/js/feathergrid.ts
index 082d776..8cd7d68 100644
--- a/js/feathergrid.ts
+++ b/js/feathergrid.ts
@@ -163,8 +163,8 @@ class FeatherGridMouseHandler extends BasicMouseHandler {
       if (isMenuClick) {
         this._grid.contextMenu.open(grid, {
           ...hit,
-          x: event.clientX,
-          y: event.clientY,
+          x: event.clientX + window.scrollX,
+          y: event.clientY + window.scrollY,
         });
         return;
       }
context-menu-ipydatagrid.mp4

@ibdafna
Copy link
Author

ibdafna commented Sep 26, 2021

@jtpio this looks good, thank you. I'll need to check whether this logic change could break existing apps, but I do not anticipate that to be the case.

@martinRenou
Copy link
Member

Closing as fixed in ipydatagrid 1.3.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants