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

DOC: Document helper for dark mode #626

Merged
merged 3 commits into from Apr 20, 2022
Merged

Conversation

larsoner
Copy link
Contributor

After #540, the first thing I needed to do was to make the main MNE-Python logo responsive. This adds a hint to people for how to do it. (It took me hours because I'm bad at JS!)

FYI I tried the <picture><srcset ...> approach where you look at if it prefers dark to light, but this was not responsive to theme changes.

@larsoner
Copy link
Contributor Author

@12rambau you might have other better ideas here, too!

@choldgraf
Copy link
Collaborator

This is a nice idea! I made a few edits and cleaned up this section in general in the latest commit, what do you think?

@12rambau
Copy link
Collaborator

@larsoner thanks for the editing of the doc, I was not super confident with what was written there and I think it' much better.

With regards to the images, It's a tricky subject. On my documentation I decided to continue using directives so that the other builds (epub and pdf) continue to work but I was forced to inject custom js to make the switch between light and dark versions of the images.

Do you think it should be written as well ? (the fact that using pure HTML will prevent pdf build of your images)

@larsoner
Copy link
Contributor Author

On my documentation I decided to continue using directives... Do you think it should be written as well ? (the fact that using pure HTML will prevent pdf build of your images)

I did the same in the end actually. The <img tag was actually just a simplification to make a point. In practice I actually don't use pure HTML... I use a standard sphinx image directive, give it a mainlogo class, and use document.getElementsByClassName('mainlogo')[0] to access it:

https://github.com/mne-tools/mne-python/blame/main/doc/index.rst#L20-L29

This seemed unnecessarily complicated as I figured people would use the docs as a minimal example of how to tweak the HTML (and the getElementsByClassName seemed less minimal), but I could change the example to use this instead if you think it would help people realize they don't have to abandon a standard directive here!

@larsoner
Copy link
Contributor Author

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.

I like those new changes! This LGTM. I'll hold off on merging for a little bit in case @12rambau would like changes!

Copy link
Collaborator

@damianavila damianavila left a comment

Choose a reason for hiding this comment

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

LGTM as well!

@damianavila
Copy link
Collaborator

@12rambau, any thoughts before merging it?

Copy link
Collaborator

@12rambau 12rambau left a comment

Choose a reason for hiding this comment

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

nothing to add from my side

@choldgraf choldgraf merged commit 76f0101 into pydata:master Apr 20, 2022
@choldgraf
Copy link
Collaborator

Many thanks @larsoner !!

@larsoner larsoner deleted the darkdoc branch April 20, 2022 14:09
@jarrodmillman jarrodmillman added this to the 0.9 milestone Jun 2, 2022
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.

None yet

5 participants