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

add sidebar min-width css variable #5245

Merged
merged 1 commit into from Sep 12, 2018
Merged

Conversation

@ivanov
Copy link
Member

@ivanov ivanov commented Aug 31, 2018

Sets it to 250px instead of the 300px that it used to be.

Down the line, it would be nice to have a minimum width that is separate from the default width, but I have not been able to make that work so just submitting this as is.

Also, there is flexbox css stuff that should get cleaned up because the last modified elements eat too much into the filename, but, again, separate PRs for that in the future.

@ivanov
Copy link
Member Author

@ivanov ivanov commented Aug 31, 2018

Here's the old 300px width, followed by the new 250px width
300px
250px

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Aug 31, 2018

It seems the code is actually saying 100px?

Sets it to 250px instead of the 300px that it used to be
@ivanov ivanov force-pushed the sidebar-width-var branch from 900eaf4 to 41eaee9 Aug 31, 2018
@ivanov
Copy link
Member Author

@ivanov ivanov commented Aug 31, 2018

oops, thanks @jasongrout - I neglected to add it when amending an earlier commit. Fixed now.

@ivanov
Copy link
Member Author

@ivanov ivanov commented Aug 31, 2018

are the tests known to be flaky? the appveyor and travis failures are different, but each has just one.

@blink1073
Copy link
Member

@blink1073 blink1073 commented Aug 31, 2018

Yes, server-related tests are a headache on CI...

@ivanov
Copy link
Member Author

@ivanov ivanov commented Aug 31, 2018

yay, all pass now!

@ellisonbg
Copy link
Contributor

@ellisonbg ellisonbg commented Aug 31, 2018

@blink1073 blink1073 added this to the 0.35 milestone Aug 31, 2018
@blink1073 blink1073 removed this from the 0.35 milestone Sep 5, 2018
@blink1073 blink1073 added this to the 1.0 milestone Sep 5, 2018
@ellisonbg
Copy link
Contributor

@ellisonbg ellisonbg commented Sep 5, 2018

I would love to see the minimum width of the wide panels decreased. There have definitely been a number of situations where I or others tried to reduce the size. However, I think it would hurt usability for us to decrease the default size. The file browser does OK at slightly narrower width, but there is an increasing number of extensions running in the L wide panel that are already "bursting at the seams" to function well at the current 300px. Examples include: the git extension, toc, extension manager.

@sccolbert any thoughts on how we could set different default and min widths for the L/R sidebars in JLab?

Another thing that would help styling content at narrower widths would be to add css classes to the stack panel container based on its width (width-normal, width-narrow, width-wide). Then extensions could hide/show elements based on the available width.

rgbkrk
rgbkrk approved these changes Sep 8, 2018
@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Sep 12, 2018

It's been nearly two weeks now. I'll merge this to keep the iteration going. If we try it out and find it's too narrow, we can bump the width back up in another pr, and keep the width still a theme variable.

Paul, do you mind opening issues for the two unsolved things you mention above?

@jasongrout jasongrout merged commit 6d66929 into jupyterlab:master Sep 12, 2018
2 checks passed
@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Sep 12, 2018

And thanks very much @ivanov! Thanks for being patient with the delay.

@ivanov
Copy link
Member Author

@ivanov ivanov commented Sep 14, 2018

Thanks @jasongrout - I think you meant to mention me, not @ pivanov - appologies to my namesake, I'll edit that out so he doesn't continue to get notifications here.

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Sep 14, 2018

Yes, thanks!

ivanov added a commit to ivanov/jupyterlab that referenced this issue Sep 21, 2018
This is a backport of jupyterlab#5245, but keeps the width at 300px, so no visual
changes.
@blink1073 blink1073 removed this from the 1.0 milestone Sep 28, 2018
@blink1073 blink1073 added this to the 0.35 milestone Sep 28, 2018
@blink1073 blink1073 mentioned this pull request Sep 28, 2018
31 tasks
@lock lock bot locked as resolved and limited conversation to collaborators Aug 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants