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

Can't do repeated closing of tabs using kb #6648

Open
ellisonbg opened this issue Jun 19, 2019 · 13 comments · Fixed by #7349 · May be fixed by #13921
Open

Can't do repeated closing of tabs using kb #6648

ellisonbg opened this issue Jun 19, 2019 · 13 comments · Fixed by #7349 · May be fixed by #13921
Assignees
Milestone

Comments

@ellisonbg
Copy link
Contributor

Open a bunch of tabs in the main work area. Click option+W to close a tab. Repeat. It only works the first time. Something about focus is being lost after the first go, causing the shortcut to stop working.

@ellisonbg ellisonbg added this to the 1.0 milestone Jun 19, 2019
@echarles
Copy link
Member

If it can help to find cause..., changing the tab when the shortcut does not work anymore makes it work again.

@mbektas
Copy link
Member

mbektas commented Jun 21, 2019

I am looking into this issue

@jasongrout
Copy link
Contributor

See phosphorjs/phosphor#396 (comment) for a suggestion about how to fix this.

@jasongrout jasongrout modified the milestones: 1.2, 2.0 Oct 11, 2019
@mbektas
Copy link
Member

mbektas commented Oct 13, 2019

@jasongrout I have been debugging this. I couldn't find a case when a tab is closed while keeping focus somewhere else. For example, if you close the tab using command palette (Close Tab item), palette resets and releases focus to newly activated tab. So, I found it safe to just activate new current tab as a solution.

@jasongrout jasongrout modified the milestones: 2.0, Future Dec 2, 2019
@jasongrout jasongrout self-assigned this Dec 2, 2019
@jasongrout
Copy link
Contributor

#7684 reverts the changes in #7349 because of unintended side effects. Re-opening this as still needing to be solved.

@jasongrout jasongrout reopened this Dec 23, 2019
@jasongrout jasongrout modified the milestones: 2.0, 2.1 Jan 2, 2020
@jasongrout jasongrout modified the milestones: 2.1, 3.0 Mar 12, 2020
@jasongrout
Copy link
Contributor

jasongrout commented Jun 16, 2020

From #8575 (comment), here's another writeup of what is going on:

The mechanics here are a bit tricky. The close shortcut is registered to be applicable to any element that is itself or a descendant of an element with the .jp-Activity css class. This makes sure you can only close actual activities. When the keybinding is run:

  1. It closes the shell's current widget
  2. Browser focus shifts to the body DOM element, which does not have the .jp-Activity class
  3. Subsequent Alt W then do not take effect since it is not considered active for the body element (since it doesn't have a .jp-Activity class)

One workaround is to make the Alt W shortcut just work on the body DOM element, like all the other application-level shortcuts defined next to it. Probably a better thing is to specifically activate the tab that is the document current tab.

My guess is that the same issue happens with the file close and cleanup shortcut.

@jasongrout jasongrout removed this from the 3.0 milestone Sep 16, 2020
@jasongrout jasongrout added this to the 3.1 milestone Sep 16, 2020
@blink1073 blink1073 modified the milestones: 3.1, 4.0 Jun 15, 2021
@renxida
Copy link

renxida commented Jul 11, 2022

Was going to file an issue, but found this.

I often need to kill tabs en-masse. Would love to have this.

@andrii-i
Copy link
Contributor

andrii-i commented Jan 24, 2023

Currently looking into this.

Probably a better thing is to specifically activate the tab that is the document current tab.

@jasongrout would it be reasonable to always focus newly chosen current tab (if there is any)?

@fcollonval
Copy link
Member

@andrii-i yes I would say so. The change would need to happen there:

@andrii-i
Copy link
Contributor

andrii-i commented Jan 25, 2023

@fcollonval Thank you. widget.close() message is what would close the tab as far as I understand. I was thinking the change should rather happen in the same place where TabBar chooses next tab to activate after the current tab is closed. Could you please point me to the logic where a new tab is chosen?

@jasongrout
Copy link
Contributor

jasongrout commented Jan 26, 2023

First, note that subtleties around Mehmet's attempt at fixing this above, and why the change was reverted and this reopened.

As for how the tab bar picks the current tab after a tab is closed, perhaps that happens in Lumino (I haven't traced through the code recently, but I do remember code that does this sort of thing at https://github.com/jupyterlab/lumino/blob/main/packages/widgets/src/tabbar.ts#L1117)

@fcollonval
Copy link
Member

The logic to pick the new current widget is indeed in Lumino; in TabBar

https://github.com/jupyterlab/lumino/blob/e7cc1adcbbfe5df85f2467947b047e9febb98166/packages/widgets/src/tabbar.ts#L1117

Maybe triggering a focus on the new active widget in Lumino is the way forward. But as pointed out by Jason, you must check the behaviour when the tab is closed programmatically - like the launcher case.

@andrii-i
Copy link
Contributor

@jasongrout @fcollonval thank you for the pointers. I will keep case of raise condition created when tab is closed programmatically in mind.

My logic was that because a) listening to currentChanged is the only way we can detect tab change on jupyterlab side and b) Lumino fires multiple currentChanged messages in a quick succession any time tab is closed programmatically as in launcher, there's no way to avoid a raise condition on jupyterlab side. And Lumino has access to source of truth, currentIndex parameter in TabBar / correct sequence of calls to set currentIndex(). I've created an issue 528 in jupyterlab/lumino to track this idea and facilitate a discussion around it on lumino side.

If you have thoughts on how to fix this within jupyterlab (or if I'm missing something in my reasoning), please share and I would love to investigate (e.g. if there is another way to know which tab is current on jupyterlab side outside of listening to currentChanged messages).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment