Make the rustdoc example links more noticable #28963
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
| @@ -48,6 +48,7 @@ r##"<!DOCTYPE html> | |||
| <title>{title}</title> | |||
|
|
|||
| <link rel="stylesheet" type="text/css" href="{root_path}main.css"> | |||
| <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/font-awesome/4.4.0/css/font-awesome.min.css"> | |||
There was a problem hiding this comment.
There's no need to include the whole font-awesome package for one small rocket icon. This won't work for offline documentation, too.
There was a problem hiding this comment.
Yeah, I thought people might say that. Just the text?
|
Good initiative. But, wow, that button is massive! Also in the wrong place, IMHO. I'd just add a line below the code sample that reads "Run this code on play.rust-lang.org now »". (People tend to read code from top to bottom, so they'll probably notice this more than something on the top right corner.) |
|
Hmm, I might be missing something, but why not make the old icon always visible (and may be have a tooltip reading "run on playpen")? |
|
@jonas-schievink I think the idea is to make it more noticeable than that would. Although I agree with @killercup that the current button design is too large. |
|
Great idea! I think the button should have a verb on it. 'Run' or 'Run in playpen' would clarify that this is an action a user can take, not just a status lozenge. |
|
rustbyexample.com uses |
|
The problem with just the icon is it's bounding box is tall and doesn't allow the icon to scale any larger. Originally I tried to make this bigger but the icon height really doesn't allow any size increase. You can add padding to make it square but I thought the icon needed to be bigger if people are supposed to notice it easier. That led to me trying text with a different icon.
|
|
@killercup The button is in the same place as before. Just shifted down a few pixels I think and it still only shows on mouseover. |
Yes, and these are both things I would change, @mdinger. 😃 |
|
Hmm...I don't know. On this page there are 5 small examples immediately in front of you. Adding a |
|
Here's a quick mockup of what I interpret @killercup's suggestion as. I darkened the code for easier contrast comparison.
|
|
Updated the PR without the rocket while these other things are discussed. Here's a picture of it.
|
|
I could also switch the word around but the word has to be small ( |
|
Is this better? Worse? The colors are a straight inverse of the original arrow. If it's too noticeable, I could make it a grey slightly darker than the background. That would be it a little more subtle. |
|
@mdinger which 'this' are you talking about? |
|
What about the style from https://www.rust-lang.org/?
The current rustdoc style doesn't even work at all for me (Windows 7 / Chrome stable):
|
|
@steveklabnik I'm asking about the most recent picture I included which is what the current PR implements (this). @iliekturtles rust-lang.org runs the example in place whereas rustdoc just links the example to the playpen. Should the icons be the same regardless? About the icon missing, does changing the browser encoding fix it? I could make changes but I need input on how it needs to be changed to be accepted. If I just need to change the word, add an icon, or change the background, those are all easy. If I have to implement something more like @killercup suggested, that is much more involved. |
|
@mdinger I like "Run" or "Run" + an icon. If you do include text it should be a verb. Not sure what you mean / how to change the encoding. Changing the font list with Chrome's dev tools just changes to a different invalid character. Copying the character to notepad or this edit box gives an arrow pointing up and to the left (preview shows an invalid character): ⇱. IE and Edge (on Win 10) both properly show an up-right arrow. |
|
@iliekturtles Try switching it to unicode or something else like this:
Okay. I dislike Yeah, that's the icon. The browser just mirrors it for looks. |
|
So, I am very interested in improving this. However, the encoding situation makes me a bit worried about the current implementation, this isn't something we should have to worry about. I think that just "Run" probably suffices. Probably. |
|
@mdinger UTF-8 was already properly detected. Attempting different encodings didn't help. Every Windows 7/10 machine with Chrome I've checked has been affected. |
|
Updated. Here is the updated picture.
Fixing the encoding is outside the scope of this issue but removing non-ASCII characters should fix the situation in the meantime. |
|
r? |
1 similar comment
|
r? |
|
@bors: r+ |
|
📌 Commit dd342b0 has been approved by |
|
@mdinger sorry about that, I've been to two conferences in two countries in the last seven days, and I wanted to sit on it a bit before merging. Thanks for your patience. |
|
It's okay. I actually tried to get it fixed the day it came up in a reddit thread so the guy who just saw the issue would think Rust devs fix issues fast but that didn't happen. |
This is to make the link more prominent so hopefully people will actually see it. The new icon is partially because I wasn't sure how easy it would be to apply the previous transformations only to the last character of the string. As it is, I wasn't sure at first but I think the look is growing on me. A minor nitpick is that the space after `Runnable` is underlined and I tried to fix that but it wasn't working for me right now. I tried switching a link with subelements to a div with subelements but I missed something because it wasn't working correctly. --- Unselected:  --- Selected:  Fixes #28958
|
💔 Test failed - auto-linux-64-x-android-t |
|
@bors: retry On Wed, Oct 21, 2015 at 2:19 PM, bors notifications@github.com wrote:
|
|
⚡ Previous build results for auto-linux-cross-opt, auto-win-msvc-32-opt are reusable. Rebuilding only auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-64-opt... |
|
@bors: retry force |
This is to make the link more prominent so hopefully people will actually see it. The new icon is partially because I wasn't sure how easy it would be to apply the previous transformations only to the last character of the string. As it is, I wasn't sure at first but I think the look is growing on me. A minor nitpick is that the space after `Runnable` is underlined and I tried to fix that but it wasn't working for me right now. I tried switching a link with subelements to a div with subelements but I missed something because it wasn't working correctly. --- Unselected:  --- Selected:  Fixes #28958
Makes rustbook code playpen links follow the style set in #28963. This is basically cut and paste from the other one. The link looks better and still works so I assume it's good.  Fixes #29308 r? @steveklabnik
This is to make the link more prominent so hopefully people will actually see it. The new icon is partially because I wasn't sure how easy it would be to apply the previous transformations only to the last character of the string. As it is, I wasn't sure at first but I think the look is growing on me.
A minor nitpick is that the space after
Runnableis underlined and I tried to fix that but it wasn't working for me right now. I tried switching a link with subelements to a div with subelements but I missed something because it wasn't working correctly.Unselected:
Selected:
Fixes #28958