-
Notifications
You must be signed in to change notification settings - Fork 13k
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
More natural rustdoc typography styling #54772
Conversation
Some changes occurred in HTML/CSS. |
(rust_highfive has picked a reviewer for you, use r? to override) |
8ba5de7
to
b4a58f3
Compare
This patch is similar in spirit to rust-lang/blog.rust-lang.org#263 and onur/docs.rs#240; it removes the specific font overrides in rustdoc.css so that the browser will use the user's configured fonts. This reduces the weight of all rustdoc pages (yay for mobile!) while also letting users use whatever fonts they prefer. The net result of the change is that the page will look less "uniform" across different browsers, OSes, and devices, but the overall aesthetic should be preserved. It also means that user preferences are respected, and that legibility is improved for users with particular font needs, or on platforms that sometimes gets confused about how to render fonts nicely (ehrm, Linux).
b4a58f3
to
b77f439
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
It seems like a good idea since it reduces the whole pages size but the rendering isn't unified anymore. Not sure what to think about this last part... cc @rust-lang/rustdoc |
@GuillaumeGomez Why is the rendering being unified a plus? |
It's like a signature. Also, I just thought about it but it'll likely break the UI tests. |
I'm skeptical about this. It puts aspects of the pages' appearance out of our control, and can lead to situations that are hard to verify, because people's system fonts are going to be different. For example, inline code spans look awkward in my browser at work (Chrome, Windows 7) because its choice of monospace font uses a shorter line-height than its sans-serif font: I agree that this will allow well-configured systems to have clearer fonts available, but i can't shake the feeling that it will look worse on some systems. We'll also need to be more careful about using text symbols as labels - right now, different elements use a circle-i character, a warning symbol, and a microscope emoji to flag certain sections. For example, Windows 7 (and many Linux systems) may not have an emoji font loaded, leaving our "unstable" banner with a little box: (On the other hand, the microscope emoji doesn't load for me with our current docs, so maybe this concern is moot.) (See again how the monospace font is way smaller than the sans-serif font in that banner.) Related to the text icons, the baseline for some of them seems to be different in certain situations. For example, in your own screenshot, the "important traits" icon is no longer inline with the "fold docs" icon. On my own system, the smaller monospace font also causes the "fold docs" icon to get out of line with the function signature: (Humorously enough, this problem doesn't occur in Internet Explorer because it uses Courier New for its monospace font instead of Consolas, which Chrome uses.) These may each be small things, but they're things we didn't need to worry about before, because we shipped a font of our choosing, giving rust documentation a uniform look and feel above and beyond the page layout. This PR has merit, but i also feel that it will introduce problems that are hard to reproduce, and possibly create complaints from people whose docs are now using a potentially-worse font. |
@QuietMisdreavus Yeah, you're definitely right that this will make the page read worse for those with unfortunate font combinations, which I agree is unfortunate. The issue with monospaced fonts generally being smaller is also a significant one. I don't actually know how to resolve those, but also feel like it's sad to have to pull in all these fonts and not allow the user to configure their own fonts. Some of the alignment issues (like the i icon) can be solved automatically by placing things in the same container and using CSS flexbox or grid with proper alignment, but that would be a more significant change. For icons, I think unicode characters (at least some of them) have pretty decent font coverage at this point, so as long as we stick to pretty basic ones, we should be fine. Maybe the path forward would be a deeper re-write that uses more modern CSS tools like flexbox and grid to set up the page such that these issues go away. The one that we probably can't get away from easily is inline monospace. There is some discussion here and here that may be relevant though, and may solve our woes. |
I suppose a way to figure it out is to decide how many people actually configure their own fonts, how many people even know that's a thing, and how many people would be worse off if we stopped shipping our own font. I doubt it will render anyone's docs illegible. Is there prior discussion about turning off font overrides? Did anyone request this (here or for any of the linked PRs in the OP)? Is this a "best practice" thing? I'm trying to find the motivation. |
I suspect the number of users who do so is relatively small. It's probably mainly users with special needs, or people like me who care about typography :) No-one requested this beyond me as far as I'm aware, so it is probably a niche case, though there is an argument for this reducing the page size by a decent amount. The current page pulls down eight-or-so fonts, which is not great for loading speed, especially on mobile. I also think this there is a "best practice" argument to be made that you shouldn't pick specific fonts unless you have a good reason to (like you need particular glyphs). Letting the browser choose gives you whatever font is standard for the user's platform, which leads to greater uniformity across the user's experiences, even if the page looks different for different users. |
"Relatively small" is still worse than "none". |
Ping from triage @steveklabnik / @rust-lang/rustdoc: I assume this requires a team decision. |
I'll close it for now. Too many potential issues for a small doesn't seem to be worth it. A system to import could be fun/useful though, but it'd be on users' side. Thanks anyway @jonhoo! |
...did you mean to leave the PR open? |
No, I just forgot to close it. 🤣 |
This is a (much) more constrained version of rust-lang#54772 that also aims at improving the situation in rust-lang#34681. It removes any font specifications that are not the "official" rustdoc font, and instead relies on the browser to provide the fallback font if the official on is not available. On Linux systems, this is particularly important, as fonts like Helvetica, Arial, and Times often look pretty bad since they're pulled from extracted MS fonts. A specification like `serif` or `sans-serif` lets the browser instead choose a good font.
…llaumeGomez Remove intermediate font specs This is a (much) more constrained version of rust-lang#54772 that also aims at improving the situation in rust-lang#34681. It removes any font specifications that are not the "official" rustdoc font, and instead relies on the browser to provide the fallback font if the official on is not available. On Linux systems, this is particularly important, as fonts like Helvetica, Arial, and Times often look pretty bad since they're pulled from extracted MS fonts. A specification like `serif` or `sans-serif` lets the browser instead choose a good font.
…llaumeGomez Remove intermediate font specs This is a (much) more constrained version of rust-lang#54772 that also aims at improving the situation in rust-lang#34681. It removes any font specifications that are not the "official" rustdoc font, and instead relies on the browser to provide the fallback font if the official on is not available. On Linux systems, this is particularly important, as fonts like Helvetica, Arial, and Times often look pretty bad since they're pulled from extracted MS fonts. A specification like `serif` or `sans-serif` lets the browser instead choose a good font.
This patch is similar in spirit to rust-lang/blog.rust-lang.org#263 and onur/docs.rs#240; it removes the specific font overrides in rustdoc.css so that the browser will use the user's configured fonts. This reduces the weight of all rustdoc pages (yay for mobile!) while also letting users use whatever fonts they prefer.
The net result of the change is that the page will look less "uniform" across different browsers, OSes, and devices, but the overall aesthetic should be preserved. It also means that user preferences are respected, and that legibility is improved for users with particular font needs, or on platforms that sometimes gets confused about how to render fonts nicely (ehrm, Linux).