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

Inconsistent toolbar button positions when rendered in notebook #15509

Closed
firezym opened this issue Dec 9, 2023 · 9 comments · Fixed by #15553
Closed

Inconsistent toolbar button positions when rendered in notebook #15509

firezym opened this issue Dec 9, 2023 · 9 comments · Fixed by #15553
Labels
bug tag:Regression Behavior that had been broken, was fixed, and is broken again tag:Release Blocker A must-have bug for the milestone to which it is tagged
Milestone

Comments

@firezym
Copy link

firezym commented Dec 9, 2023

Description

The Debugger Button is "jumping around" among different notebooks in the Notebook Panel Toolbar epecially after refreshing, as shown in below gif demo:

Reproduce

gif-pics

Expected behavior

User should see a stable button order on the Notebook Toolbar.

Context

os: wsl2 linux system, using jupyterlab 4.0.9

jupyter_client 8.6.0
jupyter_core 5.5.0
jupyter-events 0.9.0
jupyter-lsp 2.2.1
jupyter_server 2.12.1
jupyter_server_terminals 0.4.4
jupyterlab 4.0.9
jupyterlab_celltagsclasses 0.5.1
jupyterlab_gridwidth 0.2.4
jupyterlab_pygments 0.3.0
jupyterlab_server 2.25.2

@firezym firezym added the bug label Dec 9, 2023
@jupyterlab-probot jupyterlab-probot bot added the status:Needs Triage Applied to new issues that need triage label Dec 9, 2023
@krassowski
Copy link
Member

Thank you for reporting this! Can you try jupyterlab pre-release 4.1.0a4 in which the logic was refactored?

@firezym
Copy link
Author

firezym commented Dec 9, 2023

@krassowski Thanks.

I conda created a new env for 4.1.0a4, the previous "jump around" issue reduced a lot. But there is still a small chance of dis-organized order of icons after refreshing, shown at the end of the following gif ( I cut out about 60s of refreshing back and forth more than 20 times in the middle of the screen recording). Also take a screen shot of this bizzare state at the end of the gif.

image

Another issue is shown at the the first half of the gif demo, which is very frequently observed: the debugger button turn gray and disabled often after refreshing and stay gray until next refresh.

gif-pics

@JasonWeill JasonWeill changed the title The Debugger Button is "jumping around" among different notebooks in the Notebook Panel Toolbar Inconsistent toolbar button positions when rendered in notebook Dec 12, 2023
@JasonWeill JasonWeill removed the status:Needs Triage Applied to new issues that need triage label Dec 12, 2023
@JasonWeill JasonWeill added this to the 4.1.0 milestone Dec 12, 2023
@krassowski krassowski added tag:Release Blocker A must-have bug for the milestone to which it is tagged tag:Regression Behavior that had been broken, was fixed, and is broken again labels Dec 15, 2023
@krassowski
Copy link
Member

The refactor of toolbars happened in #15021 and I can confirm that it introduced a regression with toolbar items being hidden or placed randomly on first load. It feels that we should defer the position shuffling and collapsing until the layout has settled. CC @brichet

An issue which sounds similar (but is largely unrelated in the codebase) was brought up by @gabalafou in context of the menu hamburger implementation (see #13148 (comment)).

Now, I wonder if contain: strict introduced in lumino 2.0 on widgets (by yours truly) could contribute to the edge cases in the positioning logic; if it is so please feel free to override in the appropriate scopes (toolbars/menus).

@brichet
Copy link
Contributor

brichet commented Dec 18, 2023

toolbar items being hidden or placed randomly on first load

Indeed I managed to reproduce these behaviors after a lot of refresh.

It may be a side effect of #15021, but this PR does not change the way the items are included in the toolbar.
However, we added a backup of the items order in the reactive toolbar. This means that every time an item is added to the toolbar, we save the current order (it should happen only at startup in a classic installation).
This 'saved order' is used to keep the items position when the size of the toolbar changes (and some items are moved/restored to/from the responsive popup).

Is it possible that sometime the items are first added in a wrong order, and reordered later ?
It should be the problem since this reordering would not change the 'saved order' if there is no new element.

@JasonWeill
Copy link
Contributor

We recently released 4.1.0 beta 0. Is this still occurring in beta 0?

@brichet
Copy link
Contributor

brichet commented Dec 21, 2023

This is still occurring on latest beta.
On my side I can see this behavior when loading the first time Jupyterlab if several notebooks are opened in the main area. The displayed Notebook is not affected, but all the others have the toolbar reversed. It seems that it does not occur after reloading the page, only on first load.

After some tests it seems to be related to #15021.

@brichet
Copy link
Contributor

brichet commented Dec 21, 2023

I opened #15553 to fix the reversed order.
Now we have the same behavior as previously, the debugger button inserted sometime at first position 😄

I'll try to explain what I understood.
When the Notebook is hidden, the toolbar has no width because it is not rendered. Therefore, all the widget are inserted in the responsive popup.
The debugger icon should be inserted before the kernel name, but is inserted at position 0 since the kernel name is not in the toolbar.

I think we should avoid using the toolbar.insertBefore() method on a reactive toolbar (the target can be hidden), or rewrite this method for the reactive toolbar, using the 'supposed' position of the target.

@firezym
Copy link
Author

firezym commented Dec 21, 2023

@brichet thanks!
BTW, do u notice that the debugger button is sometime gray after refreshing as the main post gif showing? If it's gray, it can not be activated...

@brichet
Copy link
Contributor

brichet commented Dec 22, 2023

I updated #15553 to fix also the relative insertion in the reactive toolbar.
This should fix the debugger icon position.

BTW, do u notice that the debugger button is sometime gray after refreshing as the main post gif showing? If it's gray, it can not be activated...

This must an other issue related to the debugger itself, not to the toolbar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug tag:Regression Behavior that had been broken, was fixed, and is broken again tag:Release Blocker A must-have bug for the milestone to which it is tagged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants