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

Create new Read the docs theme with custom index #2289

Merged
merged 90 commits into from
Jul 27, 2022

Conversation

jessica-mitchell
Copy link
Contributor

This PR introduces a new theme (based on Sphinx Material) for the Read the docs documentation.
It also include a new template for index.html. Thanks to @jougs for developing a more interactive and attractive theme!

The goal is to provide users with different ways to explore the docs, improve the context for the different guides and resources, and enhance the overall theme.

Set as draft as discussion and bit more tweaking is needed. Also #2283 needs to be merged first.

@jessica-mitchell jessica-mitchell added T: Enhancement New functionality, model or documentation S: Normal Handle this with default priority I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Feb 8, 2022
@jessica-mitchell jessica-mitchell marked this pull request as draft February 8, 2022 12:11
@jessica-mitchell
Copy link
Contributor Author

@jougs after our discussion here are the changes I managed (also @heplesser @steffengraber fyi)

  • css

    • Upgraded bootstrap from 3 to 5 - which resolved a lot issues (including bad font sizes and margins, and device scaling issues) that required some hacky css. However, I was unable to get the pulse button positioned and scaled properly so I removed it. I can look at re-adding at some point with fresh eyes

    • Removed unused css (erred on the side of keeping things if unsure)

    • Merged all custom css into one custom.css file and organized the sections to 1) basic / universal properties 2) material design 3) sphinx design 4) custom index.html 5) media queries (include all possible above categories)

    • Defined the nest-colors (orange, green and blue from index.html) as variables in css

  • index.html

    • removed orange links on green banner
    • changed to row of buttons for links into docs (this seems ok)
    • font style is consistent with rest of documentation (with the caveat that the weight is heavier)
    • fade particle image both borders
    • fixed script highlighting
  • specific formatting

    • pynest example pngs now all same size and line up
    • changed how icons are placed in cards; this allows for easier control and consistency. icons are now above the text. If the aesthetics are found to be less pleasing - we can look at changing this (maybe not this PR).
    • matched margins and shadow style in cards and admonitions
    • changed admonition style to look better with orange links

Copy link
Contributor

@steffengraber steffengraber left a comment

Choose a reason for hiding this comment

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

Great, a big step for the documentation.
To improve the mobile view, I have just one small note in custom.css.
A general question: is it necessary to have the full bootstrap stack (js and css) in the code? Or is it not possible to leave some of it out?
One more note: There are also many content adjustments that don't necessarily have anything to do with theme customization. Wouldn't it have been better to put this in another pull request?

@jessica-mitchell
Copy link
Contributor Author

@steffengraber
I adjusted the padding for smaller screens, thanks

A general question: is it necessary to have the full bootstrap stack (js and css) in the code?

I just followed steps from the bootstrap site, and it might be excessive to have it all, perhaps in follow up PR I can look at simplifying this

There are also many content adjustments that don't necessarily have anything to do with theme customization. Wouldn't it have been better to put this in another pull request?

Yah probably, but now it would be very time consuming to review commits, it largely stemmed from trying to get the output to look decent and I probably got carried away with other fixes along the way.

@jessica-mitchell
Copy link
Contributor Author

@heplesser I fixed up the API, including the spacing between elements in the kernel attributes which was mentioned at the NEST conference https://nest-test.readthedocs.io/en/rtd-theme/ref_material/pynest_apis.html

@pnbabu
Copy link
Contributor

pnbabu commented Jun 30, 2022

@jessica-mitchell can we have a link for NESTML on the main menu? Instead of models, we can have something like built-in models and custom models and the latter could point to the NESTML page.

@jessica-mitchell
Copy link
Contributor Author

jessica-mitchell commented Jun 30, 2022

Instead of models, we can have something like built-in models and custom models and the latter could point to the NESTML page.

@pnbabu
I was thinking about create models for NESTML, or maybe customize models?

Copy link
Contributor

@jougs jougs left a comment

Choose a reason for hiding this comment

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

I just had another walk though and here's some things I noticed. I also noticed that some of the things we discussed earlier are fixed now and I like those much better now :-)

  • I agree with @pnbabu and think that "Built-in models" and "Custom models" represent reality much better than just "Models" and "Customize models"
  • When I'm at the index of the developer space, the "Related Projects" button at the top takes me to the maze, as does "Contact us"

@pnbabu
Copy link
Contributor

pnbabu commented Jul 5, 2022

Instead of models, we can have something like built-in models and custom models and the latter could point to the NESTML page.

@pnbabu I was thinking about create models for NESTML, or maybe customize models?

I see that the Customize models link is on the Models page. It is better to have it linked from the main page instead as Built-in and Custom models.

@gtrensch gtrensch self-requested a review July 27, 2022 11:23
Copy link
Contributor

@gtrensch gtrensch left a comment

Choose a reason for hiding this comment

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

Thank you for this extensive contribution! It looks very good to me. 👍

@terhorstd
Copy link
Contributor

This has become a very long PR with more and more changes accumulating. @jougs might have some minor points left, but @gtrensch has re-reviewed and accepted, so I'd merge this now. All lights are green.

If there are remaining points, don't hesitate to open new issues (with smaller scope).

@terhorstd terhorstd merged commit c1cebdc into nest:master Jul 27, 2022
@jessica-mitchell jessica-mitchell deleted the rtd-theme branch April 17, 2024 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Enhancement New functionality, model or documentation
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants