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

‼️ REFACTOR: HTML and CSS restructuring #472

Merged
merged 31 commits into from
Feb 6, 2022

Conversation

choldgraf
Copy link
Member

@choldgraf choldgraf commented Jan 26, 2022

This is a refactor of our HTML and CSS, with the goal of improving a number of the UI/UX issues on the book theme, tightening up our white space and sizing, using a more sensible set of CSS classes that will make the theme behave more predictably, and adding more functionality for the in-page TOC on mobile widths.

This doesn't remove any functionality, though it will likely break any custom CSS rules that were highly dependent on a particular HTML layout.

Major improvements

  • We now use sticky-top to place our sidebar and topbar, rather than fixed, which means it will flow more naturally if things like banners are placed at the top of the page.
  • Sidebar/topbar/toc font is slightly smaller than the main text font to create more space
  • There's more space for the sidebar + toc bar
  • Increased padding for the main content, and improved padding behavior on wide screens + with sidebar collapsed
  • Programmatic definition of our margin size and placement, so that there's less hacky manually-chosen numbers in there
  • The right sidebar is now viewable on mobile, via a button to toggle the toc similarly to the left sidebar
  • The topbar buttons are now divided into two div groups (left and right) and use flexbox to position and center them nicely
  • Re-arranges and modularizes our SCSS structure to follow more best practices
  • Re-arranges our HTML template structure to match more closely the sphinx-basic-ng theme
  • Adds prettier pre-commit hook for our javascript and scss files (we already use black for Python files so this follows the same convention)

Note - this also bumps our pydata-sphinx-theme back down to the 0.7 series. I think 0.8 brought a new version of bootstrap (4.6), and will cause some unexpected behavior, so let's bump that version in an explicit PR.

closes #392
closes #459 (I think?)
checks off the last two boxes in #416
supercedes #471

Todo

  • Cleanup
    • See if there is any other low-hanging fruit to make this theme's structure closer to sphinx-basic-ng
    • Pull out some component-specific SCSS rules into files in the components/ folder
    • standardize the behavior of buttons with a single css class instead of using one-offs
    • Fix print formatting
    • Topbar buttons should be bigger on mobile, and their coloring seems incorrect on mobile. Maybe stop using the bootstrap btn class
  • Docs
  • QA
    • Look at all our reference pages and make sure there are no regressions on mobile/desktop
    • Make sure that we didn't miss any CSS breakages with the latest docutils
    • Tests are passing
    • Make sure the diffs in regression tests don't seem like we've introduced a bug

Demos

Sidebar behavior 🎉

Slack_tohCGXgFs4

cc @jacobtomlinson as this will likely accept the Dask theme since it rearranges a bit of HTML (but will hopefully make it easier to sub-theme!)

@jacobtomlinson
Copy link
Contributor

Thanks for the ping here @choldgraf, these changes look great and as you say should make our lives easier. I've pinned the Dask theme to 0.2.0 for now as this change does break some things for us, but once this is merged and released I'll port our theme over.

@AakashGfude
Copy link
Member

AakashGfude commented Jan 26, 2022

Thanks, @choldgraf for the improvements here and for creating folder structure for the scss code. While we are restructuring scss, I have an opinion about the architecture of scss files being adopted. Personally, I believe, we should have separate stand-alone files for site-wide variables, mixins, and functions, and name the files as such. An architecture pattern outlined in https://sass-guidelin.es/#architecture was one of the inspirations. A boilerplate GitHub repo for the same https://github.com/KittyGiraudel/sass-boilerplate/tree/master/stylesheets/abstracts. What do you think? Will it add value here?

@choldgraf
Copy link
Member Author

@AakashGfude that's a great resource, thanks for sharing. I'll re-work the SCSS to follow this structure, I think it's great to have some kind of objective standard that we shoot for 👍

@choldgraf
Copy link
Member Author

@jacobtomlinson here are some docs on how custom header HTML can be added to the site: https://sphinx-book-theme--472.org.readthedocs.build/en/472/customize/header.html

maybe that's something you could re-use?

@choldgraf choldgraf marked this pull request as ready for review February 2, 2022 04:00
@choldgraf
Copy link
Member Author

choldgraf commented Feb 2, 2022

OK tests are passing (mostly, might need to fix some lighthouse stuff)! It would be great if others would give feedback and give look at the changes. This touches a lot of different parts of the package (thought doesn't fundamentally change its functionality). Can anybody spot some more low-hanging fruit?

@choldgraf
Copy link
Member Author

Update from meeting

We discussed this PR in our EBP meeting today and people generally thought that this was a nice improvement. I'll leave it open a bit longer in case others have any thoughts or suggestions they'd like to make!

@choldgraf
Copy link
Member Author

I'm going to plan on merging this one in later today, unless somebody suggests we should hold off. We can continue iterating on the structure, but since this one touches so many parts of the project I think it'll be easier for us to wrap our heads around things if we can do it in more iterative steps.

@choldgraf choldgraf changed the title [WIP] REFACTOR: HTML and CSS restructuring REFACTOR: HTML and CSS restructuring Feb 4, 2022
@choldgraf choldgraf changed the title REFACTOR: HTML and CSS restructuring ‼️ REFACTOR: HTML and CSS restructuring Feb 4, 2022
@mathbunnyru
Copy link

@choldgraf let's merge this? :)

@choldgraf
Copy link
Member Author

Will try to get to this today - ended up with a sick toddler at home so wasn't as productive as I hoped yesterday 😬

@mathbunnyru
Copy link

Oh, sorry for bothering you.
I wish your toddler to get well!

@choldgraf
Copy link
Member Author

choldgraf commented Feb 5, 2022

OK I made a few last-second docs updates and minor updates to CSS. I think there are still some things to figure out with the extensions integration, but figured it'd be better to merge this in so that we can iterate on those issues in smaller PRs that are easier to review. I opened up #476 to track that ongoing work.

This is a big change so we'll probably make a pre-release eventually, but it'd be great if in the meantime folks tried out master (cc @jacobtomlinson and maybe @mathbunnyru would be willing to as well?)

(once tests pass I'll merge, but right now github actions isn't happy)

@choldgraf choldgraf merged commit 910e26b into executablebooks:master Feb 6, 2022
@choldgraf choldgraf deleted the REFACTOR-html-css branch February 6, 2022 01:16
@mathbunnyru
Copy link

Anyone who interested to try this out, if you use requirements.txt, just add git+https://github.com/executablebooks/sphinx-book-theme@master#egg=sphinx-book-theme.

I've tried this and it looks great!
https://jupyter-docker-stacks--1602.org.readthedocs.build/en/1602/using/specifics.html

The feature from docutils 17.0 works well!

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.

Left ToC too narrow for nested sections and left body margin too small ENH: Support docutils 0.17.1
5 participants