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

Adaptive SVG logos look terrible on dark themes #91653

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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