-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Rustdoc accessibility: use an icon for the [-]/[+] controls #87663
Rustdoc accessibility: use an icon for the [-]/[+] controls #87663
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, actually, there's one thing that I'd like to try, but I'd like a few other people to check this out on their configuration, to see if they also prefer it this way.
The thing that I don't like about the current version of this is that it makes the icons look blurrier on hidpi screens. I'm not quite sure how to fix that, but I think adding these CSS rules might help:
image-rendering: crisp-edges;
image-rendering: -moz-crisp-edges; /* Firefox before version 65 */
image-rendering: -webkit-optimize-contrast; /* Safari */
-ms-interpolation-mode: nearest-neighbor; /* EdgeHTML Edge and Trident */
image-rendering: pixelated; /* Chrome */
The one on the left is with the image-rendering CSS things, while the one on the right doesn't have them: Here's a web upload showing it, so that we can test this on as many browsers as we can get. Because the caniuse site has quite a few caveats with getting good pixel art scaling in browsers. https://notriddle.com/notriddle-rustdoc-test/pixelated-plus-minus/std/index.html I seem to have a version that works in Chromium and Firefox on Linux, and Mobile Safari on an iPad, but I'm not terribly confident in it. We really want this icon to snap to the pixel grid. Hard. And it's not as easy to get that as I would like. |
@notriddle Maybe with them in SVG it will help. Toggle minus: <svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" stroke="#000" fill="none"><path d="M4.5 2H2v12h2.5m7-12H14v12h-2.5"/><path d="M4.5 8h7"/></svg> Toggle plus: <svg xmlns="http://www.w3.org/2000/svg" width="16" height="16" stroke="#000" fill="none"><path d="M4.5 2H2v12h2.5m7-12H14v12h-2.5"/><path d="M4.5 8h7"/><path d="M8 11.5V8.125h0V4.5"/></svg> |
@Urgau: Let's try svg! Thanks for the conversion. ;) |
The SVG you provided is all blurry on my computer, it gets better when I zoom in, but it's not great... I've read around that using number without decimal makes it better, maybe it's worth a try? I tried to "fix" the SVG but it was a complete failure. ^^' |
My SVGs have a size of 16x16 like the original PNG files, this size is very small and when a software tries to display an image with such a tiny size, it renders it in this original size and then adapts it to the target size (which is much bigger) but some software "forget" that an SVG can be rendered at any size (which avoids any scaling and is the reason for SVGs) and so they render it and adapt it instead of rendering it at the target size, that maybe be why it can appear blurry on your computer. You can see on this jsfiddle my SVGs in their original sizes but also in 350x350, I don't see any blur, do you ? Do you want me to increase their default size ? and by curiosity, what software are you using to render the SVGs ? I never heard that number without decimal makes svg better, but I'm not an expert either. |
If it gets better when you zoom in, then it's not being caused by the browser rendering it at the original size and scaling. It's caused by the lines not being placed exactly on pixel boundaries. This is why the original PNG files are not 16x16. They're 17x17. By using an odd number of pixels, you can draw a 1px line in the exact center of the image without aliasing. |
@Urgau I meant what @notriddle said: the pixel boundaries are not integers but decimals, which is what I think is the issue. |
Heu... I don't know what happen, I'm pretty sure I have enter 17x17 for the size; I don't know what to say, I should have checked, sorry.
If that the case I don't know how to resolve that because Inkscape is generating this decimal values. In any case I have redone and attached the SVGs with their correct size 17x17 (I checked this time). Let me know if it's still not good, maybe I can try something else. Toggle minus (17x17) <svg width="17" height="17" stroke="#000" fill="none" xmlns="http://www.w3.org/2000/svg"><path d="M5 2.5H2.5v12H5m7-12h2.5v12H12M5 8.5h7"/></svg> Toggle plus (17x17) <svg width="17" height="17" stroke="#000" fill="none" xmlns="http://www.w3.org/2000/svg"><path d="M5 2.5H2.5v12H5m7-12h2.5v12H12M5 8.5h7M8.5 12V8.625v0V5"/></svg> PS: By redoing them I even manage to shrink them down a little more |
Ah nice, thanks! Let's try with these new images then. |
This way, we can show the plus and minus buttons on screens, while voice control will read off actual words "Collapse" and "Expand" instead of reading "open brace minus close brace" and "open brace plus close brace". Part of rust-lang#87059
948d280
to
5e86d7c
Compare
@Urgau Not blurry anymore, thanks a lot! @notriddle I think the PR is ready. :) |
5e86d7c
to
e5ec9c1
Compare
Can you please publish a built copy of the standard library docs, like I’ve been doing? That way, we can all easily test the results. |
Sure, here you go: https://guillaume-gomez.fr/rustdoc-test/foo/struct.Foo.html (not std, simply a small crate). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add the attribute shape-rendering="crispEdges"
to each of the <svg>
elements, and then I'll approve it.
Here's a side-by-side comparison of the SVG, on the left is without crispEdges, and the right is with crispEdges. Both are zoomed in at 133%, so that the line would need to be aliased, and magnified with no interpolation after taking a screenshot.
@@ -0,0 +1 @@ | |||
<svg width="17" height="17" stroke="#000" fill="none" xmlns="http://www.w3.org/2000/svg"><path d="M5 2.5H2.5v12H5m7-12h2.5v12H12M5 8.5h7"/></svg> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<svg width="17" height="17" stroke="#000" fill="none" xmlns="http://www.w3.org/2000/svg"><path d="M5 2.5H2.5v12H5m7-12h2.5v12H12M5 8.5h7"/></svg> | |
<svg width="17" height="17" stroke="#000" fill="none" shape-rendering="crispEdges" xmlns="http://www.w3.org/2000/svg"><path d="M5 2.5H2.5v12H5m7-12h2.5v12H12M5 8.5h7"/></svg> |
@@ -0,0 +1 @@ | |||
<svg width="17" height="17" stroke="#000" fill="none" xmlns="http://www.w3.org/2000/svg"><path d="M5 2.5H2.5v12H5m7-12h2.5v12H12M5 8.5h7M8.5 12V8.625v0V5"/></svg> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<svg width="17" height="17" stroke="#000" fill="none" xmlns="http://www.w3.org/2000/svg"><path d="M5 2.5H2.5v12H5m7-12h2.5v12H12M5 8.5h7M8.5 12V8.625v0V5"/></svg> | |
<svg width="17" height="17" stroke="#000" fill="none" shape-rendering="crispEdges" xmlns="http://www.w3.org/2000/svg"><path d="M5 2.5H2.5v12H5m7-12h2.5v12H12M5 8.5h7M8.5 12V8.625v0V5"/></svg> |
e5ec9c1
to
6fe0972
Compare
That looks better, indeed! Thanks to both of you for the help! @bors: r=notriddle |
📌 Commit 6fe0972 has been approved by |
…brace, r=notriddle Rustdoc accessibility: use an icon for the [-]/[+] controls This is a reopening of rust-lang#87207 with improvement for the way of generating the `background-image` CSS property. I quote from the original PR: > This way, we can show the plus and minus buttons on screens, while voice > control will read off actual words "Collapse" and "Expand" instead of reading > "open brace minus close brace" and "open brace plus close brace". Part of rust-lang#87059 r? `@notriddle`
Rollup of 8 pull requests Successful merges: - rust-lang#81797 (Add `core::stream::from_iter`) - rust-lang#87267 (Remove space after negative sign in Literal to_string) - rust-lang#87663 (Rustdoc accessibility: use an icon for the [-]/[+] controls) - rust-lang#87720 (don't use .into() to convert types to identical types (clippy::useless_conversion)) - rust-lang#87723 (Use .contains instead of manual reimplementation.) - rust-lang#87729 (Remove the aarch64 `crypto` target_feature) - rust-lang#87731 (Update cargo) - rust-lang#87734 (Test dropping union fields more) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This is a reopening of #87207 with improvement for the way of generating the
background-image
CSS property.I quote from the original PR:
Part of #87059
r? @notriddle