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

Initial working version of theme #5

Merged
merged 33 commits into from
Jun 7, 2024
Merged

Initial working version of theme #5

merged 33 commits into from
Jun 7, 2024

Conversation

tillywoodfield
Copy link
Contributor

@tillywoodfield tillywoodfield commented May 30, 2024

Depends on IATI/design-system#6

  • Use reusable styles from IATI design system
  • Add some Sphinx-specific styling
  • Minor fixes of documentation warnings (for kitchen sink)

@tillywoodfield tillywoodfield marked this pull request as ready for review June 4, 2024 11:23
@robredpath
Copy link
Contributor

This is looking great, @tillywoodfield !

Things to fix before merging

  • The menu disappears when shrinking my browser to a very thin width, but I don't seem to get a menu-burger or any other way of navigating appearing
  • The second and third lines of the Really Long Page Title.... look like more menu links. Could we either increase the line spacing (as the RTD theme does) or use the IATI website menu (which I don't like and want to replace at some point, but is consistent)?
  • We should get rid of the Previous / Next page links because they're not relevant to the type of content we have
  • Until we get proper headers, I think the IATI logo should always link to iatistandard.org and the title block (e.g. "IATI Sphinx Theme") should link to the docs home
  • I'd prefer the header to look more like the Validator header, with the teal and graphic behind a top menu block, so that it's a bit slimmer and more consistent. But, we'll expect to be replace it soon, so no worries for now if it's hard.
  • The footer needs the Contact and Privacy Policy links.

Things to fix promptly after merging

  • The API Documentation bits of the kitchen sink don't render properly
  • I'd like a symbol other than ¶ for the anchor targets (maybe #?) and for them to be smaller.
  • We're going to need a language switcher
  • I'm not sure what it is, but I find the underlined links really intrusive, and I'm not sure why (even though I know it's good practice). Can we make the underlines a different colour? Is that something you can do?
  • The orange and red admonitions are very similar in colour - we should either make them the same, or differentiate them more

@robredpath
Copy link
Contributor

@tillywoodfield Also - just talked to @mollieodsc and she's asking whether it's the right font? It might be the logo font, but not the content font.

@mollieodsc
Copy link

@tillywoodfield @robredpath thanks Rob! Tilly, this is great! I think the the font for headings on the sphinx theme is avenir, and it looks like the iatistandard.org uses pragmatica. But really nice to see this coming together, thanks for your work on it!

@tillywoodfield
Copy link
Contributor Author

@robredpath thank you, I'm working through your feedback points currently, and thanks @mollieodsc for pointing out the fonts, I had copied the ones from the Validator, but will update to the proper ones :)

@Bjwebb
Copy link

Bjwebb commented Jun 5, 2024

We're going to need a language switcher

Once languages are set up properly on read the docs, this should appear in the read the docs menu (see this example). But, we may want a more prominent language switcher too.

@tillywoodfield
Copy link
Contributor Author

@robredpath I've made updates for most of the comments, you might need to do a cache clear/hard reload on the RTD PR build to see it all properly. The main outstanding item is collapsing the menu and providing a burger icon on small screens as I need to add some javascript for that, but for now I've placed it at the top of the page instead of disappearing.

@robredpath
Copy link
Contributor

Thanks @tillywoodfield - this is looking great. I don't think that the menu-burger has to block merging now as moving the menu to the top does mean that someone can access the content.

@robredpath
Copy link
Contributor

robredpath commented Jun 7, 2024

Just writing up a couple of points from discussion with @tillywoodfield this morning:

  • The left-hand menu should have a "home" link at the top which goes to the documentation root (I realised after we spoke that the angst I had about the link at the top was irrelevant because this is just for docs, @tillywoodfield!)
  • There shouldn't be a HOME link at the top, next to SEARCH

...and I think that's it for now!

EDIT: I remembered another - let's randomise the admonition colours (avoiding red and orange for warnings/errors) so that we're not using RAG badly

@tillywoodfield
Copy link
Contributor Author

@robredpath I've removed the HOME link from the header. I tried to add a home link to the left contents table, but found that if we hard-code it in Sphinx doesn't add the .current class to the link which allows it to behave the same way as the rest of the links (e.g bold when you are on that page). I think maybe it's best to leave it to the doc sites themselves to add a home link if required (example)

@tillywoodfield
Copy link
Contributor Author

@robredpath I had a play around with the admonition colours but I think that making them random might end up more confusing, so I've set them all to be blue for now, but happy to change it, see what you think

@robredpath
Copy link
Contributor

Thanks @tillywoodfield - that makes sense. I'd still like a couple of different colours, though - can we maybe have Attention be purple, Caution be yellow and Note be green? Leaving the rest blue makes sense - that's a good point about the randomness.

That's fine about the home link for now; we can figure that out later.

@tillywoodfield
Copy link
Contributor Author

@robredpath yep no problem, I've updated those colours

Copy link
Contributor

@robredpath robredpath left a comment

Choose a reason for hiding this comment

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

Amazing work - I'm happy!

@tillywoodfield tillywoodfield merged commit 0194ff2 into main Jun 7, 2024
3 checks passed
@tillywoodfield tillywoodfield deleted the theme branch June 7, 2024 11:57
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.

5 participants