Activate new current tab

[mbektas]

fixes #6648

This fixes the issue when current tab and active widget mismatch after closing a tab using keyboard shortcut [Alt W]. It is done by explicitly activating new current tab.

[jupyterlab-dev-mode]

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link:

[blink1073]

Great, thanks!

[jasongrout]

From #6648

@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.

Just because that's the way it's done in core currently, I don't think we should necessarily force it on users and extensions.

Is there a way to implement @sccolbert's suggestion of (a) checking if we are doing a close event, not just any change event, and (b) checking if the closed tab had focus?

[jasongrout]

For example, we could install a child-removed message interceptor somewhere that catches the widget actually removing itself. Or maybe using the activeChanged signal might help, since that also implies something about current focus.

[mbektas]

@jasongrout whenever a tab is closed, it is removed from focustracker and it explicitly sets the active widget to null (https://github.com/phosphorjs/phosphor/blob/master/packages/widgets/src/focustracker.ts#L252).

How about if we add a check if activeWidget is null when current is changed, before setting the new current to new active? It seems to work properly.

app.shell.currentChanged.connect((sender, args) => {
      if (!app.shell.activeWidget && args.newValue) {
        app.shell.activateById(args.newValue.id);
      }
});

implementing @sccolbert's suggestion should be also possible but definitely more complex.

[jasongrout]

whenever a tab is closed, it is removed from focustracker and it explicitly sets the active widget to null

More than that, it changes the current widget too. The active widget is set to null in other places too, but in those situations, the current widget is kept the same.

Yes, I think that logic might work well to identify widgets being removed. Let's try it out.

[mbektas]

Yes, I think that logic might work well to identify widgets being removed. Let's try it out.

I pushed the change.

[blink1073]

This seems reasonable to me (I tried it locally). Thanks!