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

ENH: Add section title to sidebar navigation #895

Merged
merged 3 commits into from Aug 29, 2022

Conversation

choldgraf
Copy link
Collaborator

@choldgraf choldgraf commented Aug 28, 2022

This cleans up some of our sidebar navigation code, so that it is structured more compactly in our theme, and so that there are similar navigation headers for both the site and section navigation. Here's a quick summary:

  • It moves the Site Navigation title to be inside of our site header navigation component. This is invisible in widescreen, but is visible on narrow screens when it shows up in the sidebar.
  • It adds a Section Navigation title to the "within section" navigation, so it's easy to distinguish the site/section navigation.
  • Cleans up a little bit of the CSS spacing, closes Fix slightly unequal white-space for site- and section- navigation on mobile sidebar #886

For example, note the "site navigation" and (new) "section navigation" headers:

image

Copy link
Collaborator

@12rambau 12rambau left a comment

Choose a reason for hiding this comment

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

I really like the modification !

I was thinking, do you think we should add an extra bottom margin to the version switcher on small screen.
I think a small breath before the "navigation" title could improve readability.
Capture d’écran 2022-08-28 à 14 18 50

@choldgraf
Copy link
Collaborator Author

Agree re: extra whitespace for the version switcher. Want to propose something you think makes it look better?

@12rambau
Copy link
Collaborator

Agree re: extra whitespace for the version switcher. Want to propose something you think makes it look better?

I was not able to open a PR on your branch (I don't know why it wasn't working) but as it's a minimal change I took the liberty of pushing it. Feel free to modify everything if needed

@choldgraf
Copy link
Collaborator Author

No problem @12rambau you are always welcome to push directly to my branches in a PR!

Copy link
Collaborator

@12rambau 12rambau left a comment

Choose a reason for hiding this comment

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

LGTM

@choldgraf choldgraf added this to the 0.10 milestone Aug 29, 2022
@choldgraf
Copy link
Collaborator Author

I think this is ready to go. The failing test is because of the pandas pre-release job failing but not related to this PR. I opened an issue to suggest a path forward here:

@jarrodmillman jarrodmillman merged commit ce5553b into pydata:main Aug 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix slightly unequal white-space for site- and section- navigation on mobile sidebar
3 participants