Collapsible sidebar #2159

gabalafou · 2025-03-07T04:57:44Z

Screenshot showing vertical and horizontal alignment guidelines:

gabalafou · 2025-03-07T14:45:19Z

I made the following accessibility checks:

No animation if user has set prefers-reduced-motion: reduce
Screen reader can read both expanded and collapsed state of button
Focus rings and keyboard functionality work
Minimum 24 x 24 px hit area for mouse click

gabalafou · 2025-03-11T20:06:12Z

Update: I have incorporated feedback from @smeragoel. The only thing left to do with this PR is to add a bare minimum of tests, and then I will mark it ready for review. I will not be able to get to writing tests until next week.

dprada · 2025-03-17T20:27:31Z

I am really looking forward to this implementation. Thanks @gabalafou and @smeragoel. 👏🏻

drammock · 2025-03-18T11:43:36Z

Update: I have incorporated feedback from @smeragoel.

Note to whoever merges this: please add @smeragoel as co-authored-by

gabalafou · 2025-03-21T22:56:32Z

Okay, I added tests. This is now ready for review.

trallard

Thanks for working on this @gabalafou and thanks for adding the tests.

I did not spot anything major, but left a message about reusing existing test sites to try and avoid adding too many sites, especially since we already have one for sidebars testing.

docs/user_guide/layout.rst

src/pydata_sphinx_theme/assets/styles/pydata-sphinx-theme.scss

src/pydata_sphinx_theme/theme/pydata_sphinx_theme/components/sidebar-collapse.html

tests/sites/collapse_sidebar_button/Makefile

gabalafou · 2025-03-24T20:36:44Z

@trallard thank you for review! I think I responded and addressed all of your comments. The PR now needs another round of review and then hopefully it will be good to go! :)

gabalafou · 2025-03-24T20:58:06Z

About the failing link check

The link check is expected to fail because there is a page in the docs that contains links to each of the component files in GitHub. This PR introduces a new component: sidebar-collapse. But the GitHub URL to this new component will not be live until this PR is merged.

But there was one unexpected thing. The link check reports several gnu.org links as broken. But I suspect those might just have been a network fluke, because when I go to the links, they work. The error message for all four of those links looks roughly the same:

HTTPConnectionPool(host='www.gnu.org', port=80): 
  Max retries exceeded with url: /software/gettext/ 
  (Caused by 
    NewConnectionError(
      '<urllib3.connection.HTTPConnection object at 0x7fc13020ae70>: 
      Failed to establish a new connection: [Errno 101] Network is unreachable'
    )
  )

drammock · 2025-03-24T21:52:58Z

The link check reports several gnu.org links as broken.

those have been failing regularly for over a week now at least. Currently 🙈 but hopefully fixed by #2174

Co-authored-by: Tania Allard <[email protected]>

trallard

Thanks @gabalafou 🚀 this is a rather exciting new feature.
Will wait for the CI to complete and we can merge this.

docs/user_guide/layout.rst

trallard · 2025-03-25T12:09:09Z

tests/sites/sidebars/section2/no-sidebar.rst

Thanks for making the configuration explicit 🌻

trallard · 2025-03-25T12:19:57Z

The link check reports several gnu.org links as broken.

those have been failing regularly for over a week now at least. Currently 🙈 but hopefully fixed by #2174

These fail only on CI (but not always), and they might be related to some robots.txt or something else, as link check uses requests under the hood.
I might add the gnu URLS to the list of skipped URL checks to avoid the flaky errors.

That aside, this PR looks ready to merge so will wait for the rest of the CI to complete and merge it 🚀

pllim · 2025-03-25T12:27:04Z

Ooo thanks! What release will this be in?

trallard · 2025-03-25T12:52:52Z

Should be in the next release @pllim (still to decide when this will be out #2176)

gabalafou · 2025-03-25T13:31:58Z

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

  border-right: 1px solid var(--pst-color-border);
  background-color: var(--pst-color-background);
-  overflow-y: auto;
+  overflow: hidden auto;


Note to self: this is the one change that I worry about having unintended consequences. I don't see any reason why the primary sidebar should scroll in the horizontal direction and show a horizontal scroll bar, but I may not be aware of some sites or configurations that need the primary sidebar to scroll along the x axis.

(The reason why I don't think it needs horizontal scroll is that the section table of contents is configured to reflow, and ethical ads are also responsive.)

This can happend when creating API documentation. By design sphinx will use the full path to the method/class you want to describe which can become very very long if unchecked. I have created an example myself. I faced the problem when I was doing this: https://geetools.readthedocs.io/en/stable/autoapi/geetools/index.html and I was forced to change the template which is not straightfoward for new users. I know the title can be responsive as well but I find it easier to understand when things remain on 1 single line (at least for API names)

True, this is one such instance where we'd have horizontal overflow.
@12rambau would it be possible to see how the new feature would render for your API docs? I am curious on whether we will be hitting some unexpected behaviours.

I'm currently travelling with only my phone so I cannot really make checks outside of the github realm and building this documentation requires a new release from my side. I'll let you know once I'm back home and I can locally do a test

No rush at all. Thanks for considering.

By design sphinx will use the full path to the method/class you want to describe which can become very very long if unchecked.

Yeah, we can see that on the PST API docs page with the long method name pydata_sphinx_theme.edit_this_page.

I believe the alternative would be to force the method name on one line which, if the primary sidebar is narrow enough, will cause part of the string to be hidden on the right side (in the sidebar's overflow). That means the user will have to scroll horizontally in order to see the rest of the name. That feels to me like it makes the table of contents even harder to absorb visually than if the method name is broken across two or more lines.

Here are two screenshots to show the difference.

No wrapping

With the page a bit narrow and text wrapping turned off, the user can only make out "edit this pa":

With wrapping

With the page a bit narrow and with text wrapping enabled, the user can see the entire method name:

I like the text wrap even if it looks a bit chunky at least you see everything.

I think the wrapping is fine, I do not expect this to be a very frequent situation, and if it causes problems we can reevaluate/iterate.

gabalafou · 2025-04-11T21:08:48Z

I came back to this PR because I suddenly thought I had forgotten to include @smeragoel on the git commit co-authors, but then I realized that I didn't do the actual merge for this PR (@trallard did). Then I looked at the commit and saw the following at the bottom and it made me happy:

Co-authored-by: Daniel McCloy <[email protected]>
Co-authored-by: Tania Allard <[email protected]>
Co-authored-by: Smera Goel <[email protected]>

Thanks @drammock, @trallard, @smeragoel for all of your help, you are a great team!

🌻

gabalafou force-pushed the 1072-sidebar-collapse-widescreen branch from 2a0e7c0 to 817ec63 Compare March 10, 2025 18:05

trallard added kind: enhancement New feature or request tag: component Issues or improvements associated with a given component in the theme labels Mar 12, 2025

gabalafou added 10 commits March 21, 2025 14:03

Collapsible sidebar

67bcf0d

Make the collapse button into a normal sidebar template

8e06fd7

make the expand icon bigger

1d4ce53

fix alignment

7f704da

Unset padding bottom

4f8fdaf

bigger icon

5bd0d5b

do not show collapse button if sidebar is empty

53dafcb

check for early return condition at beginning of handler

105ada3

icon height 1.3em

4d4dcbb

add tests

b766196

gabalafou force-pushed the 1072-sidebar-collapse-widescreen branch from 92186ce to b766196 Compare March 21, 2025 22:56

gabalafou marked this pull request as ready for review March 21, 2025 22:56

revert inadvertent change committed

55b96d1

trallard previously approved these changes Mar 24, 2025

View reviewed changes

alphabetic imports

fd74f7d

gabalafou dismissed trallard’s stale review via fd74f7d March 24, 2025 19:30

gabalafou added 3 commits March 24, 2025 14:22

reuse sidebars test site

dbddabb

update translations

3a7878d

oops missed ja language

a97cff9

Apply suggestions from code review

624e80f

Co-authored-by: Tania Allard <[email protected]>

drammock approved these changes Mar 24, 2025

View reviewed changes

Merge branch 'main' into 1072-sidebar-collapse-widescreen

2260af2

trallard approved these changes Mar 25, 2025

View reviewed changes

trallard mentioned this pull request Mar 25, 2025

DISCUSS - 0.17.0 Release #2176

Open

trallard merged commit 21a1870 into pydata:main Mar 25, 2025
30 of 31 checks passed

gabalafou deleted the 1072-sidebar-collapse-widescreen branch March 25, 2025 13:26

gabalafou commented Mar 25, 2025

View reviewed changes

drammock mentioned this pull request Mar 26, 2025

v0.16.0 breaks left sidebar toggling (STB) #2067

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Collapsible sidebar #2159

Collapsible sidebar #2159

gabalafou commented Mar 7, 2025 •

edited

Loading

gabalafou commented Mar 7, 2025

gabalafou commented Mar 11, 2025

dprada commented Mar 17, 2025

drammock commented Mar 18, 2025

gabalafou commented Mar 21, 2025

trallard left a comment

gabalafou commented Mar 24, 2025

gabalafou commented Mar 24, 2025 •

edited

Loading

drammock commented Mar 24, 2025

trallard left a comment

trallard Mar 25, 2025

trallard commented Mar 25, 2025 •

edited by drammock

Loading

pllim commented Mar 25, 2025

trallard commented Mar 25, 2025

gabalafou Mar 25, 2025 •

edited

Loading

gabalafou Mar 25, 2025 •

edited

Loading

12rambau Mar 25, 2025

trallard Mar 26, 2025

12rambau Mar 26, 2025

trallard Mar 26, 2025

gabalafou Apr 2, 2025

12rambau Apr 14, 2025

trallard Apr 14, 2025

gabalafou commented Apr 11, 2025

Collapsible sidebar #2159

Collapsible sidebar #2159

Conversation

gabalafou commented Mar 7, 2025 • edited Loading

gabalafou commented Mar 7, 2025

gabalafou commented Mar 11, 2025

dprada commented Mar 17, 2025

drammock commented Mar 18, 2025

gabalafou commented Mar 21, 2025

trallard left a comment

Choose a reason for hiding this comment

gabalafou commented Mar 24, 2025

gabalafou commented Mar 24, 2025 • edited Loading

About the failing link check

drammock commented Mar 24, 2025

trallard left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trallard commented Mar 25, 2025 • edited by drammock Loading

pllim commented Mar 25, 2025

trallard commented Mar 25, 2025

gabalafou Mar 25, 2025 • edited Loading

Choose a reason for hiding this comment

gabalafou Mar 25, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

No wrapping

With wrapping

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabalafou commented Apr 11, 2025

gabalafou commented Mar 7, 2025 •

edited

Loading

gabalafou commented Mar 24, 2025 •

edited

Loading

trallard commented Mar 25, 2025 •

edited by drammock

Loading

gabalafou Mar 25, 2025 •

edited

Loading

gabalafou Mar 25, 2025 •

edited

Loading