Removed toolbar scrollbar

[3coins]

References

This PR fixes #10189

Code changes

• Removed css styles for toolbar hover

User-facing changes

Backwards-incompatible changes

None

[jupyterlab-dev-mode]

Thanks for making a pull request to JupyterLab!

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

[3coins]

cc @jtpio

[fcollonval]

LGTM @3coins

You will make Firefox users happier 😉

[krassowski]

I think there might be more changes needed to remove it cleanely.

[krassowski]

Looking at #8417 I think this line can be removed too:

jupyterlab/packages/ui-components/src/components/toolbar.tsx

Line 164 in cb406d3

this.addClass('jp-scrollbar-tiny');

Not 100% sure though.

[fcollonval]

You may be right... I'm unsure we have to remove the class in case some JupyterLab remix rely on it?

The relevant PR is #8417

[3coins]

#8417

With the new responsive toolbar, scrollbar shouldn't appear at all, so this should be redundant; in an ideal world this should be removed. We need to make sure that both this and the responsive change are merged together into a release.

[krassowski]

Yes, I think the discussion is at making sure we remove things added in #8417. There is however a case for keeping some of that code as it could be used for #10305 hence I only suggested removing one extra line which is no longer needed.

[3coins]

Yes, I think the discussion is at making sure we remove things added in #8417. There is however a case for keeping some of that code as it could be used for #10305 hence I only suggested removing one extra line which is no longer needed.

I misread @fcollonval's comments earlier, agree that removing just that line should be harmless. Latest push removed that line.

[isabela-pf]

I'm excited to see the toolbar's scrollbar removed (probably since I'm one of those pesky Firefox users)! When I opened the binder and made my window narrow, I did find the scrollbar removed and the … menu items overlapping with each other so that I can't read them.

I'm using Firefox 90.0.2. Is this tied to the comments about how to remove the toolbar cleanly?

[3coins]

@isabela-pf
Thanks for reviewing this change!
That needs some more work on the responsive toolbar. One solution is to expand the toolbar popup height dynamically so that all the items can fit. Here is a POC.

Let me update this and send out another commit.

I'm using Firefox 90.0.2. Is this tied to the comments about how to remove the toolbar cleanly?

No, the comments above were referring to removing the custom scrollbar styles on the toolbar which is redundant with the responsive approach.

[fcollonval]

If you gonna update a bit the styles, I would also suggest to put the ... icon on the far right of the toolbar. This will fit better the pattern that additional elements are to be found from that button. And its position won't change depending on which toolbar item gets hidden.

From your latest screenshot:

[3coins]

@isabela-pf
I have fixed the issue, the toolbar popup will expand to fit the items now.

@fcollonval
Fixed the css to push the ellipsis to the right of toolbar.

[fcollonval]

Great changes.

I wonder if the item in the toolbar popup should be aligned left or right (right seems better - so buttons are closer to the mouse cursor). The justify current alignment on the second line does not look great.

[3coins]

@fcollonval
The kernel status icon has an auto margin on it that causes it to take most of the remaining space. I tried to address it in this PR.

[fcollonval]

The kernel status icon has an auto margin on it that causes it to take most of the remaining space. I tried to address it in this PR.

Thanks for the follow-up. This is then good to me.

[jhgoebbert]

This is great news for all Firefox users! Thank you!

[fcollonval]

@meeseeksdev please backport to 3.1.x

[lumberbot-app]

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

1. Checkout backport branch and update it.

$ git checkout 3.1.x
$ git pull

2. Cherry pick the first parent branch of the this PR on top of the older branch:

$ git cherry-pick -m1 e66585bc4bf6bdc824370be235b52fcfc74bc7c2

3. You will likely have some merge/cherry-pick conflict here, fix them and commit:

$ git commit -am 'Backport PR #10790: Removed toolbar scrollbar'

4. Push to a named branch :

git push YOURFORK 3.1.x:auto-backport-of-pr-10790-on-3.1.x

5. Create a PR against branch 3.1.x, I would have named this PR:

"Backport PR #10790 on branch 3.1.x"

And apply the correct labels and milestones.

Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon!

If these instruction are inaccurate, feel free to suggest an improvement.

[jtpio]

Thanks all! The new responsive toolbar seems to be working well on mobile devices and is a good alternative to having the scrollbar.