ENH: manage sidebars on small screens #726

12rambau · 2022-06-12T15:50:36Z

That's still a draft so i'll track down all the issues referencing this problem later.

Context

Lot's of people are complaining that the theme is not as responsive as they would expect on small devices. Specifically the sidebars both primary and secondary are a burden for everyone as they are not properly managed in the theme forcing people to discard them most of the time (specifically true for the secondary one).

proposal

In this PR I proposed to improve the design by transforming the 2 sidebars into drawers on small screens. One on the left and one on the right. without changing the interactivity that we already have on bigger screens

TODO

make drawers closable on click outside
add a dark overlay to hide the content while opening a drawer
ensure only 1 of the 3 drawers can be opened at the time
correct scrolling bug in the drawer
tune the hide/show behavior between css and JS

Before I continue working on it I would like to have other feedback to see if this feature is useful or not. (to test it simply go to small screen the rest is behaving funny.

choldgraf · 2022-06-12T17:14:55Z

Agreed this one would be quite useful. A few relevant issues where this has been discussed:

In particular, I wrote out a little suggested implementation in the top comment here:

Left sidebar / table of contents toggle drawer on mobile #317 (comment)

That is based on the pattern that Furo and the Sphinx Book Theme use. It is also similar to the approach that the sphinx-basic-ng theme takes (cc @pradyunsg).

As an example:

Here's where the input/labels are in the book theme: https://github.com/executablebooks/sphinx-book-theme/blob/af6277a7276112ee915d901984193c49e56d5a76/src/sphinx_book_theme/theme/sphinx_book_theme/layout.html#L9-L17
Here's the SCSS to customize the toggle behavior w/o using JavaScript: https://github.com/executablebooks/sphinx-book-theme/blob/57e6d802521c41bbc25b5945d1e62abc3cfc00c7/src/sphinx_book_theme/assets/styles/sections/_sidebars-toggle.scss

I think this would definitely be useful! Happy to help provide guidance on implementation if you're interested in this route.

12rambau · 2022-06-20T22:19:14Z

New version fully based on css. Could you test it from your side ? I'm pretty sure there are edge cases that I didn't catch

choldgraf

This is generally looking really nice! I made a few suggestions and comments in there but I think it would be a great improvement!

One thing we don't need to do now, but might explore over time if the header feels too cramped, is adding a secondary header that hides itself on wide screens, and put the sidebar toggle buttons in there.

choldgraf · 2022-06-21T08:04:30Z

src/pydata_sphinx_theme/assets/styles/sections/_sidebar-primary.scss

@@ -2,17 +2,21 @@
 * The primary sidebar on the left.
 * e.g., between-pages navigation.
 */
-.bd-sidebar {
+.bd-sidebar-primary {


I'm +1 on this, but just a note that this will be a breaking change so we should note it in the release notes

src/pydata_sphinx_theme/assets/styles/variables/_layout.scss

src/pydata_sphinx_theme/assets/styles/components/_sidebar_toggle.scss

src/pydata_sphinx_theme/assets/styles/sections/_sidebar-secondary.scss

src/pydata_sphinx_theme/theme/pydata_sphinx_theme/sections/sidebar-secondary.html

src/pydata_sphinx_theme/assets/styles/variables/_icons.scss

src/pydata_sphinx_theme/assets/styles/sections/_header.scss

12rambau · 2022-06-21T23:04:26Z

There are still issues with breakpoints as breakpoints-up and breakpoint-down are not behaving the same way. What do you think if I fix these values for both sidebars as layout variables ?

choldgraf · 2022-06-22T08:09:38Z

There are still issues with breakpoints as breakpoints-up and breakpoint-down are not behaving the same way. What do you think if I fix these values for both sidebars as layout variables ?

I'm not exactly sure what you mean by this, but if you're saying that we use $variables to accomplish the same thing manually, I think it's fine if it gets the result we want

choldgraf · 2022-06-22T08:15:10Z

Looking better! A couple other quick comments:

On mobile, the presence of the left button is pushing all of the navigation links to the right:
I think they're slightly too large...I think 1.2em looked a bit better to me. Also would help if they were a more muted color so they don't pop out so much.
The icons should be flipped horizontally I think. We want the arrow of the icon to be pointing in the direction that the draw will fly out.

I'll also note that I'm starting to feel like the header on mobile is quite "busy". It makes it feel less clean now that the logo is no longer right up against the left side. What do you think about either:

trying to put them beneath the header in their own small secondary header?
Putting the buttons inside the header dropdown, just after navbar-end?

Maybe that could resolve some of the above issues as well?

12rambau · 2022-06-22T20:34:51Z

So I reintroduced the toc_width_show parameter to unable different behaviours when the secondary sidebar is missing. I was forced to place it outside of the block elements in the layout.html so that any template have access to it (it's used in header-article, layout itself and secondary-sidebar). Using sass parameters, I can accept any value to this class (toc-show-xs, toc-show-sm….
Same for the primary sidebar. If one day we decide to make it a parameter we can change the variable in _variables.sass and all the relative elements will be modified together.
I realized that the squeezing were not synchronized everywhere. We were mixing breakpoint-up and breakpoint-down that doesn’t use the same limits. I choose to stick to breakpoint-up as it allows to use “xl”. (in breakpoint-down “xl” is equivalent to always). I tweeked all the calls to respect that.
I added a header-article navbar to display the 2 icons so that they don’t increase the number of things to display in the top navbar. this bar is covered when the top navbar is expanded so that we can't open sidebars and top navbar at the same time.

choldgraf · 2022-07-07T09:45:10Z

OK I made a little bit of progress here, just pushed a commit that reworks the sidebar logic a little bit, but I think there is still more to be done. The spacing is a little bit off, and the buttons don't show up at quite the right time, but it is an iterative improvement I hope 😬 (also merged main in)

I can try to keep thinking on this one, but just wanted to push the work thus far for transparency and so we don't accidentally make a big merge conflict

choldgraf · 2022-07-07T14:17:51Z

OK I think that I got this working! Wanna take a look? In the GIF below I'm showing going from wide to mobile width, and you can see the sidebar elements collapsing into buttons. I then click on the "home page" which shows both buttons not appearing because there are no sidebars on that page.

?

12rambau · 2022-07-07T16:07:13Z

That's excellent

Correct me if I'm wrong but the offset to leave space for the article-header is never changed even when no header is on the page right ? That's not a problem to me just want to make sure that it's a feature

choldgraf · 2022-07-07T18:05:23Z

Correct me if I'm wrong but the offset to leave space for the article-header is never changed even when no header is on the page right ? That's not a problem to me just want to make sure that it's a feature

I think that's true - I figured that it would look weird if the titles of pages jumped up and down based on whether there were going to be sidebars.

12rambau · 2022-07-08T06:26:30Z

I fixed the test for the new sidebar implementation, I think we cannot test the behaviour in the current tests, that will be something for #585 I'm happy with the curent state, @choldgraf thanks for your help!

choldgraf · 2022-07-08T07:19:46Z

Ah thanks for fixing the test!

Hmm, so what do you think we should do about this PR re: the next release? I think it seems in pretty good shape, should we get it into v0.10? Or hold off until that is released before merging this one in?

My feeling is that we'll need to make a release candidate with this release anyway, so that could provide some time for others to provide feedback on the sidebar behavior.

What do you or others think?

12rambau · 2022-07-08T07:30:41Z

I'm timeline agnostic.Although It's a big change but highly requested one (many documentations are playing with their custom css to avoid the primary sidebar to end up on top of the page on small screen). I think it's ready to go. Happy to hear others feelings about it being shiped with v0.10

choldgraf · 2022-07-08T07:38:46Z

anybody else have thoughts on #726 (comment) ?

cc @jorisvandenbossche @jarrodmillman @drammock @bryevdv @tupui

tupui · 2022-07-08T08:05:20Z

Oh waou this is great! Since it's a major improvement, I would not delay it.

Otherwise it seems intuitive. I am just not sure if it should not stick when you scroll (maybe a floating object like rdt is doing?). In any case it can be continued in another iteration.

drammock · 2022-07-08T13:46:04Z

I'd say put it in the RC, don't wait until after release.

choldgraf · 2022-07-08T15:00:25Z

OK in that case, I think we should merge this in a day or two (ideally after an iteration of comments from somebody :-) )

choldgraf · 2022-07-10T12:46:05Z

OK unless anybody objects I'll plan to merge this one later today or tomorrow, and then we can get feedback from folks in new issues etc

choldgraf · 2022-07-10T18:42:15Z

OK let's merge this one and iterate a bit!

gabalafou · 2023-10-31T16:03:06Z

src/pydata_sphinx_theme/assets/styles/sections/_sidebar-primary.scss

@@ -86,6 +87,11 @@
 /* Between-page links and captions */
 nav.bd-links {
  font-size: var(--pst-sidebar-primary-font-size);
+  margin-right: -1rem;


@12rambau do you remember why you styled nav.bd-links (the nav element containing the section navigation links) to have margin-right: -1rem? (Aside: can you tell me what the bd- prefix means?)

I can plainly see and understand why you set the negative -1rem margin for the in-page TOC in the right sidebar (it's so that the vertical notch that indicates the current section is aligned properly), but so far I haven't been able to figure out what the -1rem margin is doing for the left sidebar. (On the left sidebar, the current section notches are not aligned to any vertical line.)

the bd- prefix is the bootstrap one that they use usually in their demo examples. it's a very old issue but for the release of version 1 we would like to get rid of them #37.

About the -1rem I guess the only way to find out is to remove it 😄

12rambau added 5 commits June 12, 2022 15:48

manage primary sidebar

7e286ff

customize the color of the arrow

aa1b564

hide the arrow when not needed

fdb0e44

deal with secondary sidebar

38b37a8

keep the parameter of the secondary sidebar

19546f2

toggle without js

a4b1828

12rambau marked this pull request as ready for review June 20, 2022 22:19

choldgraf reviewed Jun 21, 2022

View reviewed changes

12rambau added 8 commits June 21, 2022 21:38

tune btn rendering

2a4d0cd

change alignement and font size

e835a9f

change pcursor style

b74cd04

cleaning

35eaf63

reintroduce bd-toc

e07a165

use bootstrap zindex varables

5f6c6d2

overlap on top of the header

c687c64

reintroduce the toc_width behaviour

a367772

12rambau added 6 commits June 22, 2022 13:42

reduce font size

d3f245d

use variables to control sidebars

75ba580

cleaning

4cff4b0

cleaning

b91ba97

add the sidebars button in the article header

9cf8076

inverse toggle sides

09a919a

12rambau changed the title ~~manage sidebars on small screens~~ ENH: manage sidebars on small screens Jun 23, 2022

dimmed the rendering of the header article

bdab3b6

choldgraf added 2 commits July 7, 2022 09:13

Cleaning up sidebar behavior

3220cab

Fix sidebars

3690f39

choldgraf marked this pull request as draft July 7, 2022 09:44

choldgraf marked this pull request as ready for review July 7, 2022 14:17

fix tests

7b49fc2

choldgraf added 3 commits July 8, 2022 08:36

Clean up CSS

0f38ec8

Merge branch 'main' into sidebars

cd5a789

Fix random scss bug

cd165d3

jarrodmillman approved these changes Jul 10, 2022

View reviewed changes

choldgraf merged commit 177c05e into pydata:main Jul 10, 2022

12rambau deleted the sidebars branch July 11, 2022 06:12

This was referenced Jul 13, 2022

Fix missing sidebar #777

Closed

search functionality missing #752

Closed

jarrodmillman modified the milestones: 0.11, 0.10 Jul 26, 2022

gabalafou reviewed Oct 31, 2023

View reviewed changes

gabalafou mentioned this pull request Nov 6, 2023

Consistent focus ring (first pass) #1549

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: manage sidebars on small screens #726

ENH: manage sidebars on small screens #726

12rambau commented Jun 12, 2022 •

edited by choldgraf

choldgraf commented Jun 12, 2022

12rambau commented Jun 20, 2022

choldgraf left a comment

choldgraf Jun 21, 2022

12rambau commented Jun 21, 2022

choldgraf commented Jun 22, 2022

choldgraf commented Jun 22, 2022 •

edited

12rambau commented Jun 22, 2022

choldgraf commented Jul 7, 2022 •

edited

choldgraf commented Jul 7, 2022 •

edited

12rambau commented Jul 7, 2022 •

edited

choldgraf commented Jul 7, 2022

12rambau commented Jul 8, 2022

choldgraf commented Jul 8, 2022

12rambau commented Jul 8, 2022

choldgraf commented Jul 8, 2022

tupui commented Jul 8, 2022

drammock commented Jul 8, 2022

choldgraf commented Jul 8, 2022

choldgraf commented Jul 10, 2022

choldgraf commented Jul 10, 2022

gabalafou Oct 31, 2023

12rambau Oct 31, 2023

ENH: manage sidebars on small screens #726

ENH: manage sidebars on small screens #726

Conversation

12rambau commented Jun 12, 2022 • edited by choldgraf

Context

proposal

TODO

choldgraf commented Jun 12, 2022

12rambau commented Jun 20, 2022

choldgraf left a comment

Choose a reason for hiding this comment

choldgraf Jun 21, 2022

Choose a reason for hiding this comment

12rambau commented Jun 21, 2022

choldgraf commented Jun 22, 2022

choldgraf commented Jun 22, 2022 • edited

12rambau commented Jun 22, 2022

choldgraf commented Jul 7, 2022 • edited

choldgraf commented Jul 7, 2022 • edited

12rambau commented Jul 7, 2022 • edited

choldgraf commented Jul 7, 2022

12rambau commented Jul 8, 2022

choldgraf commented Jul 8, 2022

12rambau commented Jul 8, 2022

choldgraf commented Jul 8, 2022

tupui commented Jul 8, 2022

drammock commented Jul 8, 2022

choldgraf commented Jul 8, 2022

choldgraf commented Jul 10, 2022

choldgraf commented Jul 10, 2022

gabalafou Oct 31, 2023

Choose a reason for hiding this comment

12rambau Oct 31, 2023

Choose a reason for hiding this comment

12rambau commented Jun 12, 2022 •

edited by choldgraf

choldgraf commented Jun 22, 2022 •

edited

choldgraf commented Jul 7, 2022 •

edited

choldgraf commented Jul 7, 2022 •

edited

12rambau commented Jul 7, 2022 •

edited