Skip to content

rustdoc: Make the #[stable(since)] version attribute clearer with a tooltip#33705

Merged
bors merged 1 commit into
rust-lang:masterfrom
lqd:rustdoc-version-tooltip
May 19, 2016
Merged

rustdoc: Make the #[stable(since)] version attribute clearer with a tooltip#33705
bors merged 1 commit into
rust-lang:masterfrom
lqd:rustdoc-version-tooltip

Conversation

@lqd

@lqd lqd commented May 18, 2016

Copy link
Copy Markdown
Member

Rustdoc's new 'since' version placement only shows the version number in which the item was marked stable. This gains space but might make the meaning of this version string less clear in the docs, so I tried to bring some explicitness in a tooltip.

@rust-highfive

Copy link
Copy Markdown
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @cmr (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@lqd

lqd commented May 18, 2016

Copy link
Copy Markdown
Member Author

I thought I had pushed/squashed that commit yesterday but I was very wrong

@steveklabnik

Copy link
Copy Markdown
Contributor

@bors: r+ rollup

I like it, thank you!

@bors

bors commented May 18, 2016

Copy link
Copy Markdown
Collaborator

📌 Commit e733629 has been approved by steveklabnik

@GuillaumeGomez

Copy link
Copy Markdown
Member

tmp

Sorry but I don't like this change. For me it's way too much.

@steveklabnik

Copy link
Copy Markdown
Contributor

@GuillaumeGomez I am confused. The title should not be shown on the page, only on hover.

@steveklabnik

Copy link
Copy Markdown
Contributor

@bors: r-

until we sort out what is going on

@GuillaumeGomez

Copy link
Copy Markdown
Member

My screenshot is wrong, however it makes the "since" disappears (and it doesn't show back when you hover the title or whatever). I'm not against a tootip when you hover the "since", but this is not what this seems to do.

@lqd

lqd commented May 18, 2016

Copy link
Copy Markdown
Member Author

@GuillaumeGomez Salut Guillaume ;)

What did you mean by making the "since" disappear ? Like, the word "since" is missing ? If that is the case, I just added the title, there doesn't seem to be a "since" right now on nightly docs, eg https://doc.rust-lang.org/nightly/std/collections/hash_map/struct.HashMap.html#method.with_hasher to link around your screenshot.

@GuillaumeGomez

Copy link
Copy Markdown
Member

@GuillaumeGomez Salut Guillaume ;)

Salut ! :)

What did you mean by making the "since" disappear ? Like, the word "since" is missing ? If that is the case, I just added the title, there doesn't seem to be a "since" right now on nightly docs, eg https://doc.rust-lang.org/nightly/std/collections/hash_map/struct.HashMap.html#method.with_hasher to link around your screenshot.

No, I'm referring to "1.7.0" for example. The "since tag" if you prefer. With your code, they disappear.

@lqd

lqd commented May 19, 2016

Copy link
Copy Markdown
Member Author

@GuillaumeGomez :
That is weird, adding a title shouldn't impact layout/rendering, I guess this is what was confusing Steve as well. What browser/os are you testing with ?

Here's a Chrome 50 (win) screenshot of your "since" span I modified, on HashMap, there's both "1.0.0" and the tooltip showing:
chrome50

Same under FF/win 46:
ff46

I don't have other OSes handy but I tested the PR on OSX as well.

The #[stable(since)] to methods like with_hasher (and what your screenshot was showing) are divs that I didn't modify so they should still be here.

Maybe you're talking about since tags other than those 2 kinds ? Could you share a screenshot of the behavior you're seeing ?

EDIT: to ping guillaume and remove a wrong comment on my part

@GuillaumeGomez

GuillaumeGomez commented May 19, 2016

Copy link
Copy Markdown
Member

I tested on chrome on windows, but not on this "since tag", it was this one. And it seems to be working on firefox/debian. Strange...

@steveklabnik: Let's merge this for now and if I have another problem, it'll be always time to fix it.

@lqd

lqd commented May 19, 2016

Copy link
Copy Markdown
Member Author

Yeah IIRC those are divs rendered by render_stability_since_raw() which I didn't modify — but probably should/will at some point if you guys want me to add a tooltip there as well :)

@GuillaumeGomez

Copy link
Copy Markdown
Member

This would be very appreciated. :)

@lqd

lqd commented May 19, 2016

Copy link
Copy Markdown
Member Author

No problem.

Here's how it looks
chrome50

@GuillaumeGomez

Copy link
Copy Markdown
Member

Great, thanks! Can you squash your commits please?

@lqd

lqd commented May 19, 2016

Copy link
Copy Markdown
Member Author

sure

@lqd

lqd commented May 19, 2016

Copy link
Copy Markdown
Member Author

@GuillaumeGomez I hope I was able to do it correctly :)

@steveklabnik

Copy link
Copy Markdown
Contributor

@lqd looks good here!

@bors: delegate=guillaumegomez

@bors

bors commented May 19, 2016

Copy link
Copy Markdown
Collaborator

✌️ @GuillaumeGomez can now approve this pull request

@GuillaumeGomez

Copy link
Copy Markdown
Member

@bors: r+ rollup

Thanks @lqd!

@bors

bors commented May 19, 2016

Copy link
Copy Markdown
Collaborator

📌 Commit 352a70b has been approved by GuillaumeGomez

@lqd

lqd commented May 19, 2016

Copy link
Copy Markdown
Member Author

merci @GuillaumeGomez and thank you @steveklabnik :)

Manishearth added a commit to Manishearth/rust that referenced this pull request May 19, 2016
…laumeGomez

rustdoc: Make the #[stable(since)] version attribute clearer with a tooltip

Rustdoc's new 'since' version placement only shows the version number in which the item was marked stable. This gains space but might make the meaning of this version string less clear in the docs, so I tried to bring some explicitness in a tooltip.
bors added a commit that referenced this pull request May 19, 2016
Rollup of 10 pull requests

- Successful merges: #33353, #33611, #33696, #33698, #33705, #33708, #33712, #33720, #33721, #33730
- Failed merges:
@bors bors merged commit 352a70b into rust-lang:master May 19, 2016
@lqd lqd deleted the rustdoc-version-tooltip branch May 23, 2016 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants