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

Responsive document toolbar #10720

Merged
merged 3 commits into from Jul 30, 2021
Merged

Conversation

3coins
Copy link
Contributor

@3coins 3coins commented Jul 28, 2021

References

This PR fixes the issue #10595

Code changes

Toolbar class has a new default button and a popup. The resize handler is added which keeps track of widths of items and moves them to the popup if the cumulative width increases toolbar width. Alternatively, if toolbar width is big enough to accommodate any widgets in the popup, they are moved back to the toolbar.

User-facing changes

Toolbar has a new default button and a popup that stores items that don't fit in the toolbar when the toolbar is resized.

responsive-toolbar

Backwards-incompatible changes

None

@jupyterlab-dev-mode
Copy link

Thanks for making a pull request to JupyterLab!

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

@krassowski
Copy link
Member

My slight preference would be to keep the items that indicate state (kernel name, kernel busy indicator, debugger toggle, cell type switch) visible as long as possible (hide them last). Would this be feasible to implement? What do others think?

@3coins
Copy link
Contributor Author

3coins commented Jul 28, 2021

My slight preference would be to keep the items that indicate state (kernel name, kernel busy indicator, debugger toggle, cell type switch) visible as long as possible (hide them last). Would this be feasible to implement? What do others think?

I had a slight inclination towards that idea while implementing this. It is definitely possible to implement, however I feel that it might increase the complexity of the code quite a bit without providing any huge benefit to users. Also, this code is quite general at the moment that it works for all toolbars, so it will require some kind of configuration added for all toolbars to define elements to hide last. Would love to hear feedback from others.

@fcollonval
Copy link
Member

We definitely should extend the metadata of toolbar items to add the ability to select which element should be hidden first.

I see two paths:

  • Simple approach: define a primary and a secondary groups of items. The secondary groups are hidden first starting from the most rightful element. Then the primary group is hidden from the most rightful element.
  • Advanced approach: there is a ranking of visibility on all items. That ranking is used to determine in which order element are hidden.

I would favor the first approach as visibility importance will definitely depends on the user taste; so it should be customizable (don't worry about that here) and I think a simple primary / secondary approach will be easier to grasp than figure out the ranking of all items.

cc @ellisonbg

@ellisonbg
Copy link
Contributor

@fcollonval I do think that over time we need to tackle this question of additional metadata for toolbar items to help prioritze when different items get moved to the overflow toolbar. Adding another option to the list:

  • Introduce semantic sections for toolbar items and move the entire section the overflow menu at once. I have a seen a number of applications use this approach very effectively. In particular Google Docs uses a triple dot icon with an overflow toolbar in the right-most position in the toolbar. As the width decreases, groups are moved to the overflow menu starting from the right-most group and moving left.

Full width:

Screen Shot 2021-07-29 at 9 12 45 AM

A few groups moved to the overflow:

Screen Shot 2021-07-29 at 9 12 59 AM

@ellisonbg
Copy link
Contributor

At the same time, I believe this PR is enough of an improvement, that we should merge it and iterate further to add sections and/or other metadata.

@blink1073 blink1073 added this to the 4.0 milestone Jul 30, 2021
Copy link
Member

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge and iterate sounds good, thank you!

@blink1073 blink1073 merged commit ab688dd into jupyterlab:master Jul 30, 2021
@welcome
Copy link

welcome bot commented Jul 30, 2021

Congrats on your first merged pull request in this project! 🎉
congrats
Thank you for contributing, we are very proud of you! ❤️

@3coins
Copy link
Contributor Author

3coins commented Jul 30, 2021

We definitely should extend the metadata of toolbar items to add the ability to select which element should be hidden first.

I see two paths:

  • Simple approach: define a primary and a secondary groups of items. The secondary groups are hidden first starting from the most rightful element. Then the primary group is hidden from the most rightful element.
  • Advanced approach: there is a ranking of visibility on all items. That ranking is used to determine in which order element are hidden.

I would favor the first approach as visibility importance will definitely depends on the user taste; so it should be customizable (don't worry about that here) and I think a simple primary / secondary approach will be easier to grasp than figure out the ranking of all items.

cc @ellisonbg

@krassowski @fcollonval Opened a new PR for further updates to responsive toolbar.

3coins pushed a commit to 3coins/jupyterlab that referenced this pull request Sep 28, 2021
3coins pushed a commit to 3coins/jupyterlab that referenced this pull request Sep 29, 2021
3coins pushed a commit to 3coins/jupyterlab that referenced this pull request Sep 29, 2021
blink1073 pushed a commit that referenced this pull request Sep 29, 2021
* Manual backport of #10720, #10790, #11108

* Fix for failing UI tests

Co-authored-by: Piyush Jain <pijain@amazon.com>
@github-actions github-actions bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Feb 6, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Design System CSS enhancement pkg:apputils pkg:debugger pkg:docregistry pkg:notebook status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. tag:CSS For general CSS related issues and pecadilloes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants