Skip to content

Adaptive SVG logos look terrible on dark themes #91653

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
newpavlov opened this issue Dec 8, 2021 · 4 comments · Fixed by #91958
Closed

Adaptive SVG logos look terrible on dark themes #91653

newpavlov opened this issue Dec 8, 2021 · 4 comments · Fixed by #91958
Assignees
Labels
A-rustdoc-themes Area: Themes for HTML pages generated by rustdoc A-rustdoc-ui Area: Rustdoc UI (generated HTML) T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@newpavlov
Copy link
Contributor

In RustCrypto crates we use SVG logo which changes style depending on preferred theme using @media(prefers-color-scheme:dark) { .. } CSS rules. You can see the sha2 docs for an example which uses it.

But the problem is how the logo gets rendered on dark themes:
ayu
dark

The reason for such horrible result is the white shadow filter applied to the logo. I understand why it was added, but I think it's a wrong approach, especially for projects which aim to properly support dark themes in their media files.

@Nemo157 Nemo157 transferred this issue from rust-lang/docs.rs Dec 8, 2021
@Nemo157 Nemo157 added A-rustdoc-themes Area: Themes for HTML pages generated by rustdoc A-rustdoc-ui Area: Rustdoc UI (generated HTML) labels Dec 8, 2021
@Nemo157
Copy link
Member

Nemo157 commented Dec 8, 2021

There was a mention back when this was implemented about how it is not possible for a media query approach to work, when I look at the sha2 docs it looks like

image

because I use a light theme generally in my browser, but have rustdoc explicitly set to a dark theme.

I'm confused why the outline is being applied to these docs though, #75249 was meant to only apply it to the default rust logo, and I don't see any relevant newer PRs when searching for logo.

@camelid camelid added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Dec 13, 2021
@jsha
Copy link
Contributor

jsha commented Dec 15, 2021

I believe I broke this in https://github.com/rust-lang/rust/pull/86157/files#diff-0bed4892d95534279b3195c75733913b7735505ae95b8d3d65d63e9ce6a06c1cL144-L159. In #75249 we are supposed to apply the rust-logo class only when the logo is the default one. In #86157 I missed that distinction and now the <img> always gets the rust-logo class, even when it's a custom logo. I'll fix.

@jsha
Copy link
Contributor

jsha commented Dec 15, 2021

By the way, to confirm what @camelid said: the media query embedded in the SVG isn't really right. That gets the browser's preferred light/dark mode, but the user may have overridden that in rustdoc settings.

As a concrete example, I'm using Chrome on Linux, which doesn't have a light/dark mode setting. I can select a dark theme in rustdoc settings, which will get me this for the Rust Crypto logo (with #91958 applied):

image

That's still an improvement over the status quo, and overall I think it's fine. But if we really wanted to support theme-sensitive logos perfectly, I think we'd need to offer separate configuration options for a light logo and a dark logo.

@newpavlov
Copy link
Contributor Author

newpavlov commented Dec 15, 2021

I believe that the media query approach is a correct one in principle. But I agree that, unfortunately, the current state of browsers is far from ideal, e.g. it would be nice to have a way to switch preferred mode for a page using CSS or JS.

I'm using Chrome on Linux, which doesn't have a light/dark mode setting

Well, it kind of has. Try launching it with chromium-browser --force-dark-mode (I don't have Chrome installed, but I think it should work similarly).

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-rustdoc-themes Area: Themes for HTML pages generated by rustdoc A-rustdoc-ui Area: Rustdoc UI (generated HTML) T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants