Skip to content

add support for themed img #1445

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

Merged
merged 1 commit into from
Dec 8, 2021
Merged

add support for themed img #1445

merged 1 commit into from
Dec 8, 2021

Conversation

yordis
Copy link
Contributor

@yordis yordis commented Dec 6, 2021

closes #1405

@yordis
Copy link
Contributor Author

yordis commented Dec 6, 2021

@josevalim how do you test these types of things locally? I pointed my project to use ex_doc with path: "..." config, but I may be doing something wrong since I don't see the code.

Is that the way to go?

@jonatanklosko
Copy link
Member

@yordis you can run mix docs directly in the ex_doc repo and then open doc/index.html. README is included in the docs, so you can temporarily put content there to verify if it is rendered as expected.

@yordis
Copy link
Contributor Author

yordis commented Dec 7, 2021

@jonatanklosko yeah, I think I figured it out, it works

Screen.Recording.2021-12-06.at.7.06.45.PM.mov

@yordis
Copy link
Contributor Author

yordis commented Dec 7, 2021

Last-mile, dealing with the epub

Screen.Recording.2021-12-06.at.7.10.10.PM.mov

@@ -165,6 +165,10 @@ img {
max-width: 100%;
}

img[src*="#gh-dark-mode-only"] {
Copy link
Member

Choose a reason for hiding this comment

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

Alternatively we could do:

body:not(.night-mode) img[src*="#gh-dark-mode-only"] {
  display: none;
}

This way we don't need to explicitly do display: inline to revert this, which is good because the image may potentially have different display.

Maybe it would make sense to have both of these together as body:not(.night-mode) img and body.night-mode img in a single file?

Copy link
Contributor Author

@yordis yordis Dec 7, 2021

Choose a reason for hiding this comment

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

Maybe it would make sense to have both of these together as body:not(.night-mode) img and body.night-mode img in a single file?

I am following the existing pattern. What would you propose? Dont have any preference.

Worth noting that this is only inside the .content-inner

@josevalim
Copy link
Member

For EPUB we should always hide the dark mode one.

@yordis
Copy link
Contributor Author

yordis commented Dec 8, 2021

@josevalim is there a CSS or something? Sorry, I actually don't know how to do that, I googled about it, but I am getting lost the more I click links.

@jonatanklosko
Copy link
Member

@yordis there are separate styles for EPUB in assets/less/epub.less :)

@yordis yordis marked this pull request as ready for review December 8, 2021 00:46
@yordis
Copy link
Contributor Author

yordis commented Dec 8, 2021

Thank you all (specially you @jonatanklosko ❤️ ) ready to CR

@@ -13,3 +13,7 @@
@import './content/footer';
@import './content/bottom-actions.less';
}

body:not(.night-mode) .content-inner img[src*="#gh-dark-mode-only"] {
Copy link
Member

Choose a reason for hiding this comment

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

Actually now I wonder if we can do it in general.less with a fancy selector like body:not(.night-mode) & img[src*="#gh-dark-mode-only"], but either is fine ^^

Copy link
Member

@jonatanklosko jonatanklosko left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@josevalim josevalim merged commit e1da25c into elixir-lang:main Dec 8, 2021
@josevalim
Copy link
Member

💚 💙 💜 💛 ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Some documentation assets don't look great in darkmode
3 participants