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

Fix several CSS elements and bugs for Sphinx 5 #818

Merged
merged 14 commits into from
Jul 30, 2022

Conversation

choldgraf
Copy link
Collaborator

@choldgraf choldgraf commented Jul 17, 2022

This PR touches the CSS in several different places, with the goal of making sure it works with Sphinx 5 / docutils .18+. It also fixes a few bugs in the process as well as a few DRY improvements by re-using SCSS variables.

This fixes all of the things things mentioned in the issue below, closes #817 .

In addition, there were a few other things I fixed up along the way. They were noticeable when I was testing locally with docutils .18/sphinx 5, but I'm not sure exactly which were due to the version change and which were just things to improve in general, so it's kind-of a mishmash of stuff.

  • Re-use SCSS variables in a few places (e.g. for border radius) and HTML variables in others (e.g. header/caption weight)
  • Improve styling of the sidebar in general to be similar to our code caption blocks, and make other elements lay out properly when next to a sidebar.
  • Removed the box-shadow behind our code blocks, since it wasn't clear what it was accomplishing other than making our shadow a bit wider (but this was invisible in dark mode, and only 1px in light mode).
  • Cleaning up our sphinx-copybutton and sphinx-togglebutton code a bit for their latest releases (removes most of the togglebutton special-casing).
  • Getting our figure captions to center properly

@choldgraf
Copy link
Collaborator Author

I'd appreciate a review from somebody on this, at least at the level of how the visual style looks in the end result. It would be nice to get support for Sphinx 5 in the next release!

@jarrodmillman jarrodmillman added this to the 0.10 milestone Jul 26, 2022
@drammock
Copy link
Collaborator

drammock commented Jul 26, 2022

working on a thorough review. one thing that looks odd so far: the left border color on this code cell:

Screenshot 2022-07-26 at 16-47-24 PyData Library Styles — PyData Theme 0 10 0rc1 documentation

EDIT this is present on main / current dev docs, not specific to sphinx 5 / docutils 0.18

Copy link
Collaborator

@drammock drammock left a comment

Choose a reason for hiding this comment

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

looks good! I didn't spot anything odd-looking in the rendered docs that isn't already that way on main / devdocs. Code changes look reasonable, just a couple small comments.

@choldgraf
Copy link
Collaborator Author

thanks @drammock ! I implemented both suggestions 👍 I like your footnote spacing better :-)

@choldgraf
Copy link
Collaborator Author

Ready for review / merge

I just rebased this one on main and fixed a few bugs that I noticed as a result. I think it'd be helpful if somebody took another look at the resulting CSS in our Read the Docs build to see if there are any obvious CSS bugs introduced.

If we don't notice any, I suggest that we merge this in and make an rc2. @jarrodmillman would you be willing to help with that? :-)

@tupui
Copy link
Contributor

tupui commented Jul 28, 2022

I tested this branch with SciPy and it works great. Only issue I still get is the empty sidebar when there is no element to show.

Screenshot 2022-07-28 at 00 15 24

It's more a problem on small screens as the right panel will be present and empty.
Screenshot 2022-07-28 at 00 16 21
Screenshot 2022-07-28 at 00 16 32

Minor things I saw which I suppose are linked to docutils. Some left-right padding on a button got removed and on Sphinx-Design cards it was adding some horizontal lines around the body.

Screenshot 2022-07-28 at 00 35 36

Screenshot 2022-07-28 at 00 35 59

@tupui
Copy link
Contributor

tupui commented Jul 28, 2022

And something weird is happening upon resizing before it goes to the mobile mode
Screenshot 2022-07-28 at 00 45 22

@choldgraf
Copy link
Collaborator Author

And something weird is happening upon resizing before it goes to the mobile mode

That's what happens when it runs out of horizontal space...not sure how to deal with that yet haha. You could try navbar_align: left to align the navbar items to the left, just by the logo. That should create more space

@drammock
Copy link
Collaborator

And something weird is happening upon resizing before it goes to the mobile mode

This is reported in #845, further discussion of this problem can go there

@choldgraf
Copy link
Collaborator Author

By the way - I welcome others to push changes to this PR branch, I am really busy with a grant deadline for the next few days and would like to get a new release out soon if possible because there are many nice new changes to add!

@choldgraf
Copy link
Collaborator Author

choldgraf commented Jul 30, 2022

OK I think that I fixed the sidebar one in #849

Want to try that out @tupui ?

I also looked at our Sphinx Design page, and don't see any messed up padding or extra lines, so I'm not sure what to do there. Maybe we can track that and follow up in a new issue.

I'm going to merge this, and cut a new RC so that we can have people test out

@choldgraf choldgraf changed the title Fixing several CSS elements and bugs for Sphinx 5 Fix several CSS elements and bugs for Sphinx 5 Jul 30, 2022
@choldgraf choldgraf merged commit 048b09c into pydata:main Jul 30, 2022
@tupui
Copy link
Contributor

tupui commented Jul 30, 2022

@choldgraf I just tried main and I still have in the output this defined bd-sidebar-secondary bd-toc although the TOC is empty.

@choldgraf
Copy link
Collaborator Author

Can you provide a link or open up an issue with a reproducible set of steps? I can't figure out how to reproduce on my side

@tupui
Copy link
Contributor

tupui commented Jul 31, 2022

@choldgraf you can use this PR: scipy/scipy#16660

To build the SciPy and the doc (build should take around 3 mins with 6 cores, then 5 mins for the doc itself):

# once you cloned my branch, create an conda env to install everything
# don't try on windows as it will mostly fail.
mamba create env -f environment.yml
python dev.py doc -j 6  # number of cores

Thanks for looking into this Chris! Let me know if you want me to try something to help.

@choldgraf
Copy link
Collaborator Author

Hmmm you don't have a Read the Docs PR preview to use? I'm sorry but I don't have time to download and build the scipy docs locally, I'm on vacation right now :-/

@tupui
Copy link
Contributor

tupui commented Aug 1, 2022

Oh sure no worries. If it's a build that you need, I can update the current PR I have so that it builds with the RC. All that can wait until you are back. FYI SciPy 1.9 got released this WE.

I will post here the link once the build is done (just started).

@choldgraf
Copy link
Collaborator Author

Ok sounds good - I think we should open a separate issue to track this instead of reusing this PR. Can you do that and provide a link to the reproducible example there?

Do you think this should block the next release if the theme?

@tupui
Copy link
Contributor

tupui commented Aug 2, 2022

Sure, I will open a new issue (see #854). What I am seeing now that is more of an issue for the next release is that build time significantly increased. I noticed this as well locally but thought it was a hiccup at first. But on the CI it seems to be consistently happening (timeout, not even finishing).

@choldgraf
Copy link
Collaborator Author

Can you please open new issues for anything? 🙂

Comment on lines +121 to +131
## Footnotes

Here's one footnote[^1] and another footnote [^2] and a named footenote[^named], symbol [^*].

[^1]: Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar.
[^2]: Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar.
[^named]: Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar.
[^*]: Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar. Foo bar foo bar.

## Version changes

Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't look correct? It is a whole section inserted into the middle of another one (Admonition sidebars). It also restores the Version changes heading that was removed in https://github.com/pydata/pydata-sphinx-theme/pull/836/files#diff-0607a043e4091f4e22ccc62684762b4a6b00d03e638ff096277e7f4dde9cba77L97, but none of its section content.

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 Sphinx 5 / Docutils 0.18+
5 participants