Improve doc theme logo display #74361

GuillaumeGomez · 2020-07-15T12:15:22Z

The first commit cleans up the whitespaces and converts them to tabs. We should definitely write a tidy check for this (will do it in another PR).

Screenshots:

r? @lzutao
cc @Cldfire

tesuji · 2020-07-15T12:31:00Z

src/librustdoc/html/static/themes/ayu.css

@@ -61,6 +61,10 @@ pre {
 	background-color: #14191f;
 }

+.logo-container > img {
+	filter: drop-shadow(0 0 5px #fff);


Do we have license agreement from @UtherII ? (Is he/she the same person here: https://www.reddit.com/r/rust/comments/hrfi8k/ayu_theme_is_now_available_on_nightly_rustdoc/fy4dzum/ ?).
The POC doesn't have any license files: /~https://github.com/UtherII/poc_rustdoc .

Yes I am. Do you want me to add a comment? It seems a bit too much for just a line of CSS but I can add it... Also, they posted it in a reddit channel, I tested it. So not sure what are the rules in this case too...

I am not a lawyer too. I'm just afraid legal issues when UtherII doesn't explicitly grant us the permissions to copy the code.

That seems to be a bit of long stretch considering the code was provided in an open discussion but if someone knows about it...

No problem. You can use it.
I will put a license on my POC.

Thanks a lot! Still very interested in what you'll come up with, with your own POC. :)

tesuji · 2020-07-15T12:39:06Z

Also, is drop-shadow compatible with other browses (and old version of IE)? As I only find its doc in MDN.
The other place is https://www.w3.org/TR/filter-effects/#dropshadowEquivalent (December 2018).

Cldfire · 2020-07-15T12:40:16Z

I think this works great 👍

tesuji · 2020-07-15T12:43:18Z

I don't have much experience with front-end things so r? @ollie27

GuillaumeGomez · 2020-07-15T12:48:07Z

Also, is drop-shadow compatible with other browses (and old version of IE)? As I only find its doc in MDN.
The other place is https://www.w3.org/TR/filter-effects/#dropshadowEquivalent (December 2018).

https://caniuse.com/#search=drop-shadow

So not a bad support, but not the best. However, there doesn't seem to be an older equivalent unfortunately...

GuillaumeGomez · 2020-07-15T13:58:10Z

r? @kinnison

pickfire · 2020-07-15T15:48:27Z

@GuillaumeGomez Is our plan incorporate all the themes from mdbook (light, rust, coal, ayu, navy) here? I personally quite like the themes from mdbook. I personally use rust and coal theme.

But still, I do prefer that the browser detects my current theme (prefer-color-scheme: dark). My phone switches between dark and light mode depending on the time.

GuillaumeGomez · 2020-07-15T16:11:31Z

No, it's not scheduled or anything, and I'd prefer to keep the count low if possible. It's quite a lot of maintenance after all... Any change would has to be done in all themes so I'm not too favourable to grow the default rustdoc themes much more than this.

However, it can be added either locally or on docs.rs (if they allow it of course, same issue as here).

XAMPPRocky · 2020-07-15T17:49:55Z

An alternative to this would be to provide a white version of the logo in dark color schemes. The SVG for the website's favicon already has CSS embedded inside it to do this, so it should be as simple as using this one instead. You can read rust-lang/www.rust-lang.org#1185 for more info.

GuillaumeGomez · 2020-07-15T18:40:16Z

@XAMPPRocky This can't be a solution because crates can provide their own images. You can't expect crates' owners to provide multiple images for one logo, it's just not viable. Adding a contrast allows to remove this issue.

XAMPPRocky · 2020-07-15T18:48:32Z

@GuillaumeGomez I'm not sure I was clear. It's a single SVG file with CSS embedded inside it.

GuillaumeGomez · 2020-07-15T18:50:56Z

I think I understood what you meant. 😄

And for example: https://docs.rs/rocket/0.4.5/rocket/

Crates can provide their own logo, replacing the rust logo. This is why I said that we should not expect to get multiple versions of an image, even if it's only CSS/SVG. :)

XAMPPRocky · 2020-07-15T18:55:38Z

I think I understood what you meant. 😄

Then I guess I don't understand what you mean. A user can already replace the rust logo with their own SVG/CSS logo that changes based on colour scheme so you should already be expecting that.

GuillaumeGomez · 2020-07-15T18:57:28Z

In case they use a PNG image (or more simply, a non-vectorized one), you can't switch the colors in it. Like I said, this is why I added a contrast by using the shadow. Like that, whatever your image color scheme, it'll still be visible.

XAMPPRocky · 2020-07-15T19:05:02Z

I don't think we should add extra styling to user's logos. If you're going out of your way to change the logo you care about how it looks and you are not going to always want a white drop shadow on your logo. We should leave it to the user to decide how to make their logo best contrast with background.

GuillaumeGomez · 2020-07-15T19:08:26Z

It's simply not possible. I repeat myself but we can't expect people to provide vectorized images all the time when we can just put a shadow and make it work in all cases. The point is to make it simpler for users, not more complicated.

XAMPPRocky · 2020-07-15T19:14:08Z

@GuillaumeGomez I don't know why you believe I'm saying that all users have to vector images? I'm only talking about changing the rust logo that we ship with.

GuillaumeGomez · 2020-07-15T19:17:47Z

@XAMPPRocky But then it doesn't solve the current issue for other crates?

XAMPPRocky · 2020-07-15T19:31:33Z

Sure, though I don't think this PR solves this issue for other crates either. There are plenty of designs that will look worse with a white drop shadow. I don't think there's any single solution around the added complexity of having a logo look good in both light and dark colour schemes. I think it's best to leave it up to users to decide if their logo looks good enough, whether they need make a couple of adjustments, or if they want to have a dynamic logo.

Cldfire · 2020-07-15T19:50:49Z

I think it's best to leave it up to users to decide if their logo looks good enough, whether they need make a couple of adjustments, or if they want to have a dynamic logo.

I agree with @XAMPPRocky here, this would seem to make the most sense.

GuillaumeGomez · 2020-07-15T19:52:58Z

I'm strongly opposed to that (because of the complexity added for the users that would have to provide much more than just an image contrary to the current behavior) but at this point, let's just ask what the other rustdoc people think.

PS: they already can override the style for the image if they want, just wanted to precise it.

cc @rust-lang/rustdoc @Manishearth @jyn514

Manishearth · 2020-07-15T20:20:51Z

I think dynamic logos are more work than people are going to be willing to put into it, especially if their logos aren't vector graphics.

A contrasting border (or shadow) is one of the easiest ways to make arbitrary images not look terrible against a background, and I've seen that technique used elsewhere.

Furthermore! The CSS solution only applies to system dark mode, not webpage dark mode. Rustdoc has a builtin dark theme (and there's now a third Ayu theme). I don't think there's a way to communicate the theme being used to an embedded SVG image. This technique does not work here, it's not a favicon that matters here.

Now, one option is to make it possible for users to supply multiple logos in Cargo. A problem I see is that this will get less great if we add more themes, though we could stick to a dark/light distinction and expect themes to pick one.

But the shadow seems like the easiest way to do things here. Perhaps it should just be a solid border and not a shadow, I don't know: CSS borders can't snap to nontransparent parts of images, but you can kinda emulate that with shadows.

GuillaumeGomez · 2020-07-15T20:24:31Z

I might add a bit of context here: I tried using box-shadow before filter and the result is really not great. It just makes a square around the image, just like a border would, this is why I didn't use it. I think using border wouldn't allow to have a rendering as good as this one too.

Manishearth · 2020-07-15T20:28:44Z

Yeah you have to do a blur filter to get a proper shadow on images. But I believe you can write one that creates a solid blur. Idk.

GuillaumeGomez · 2020-07-15T20:30:14Z

I wasn't able to without blurring the full image so I'm curious to see if it's possible. That'd be a nice trick. :)

Cldfire · 2020-07-15T20:37:40Z

Furthermore! The CSS solution only applies to system dark mode, not webpage dark mode. Rustdoc has a builtin dark theme (and there's now a third Ayu theme). I don't think there's a way to communicate the theme being used to an embedded SVG image. This technique does not work here, it's not a favicon that matters here.

Ah yeah, good point. That solution wouldn't actually do the trick here.

XAMPPRocky · 2020-07-15T20:38:59Z

I really don't like how the goal posts have moved in this discussion. The original issue (#74350) was about the Rust logo being difficult to see in dark color schemes, no one has asked for a solution for all logos. I really don't think we should try to guess what will look best for someone else's design or try to solve a problem no one actually has.

GuillaumeGomez · 2020-07-15T20:41:50Z

If the issue exists for the rust logo, it exists for all logo. The only available solution is to add a contrast around the image (like you do for subtitles for example) to make it visible, whatever the logo's colors. So no, it's just that we're trying to have a broader vision of the problem. At least I don't see it as moving away from the original issue.

XAMPPRocky · 2020-07-15T20:51:33Z

If the issue exists for the rust logo, it exists for all logo

I'm not going to comment any further but this is simply not true. To use your own example the Rocket logo doesn't have this issue and arguably looks worse with this change.

Current

Proposed

GuillaumeGomez · 2020-07-15T20:55:24Z

This is just one example. And what if we decide to add a redshift-like theme? Then you'll come up with a green logo and say it's not worth it? I'm not saying your solution is bad, I'm saying it'd require too much efforts from people who use rustdoc. They very likely won't want to provide as many images as there are themes, which makes perfect sense. The solution I offer may not be the best but I think it offers the best ratio of advantages vs shortcomings.

Manishearth · 2020-07-15T21:00:44Z

I really don't think we should try to guess what will look best for someone else's design or try to solve a problem no one actually has.

Ah.

I'm okay with applying a special CSS class to the rust logo only so that the problem is solved for the Rust logo.

bors · 2020-07-17T03:50:56Z

☔ The latest upstream changes (presumably #74422) made this pull request unmergeable. Please resolve the merge conflicts.

GuillaumeGomez · 2020-07-17T08:12:37Z

Rebased.

GuillaumeGomez · 2020-07-23T19:14:14Z

I *think* the global opinion seems ok with this change. Should we move forward then? cc @rust-lang/rustdoc

Manishearth · 2020-07-23T19:22:43Z

I would prefer to special case the rust logo for now but I don't feel strongly. I'm okay with the approach in this PR, but it might be worth opening an internals topic to ask if people are having problems.

GuillaumeGomez · 2020-07-23T19:27:45Z

Ok, so here is what I propose: we merge this PR as is to fix the current issue and we open a topic so we can get people feedback on this and then update if needed. What do you think?

Manishearth · 2020-07-23T19:32:03Z

Seems fine

GuillaumeGomez · 2020-07-23T19:35:17Z

Great! I let you handle the topic? (never know if I should open it on internals or users...)

@bors: r=Manishearth

bors · 2020-07-23T19:35:19Z

📌 Commit 839216a has been approved by Manishearth

bors · 2020-07-23T19:35:20Z

🌲 The tree is currently closed for pull requests below priority 100, this pull request will be tested once the tree is reopened

GuillaumeGomez · 2020-07-23T19:35:30Z

@bors: rollup

Manishearth · 2020-07-23T19:42:31Z

@GuillaumeGomez I don't have time to organize that. Please do so, on internals.

GuillaumeGomez · 2020-07-23T19:46:15Z

Ok! Will do as soon as this PR is merged then.

@ghost

Rollup of 12 pull requests Successful merges: - rust-lang#74361 (Improve doc theme logo display) - rust-lang#74504 (Add right border bar to Dark and Light theme) - rust-lang#74572 (Internally unify rustc_deprecated and deprecated) - rust-lang#74601 (Clean up E0724 explanation) - rust-lang#74623 (polymorphize GlobalAlloc::Function) - rust-lang#74665 (Don't ICE on unconstrained anonymous lifetimes inside associated types.) - rust-lang#74666 (More BTreeMap test cases, some exposing undefined behaviour) - rust-lang#74669 (Fix typo) - rust-lang#74677 (Remove needless unsafety from BTreeMap::drain_filter) - rust-lang#74680 (Add missing backticks in diagnostics note) - rust-lang#74694 (Clean up E0727 explanation) - rust-lang#74703 (Fix ICE while building MIR with type errors) Failed merges: r? @ghost

GuillaumeGomez · 2020-07-24T14:08:23Z

I created the topic: https://internals.rust-lang.org/t/feedback-on-new-rust-documentation-logo-background/12787

tesuji reviewed Jul 15, 2020

View reviewed changes

rust-highfive assigned kinnison Jul 15, 2020

Cldfire mentioned this pull request Jul 15, 2020

Improve ayu rustdoc theme #74371

Merged

bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 17, 2020

Improve logo image display in different themes

839216a

GuillaumeGomez force-pushed the theme-logo branch from 0d8600b to 839216a Compare July 17, 2020 08:12

GuillaumeGomez added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 23, 2020

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 23, 2020

JohnTitor mentioned this pull request Jul 24, 2020

Rollup of 12 pull requests #74710

Merged

bors merged commit 38b2956 into rust-lang:master Jul 24, 2020

GuillaumeGomez deleted the theme-logo branch July 24, 2020 14:02

cuviper added this to the 1.47.0 milestone May 2, 2024

Improve doc theme logo display #74361

Improve doc theme logo display #74361

Conversation

GuillaumeGomez commented Jul 15, 2020 • edited Loading

tesuji Jul 15, 2020 • edited Loading

Choose a reason for hiding this comment

GuillaumeGomez Jul 15, 2020 • edited Loading

Choose a reason for hiding this comment

tesuji Jul 15, 2020 • edited Loading

Choose a reason for hiding this comment

GuillaumeGomez Jul 15, 2020

Choose a reason for hiding this comment

UtherII Jul 15, 2020 • edited Loading

Choose a reason for hiding this comment

GuillaumeGomez Jul 15, 2020

Choose a reason for hiding this comment

tesuji commented Jul 15, 2020 • edited Loading

Cldfire commented Jul 15, 2020

tesuji commented Jul 15, 2020

GuillaumeGomez commented Jul 15, 2020 • edited Loading

GuillaumeGomez commented Jul 15, 2020

pickfire commented Jul 15, 2020 • edited Loading

GuillaumeGomez commented Jul 15, 2020 • edited Loading

XAMPPRocky commented Jul 15, 2020 • edited Loading

GuillaumeGomez commented Jul 15, 2020

XAMPPRocky commented Jul 15, 2020 • edited Loading

GuillaumeGomez commented Jul 15, 2020

XAMPPRocky commented Jul 15, 2020

GuillaumeGomez commented Jul 15, 2020 • edited Loading

XAMPPRocky commented Jul 15, 2020

GuillaumeGomez commented Jul 15, 2020

XAMPPRocky commented Jul 15, 2020

GuillaumeGomez commented Jul 15, 2020

XAMPPRocky commented Jul 15, 2020

Cldfire commented Jul 15, 2020

GuillaumeGomez commented Jul 15, 2020 • edited Loading

Manishearth commented Jul 15, 2020

GuillaumeGomez commented Jul 15, 2020

Manishearth commented Jul 15, 2020

GuillaumeGomez commented Jul 15, 2020

Cldfire commented Jul 15, 2020

XAMPPRocky commented Jul 15, 2020

GuillaumeGomez commented Jul 15, 2020

XAMPPRocky commented Jul 15, 2020

Current

Proposed

GuillaumeGomez commented Jul 15, 2020

Manishearth commented Jul 15, 2020

bors commented Jul 17, 2020

GuillaumeGomez commented Jul 17, 2020

GuillaumeGomez commented Jul 23, 2020

Manishearth commented Jul 23, 2020

GuillaumeGomez commented Jul 23, 2020

Manishearth commented Jul 23, 2020

GuillaumeGomez commented Jul 23, 2020

bors commented Jul 23, 2020

bors commented Jul 23, 2020

GuillaumeGomez commented Jul 23, 2020

Manishearth commented Jul 23, 2020

GuillaumeGomez commented Jul 23, 2020

GuillaumeGomez commented Jul 24, 2020

GuillaumeGomez commented Jul 15, 2020 •

edited

Loading

tesuji Jul 15, 2020 •

edited

Loading

GuillaumeGomez Jul 15, 2020 •

edited

Loading

tesuji Jul 15, 2020 •

edited

Loading

UtherII Jul 15, 2020 •

edited

Loading

tesuji commented Jul 15, 2020 •

edited

Loading

GuillaumeGomez commented Jul 15, 2020 •

edited

Loading

pickfire commented Jul 15, 2020 •

edited

Loading

GuillaumeGomez commented Jul 15, 2020 •

edited

Loading

XAMPPRocky commented Jul 15, 2020 •

edited

Loading

XAMPPRocky commented Jul 15, 2020 •

edited

Loading

GuillaumeGomez commented Jul 15, 2020 •

edited

Loading

GuillaumeGomez commented Jul 15, 2020 •

edited

Loading