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: White space behavior for main content #753

Merged
merged 7 commits into from
Jun 27, 2022

Conversation

choldgraf
Copy link
Collaborator

@choldgraf choldgraf commented Jun 24, 2022

This is an attempt at making the whitespace behavior more predictable for the general page layout. It does the following things:

  • Removes the practice of adding in empty <div> elements with manually-defined Bootstrap columns if the primary/secondary sidebars are not defined. Instead they will simply not be there at all.
  • Uses justify-content: center which means the sidebars and article content will bunch in the middle of the page
  • This means that if there's no sidebar at all, the content will be directly in the middle.
  • Also removes our custom "check for sidebars and define a bootstrap class for max-width of content" logic, and instead just uses a single max-width rule for our article content
  • Documents our use of :notoc: to hide the secondary sidebar, and also renames this (previously undocumented) variable to be theme_html_remove_secondary_sidebar. Ideally we'd want this to be more like a dict ({"theme_html": {"remove_secondary_sidebar": True}}) but we can't do that with reStructuredText metadata unfortunately.

Example

Here's an example of deleting the primary and secondary sidebars in succession.

chrome_p3JJhuyNHk

In addition, if you wanted the page content to take up all available space, you could add a CSS rule like:

.bd-content {
  max-width: 100%;
  flex-grow: 1;
}

I think that this should result in more predictable behavior for folks like @mwaskom while still looking nice. What do folks think?

Example pages

closes #704 closes #760

@mwaskom
Copy link

mwaskom commented Jun 24, 2022

Hm, it's possible that I've insufficiently rolled back my various customizations, but I removed my custom layout.html, sidebar-primary.html and sidebar-secondary.html templates, and now this is what I am getting:

image

Not sure where that "show source" link is coming from, wasn't there before. I assume that's a conf option. In any case it occurs to me that now I really might want :notoc:, which wasn't sufficiently doing the trick before, and indeed that does get rid of the empty (aside from source link) right sidebar, but the horizontal use of space is still inefficient:

image

@mwaskom
Copy link

mwaskom commented Jun 24, 2022

For comparison, here's where my hacks had gotten me to (this is against your branch, not v0.9.0):

image

@choldgraf
Copy link
Collaborator Author

@mwaskom I think that if you use the max-width and flex-grow CSS rules I mentioned above, then it should grow your content to fill the whole screen.

Would it be helpful if we added a CSS helper class like full-width like:

.full-width {
  max-width: 100%;
  flex-grow: 1;
}

that could be re-used?

(and yep, I think :notoc: is what you want in order to remove the TOC entirely)

@mwaskom
Copy link

mwaskom commented Jun 24, 2022

Oh sorry I missed that was an extra customization step.

you could add a max-width: 100%; flex-grow: 1; CSS rule

Sorry for being dense but add this where?

@choldgraf
Copy link
Collaborator Author

choldgraf commented Jun 24, 2022

In your own custom CSS file, I'd add a selector like:

.bd-content {
  max-width: 100%;
  flex-grow: 1;
}

then if you delete the sidebars the content should expand.

@mwaskom
Copy link

mwaskom commented Jun 25, 2022

Yay! Looking good now!

@12rambau
Copy link
Collaborator

Is there a page in the documentation that is behaving as described in this PR ?

@choldgraf
Copy link
Collaborator Author

Yep, two examples:

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.

Let's make it happen ! it will be very useful!

tests/test_build.py Outdated Show resolved Hide resolved
Co-authored-by: Rambaud Pierrick <12rambau@users.noreply.github.com>
@choldgraf
Copy link
Collaborator Author

@bryevdv would love feedback on whether this would resolve your suggestion in #760 !

@mwaskom
Copy link

mwaskom commented Jun 27, 2022

The landing page now has both sidebars turned off

Not sure if it's related to this change, but note that the version switcher position here is pretty awkward:

image

@choldgraf
Copy link
Collaborator Author

@mwaskom ah yeah this is on main too, so I opened an issue to track it:

@choldgraf
Copy link
Collaborator Author

OK let's merge this one in so that @12rambau can iterate on #726

@choldgraf choldgraf merged commit db0d73d into pydata:main Jun 27, 2022
@choldgraf choldgraf deleted the enh-spacing branch June 27, 2022 14:15
@bryevdv
Copy link
Contributor

bryevdv commented Jun 27, 2022

@choldgraf This seems promising, will definitely check it out when it's released

@mwaskom
Copy link

mwaskom commented Jun 27, 2022

Thanks @choldgraf! I’m delighted to learn how effective it can be to bully you on twitter 😝

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.

Support for "full width" pages Empty space allocated for right sidebar when not used
5 participants