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 search field back to sidebar #775

Merged
merged 17 commits into from
Nov 14, 2023

Conversation

choldgraf
Copy link
Member

@choldgraf choldgraf commented Nov 8, 2023

This makes minor CSS adjustments to fix some changes made in the latest pydata theme update.

Brief summary:

  • Changes the default behavior of our search component. The icon button will show up on narrow screens, and the search field button will show up in the sidebar on wider screens.
  • Re-applies the same primary/secondary colors to keep this consistent. It uses a slightly lighter color on dark mode to meet a11y contrast requirements (I used this little tool for this).
  • De-saturates the background on dark mode (cc @Gouvernathor)
  • Hides the "back to top" button on wide screens when the TOC of the page is present to avoid imposing it on the content.
  • Standardizes the size of header buttons that use SVGs
  • Fixes a regression in Make article width take up whole page in absence of secondary sidebar #771 for content width on narrow screens

Issues resolved:

I think this is ready to merge, the two failures aren't related to this PR...

@choldgraf
Copy link
Member Author

choldgraf commented Nov 9, 2023

@Gouvernathor I've got some changes in this one that are relevant to you, if you want to take a look. Let me know if this is a reasonable step forward to avoid unexpected CSS changes.

I think we do want to incorporate improved accessibility, but hopefully this PR does so without changing the defaults as much. I don't want to get into a long debate about minor design details, so please focus your comments on anything really major that is a huge problem as opposed to stuff that might be a matter of preference! (agreeing on design is always a huge challenge haha)

@Gouvernathor
Copy link

Gouvernathor commented Nov 10, 2023

That sounds great, how can I test how it renders ? I tried editing the .scss files from my local install but it doesn't seem to work.

@choldgraf
Copy link
Member Author

I think for now your best bet is to look at the rendered documentation preview from this PR.

https://sphinx-book-theme--775.org.readthedocs.build/en/775/

It will be way easier to just check that out and estimate how it would look rather than to build your docs locally with this PR.

@Gouvernathor
Copy link

Gouvernathor commented Nov 10, 2023

Thanks ! That's more than enough.

As for the background color, I would prefer the former color, #121212, which is visually much darker than this new one.
An even darker one would be acceptable too, for me, but a simple rollback to the previous color (which is still part of the version of book that's considered "stable") may be more acceptable for users at-large who would see no change at all.

The "back to top" and research bar changes are great.
If the initiative of making the "back to top" toggleable on the base theme gets merged, it would probably be cleaner to just do that instead, though that's more of an internal concern, and it doesn't prevent merging this in the meantime.

@choldgraf
Copy link
Member Author

Fair enough - I think keeping the old background color is less messy anyway. I'd rather not change something if we don't have a clear rationale for doing so. I've reverted that back in the last commit

@choldgraf
Copy link
Member Author

choldgraf commented Nov 14, 2023

OK going to merge this one in. I think it's a good enough improvement and is another step to unblocking folks with all the upstream updates.

@choldgraf choldgraf merged commit 3642d42 into executablebooks:master Nov 14, 2023
13 of 15 checks passed
@choldgraf choldgraf deleted the pydata-style branch November 14, 2023 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants