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.

Sign up for GitHub

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

Jump to bottom

Clicking in the minimap doesn't close the context menu #16853

Open
divyansshhh opened this issue Oct 10, 2024 · 7 comments
Open

Clicking in the minimap doesn't close the context menu #16853

divyansshhh opened this issue Oct 10, 2024 · 7 comments
Labels
bug
Milestone
4.3.x

Comments

@divyansshhh
Copy link
Contributor

Description

Explained in the GIF below -

22068-48-0_Mini_map_right_click_menu

Reproduce

Open a context menu in the minimap and click on the cells in the minimap

Context

  • JupyterLab version: 4.3.0b3
@jupyterlab-probot jupyterlab-probot bot added the status:Needs Triage Applied to new issues that need triage label Oct 10, 2024
@jtpio jtpio added this to the 4.3.0 milestone Oct 10, 2024
@nakul-py
Copy link

\assign

@krassowski
Copy link
Member

Some pointers:

The pointer down handler for minimap is defined here:

jupyterlab/packages/ui-components/src/components/windowedlist.ts

Lines 1523 to 1538 in c321c27

/**
* Handle `pointerdown` events on the virtual scrollbar.
*/
private _evtPointerDown(event: PointerEvent): void {
let target = event.target as HTMLElement;
while (target && target.parentElement) {
if (target.hasAttribute('data-index')) {
const index = parseInt(target.getAttribute('data-index')!, 10);
return void (async () => {
await this.scrollToItem(index);
this.jumped.emit(index);
})();
}
target = target.parentElement;
}
}

and called from here (where the default action and propagation also get cancelled):

jupyterlab/packages/ui-components/src/components/windowedlist.ts

Lines 916 to 922 in c321c27

handleEvent(event: Event): void {
switch (event.type) {
case 'pointerdown':
event.preventDefault();
event.stopPropagation();
this._evtPointerDown(event as PointerEvent);
break;

On a tangent, we might want to allow click + drag action in the future.

@JasonWeill JasonWeill removed the status:Needs Triage Applied to new issues that need triage label Oct 15, 2024
@JasonWeill
Copy link
Contributor

@krassowski Should this be a blocker for 4.3.0, or should it be fixed in 4.3.x or 4.4.0?

@krassowski
Copy link
Member

I don't think this is a blocker. This issue is not specific to 4.3 either - it exists in 4.2.x too. I just don't think there is much value in backporting a fix to 4.2.x as the virtual scrollbar was less prominent there. If it gets fixed before 4.3.0 is released - great. If not we probably patch it in 4.3.x

@JasonWeill JasonWeill mentioned this issue Oct 15, 2024
Open
@jtpio jtpio modified the milestones: 4.3.0, 4.3.x Oct 28, 2024
@peytondmurray
Copy link
Contributor

Hey all, I'm going to take a look into this, if that's okay with everyone. Thanks @krassowski for the pointers on where to start!

@peytondmurray
Copy link
Contributor

peytondmurray commented Dec 23, 2024 •
edited
Loading

Hmm, after a quick look it seems like letting the event bubble up does the trick, rather than stopping propagation inside the WindowedList event handler. Is there any reason this wouldn't be a viable solution?

@krassowski
Copy link
Member

I think this would be the right solution. I would double check if user-select: none is set because cancelling the event might have a side effect of preventing text selection within the minimap which I think is desirable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
4.3.x
Development

No branches or pull requests
6 participants
@jtpio @krassowski @peytondmurray @divyansshhh @JasonWeill @nakul-py