search

jupyterlab / lumino

Lumino is a library for building interactive web applications
https://lumino.readthedocs.io/
Other
598 stars 127 forks source link

Do not set the dockpanel as parent of the tabbar #606

Closed brichet closed 11 months ago

brichet commented 12 months ago

This PR fixes https://github.com/jupyterlab/lumino/issues/604

It adds an attribute to Widget to disable the tracking from focusTracker, event if its elements can get the focus.

By default all the widgets are trackable, except for the TabBar.

It avoids setting the parent of the tab bar in the dock panel.

fcollonval commented 11 months ago

Thanks a lot for digging this one up @brichet

I would rather avoid adding a new attribute for this.

Looking at the current behavior, when the dock panel creates a split, it adds a tab bar. When that tab bar is attached to the panel, its parent is set.

https://github.com/jupyterlab/lumino/blob/07d4bab5b73b753332d941495c27109fea9d25c2/packages/widgets/src/docklayout.ts#L1133

That triggers the child-added / child-removed message listened by the focus tracker.

https://github.com/jupyterlab/lumino/blob/07d4bab5b73b753332d941495c27109fea9d25c2/packages/widgets/src/widget.ts#L232

The addition of the split handle does not triggered such messages as they are directly attached to the HTML element.

This is actually wrong as adding/removing a tab bar should be transparent for dock panels. The message child-added and child-removed should only be triggered for child panels.

The wrong behavior can also be seen in JupyterLab by the wrongly addition of the class jp-Activity to the tab bars (it should only be on the MainAreaWidget) :

tabbar-issue

Therefore I think a better fix would be to not set the tab bar parent, simply attaching it as it is done for the handle.

cc @afshin @krassowski what do you think?

Another approach could be the inhibition of the child-added/child-removed message emission. But I don't see a good way of doing this without overriding the set parent method of the tab bar. But then it may have unwanted effect for downstream code consuming tab bar outside a dock panel (like the weird usage in the JupyterLab sidebars).

brichet commented 11 months ago

Therefore I think a better fix would be to not set the tab bar parent, simply attaching it as it is done for the handle.

Thanks for the suggestion @fcollonval, I reverted the previous changes and add a commit to avoid setting the parent of the tab bar.