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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use different logos for light and dark theme #691

Merged
merged 17 commits into from Jun 5, 2022
Merged

Conversation

12rambau
Copy link
Collaborator

@12rambau 12rambau commented Jun 3, 2022

I refactored navbar-logo.html to support 3 different logo structures (2 logos, 1 logo, no logo). I made the following modifications:

find the link

As the same link will be used whatever the logo, I decided to set a variable at the begining of the file

2 logos

The user set the logos in the defined static folder and set the relative path in the options:

html_static_path = ["_static"]
html_theme_options = {
    "light_logo": "logo-light-mode.png",
    "dark_logo": "logo-dark-mode.png",
}

I then create 2 independant logos, one using only-dark the other only-light.

This option only works if both the logo are set

demo ?

As long as the theme documentation has not yet a logo (please have look in #373) I cannot really demo it here. you can either trust me (not recomended) or test it on one of your doc and let me know 馃槃

Fixes #674 fixes #545

@12rambau 12rambau marked this pull request as ready for review June 3, 2022 21:30
Copy link
Collaborator

@choldgraf choldgraf left a comment

Choose a reason for hiding this comment

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

Nice! A few quick ideas/comments from me but this is looking good!

Copy link
Collaborator

@choldgraf choldgraf left a comment

Choose a reason for hiding this comment

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

This looks pretty good to me! A few quick suggestions on docs wording

@@ -38,3 +38,6 @@ page_sidebar_items = page-toc.html, edit-this-page.html
switcher =
pygment_light_style = tango
pygment_dark_style = native
light_logo =
dark_logo =
html_title =
Copy link
Collaborator

Choose a reason for hiding this comment

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

this isn't necessary because html_title is built into Sphinx, I believe

docs/user_guide/configuring.rst Outdated Show resolved Hide resolved
docs/user_guide/configuring.rst Outdated Show resolved Hide resolved
docs/user_guide/configuring.rst Outdated Show resolved Hide resolved
docs/user_guide/configuring.rst Outdated Show resolved Hide resolved
docs/user_guide/configuring.rst Outdated Show resolved Hide resolved
@jarrodmillman jarrodmillman added this to the 0.9 milestone Jun 4, 2022
12rambau and others added 5 commits June 4, 2022 20:32
Co-authored-by: Chris Holdgraf <choldgraf@gmail.com>
Co-authored-by: Chris Holdgraf <choldgraf@gmail.com>
Co-authored-by: Chris Holdgraf <choldgraf@gmail.com>
Co-authored-by: Chris Holdgraf <choldgraf@gmail.com>
Co-authored-by: Chris Holdgraf <choldgraf@gmail.com>
Copy link
Collaborator

@choldgraf choldgraf left a comment

Choose a reason for hiding this comment

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

Made a few small docs tweaks! This looks good to me! I am going to merge and try this out in #698 !

@choldgraf choldgraf merged commit 9f5f942 into pydata:main Jun 5, 2022
@12rambau
Copy link
Collaborator Author

12rambau commented Jun 5, 2022

I had not removed the html_title from conf 馃槃. I will clean it somewhere else

@12rambau 12rambau deleted the logo branch June 5, 2022 09:34
@choldgraf
Copy link
Collaborator

I removed it just before merge but reintroduced a version of it back into the branding update PR 馃檪

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 a logo for the dark theme Configurable way to insert site title
3 participants