Skip to content
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: Make the #[stable(since)] version attribute clearer with a tooltip #33705

Merged
merged 1 commit into from
May 19, 2016
Merged

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

merged 1 commit into from
May 19, 2016

Conversation

lqd
Copy link
Member

@lqd lqd commented May 18, 2016

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
Collaborator

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
Copy link
Member Author

lqd commented May 18, 2016

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

@steveklabnik
Copy link
Member

@bors: r+ rollup

I like it, thank you!

@bors
Copy link
Contributor

bors commented May 18, 2016

📌 Commit e733629 has been approved by steveklabnik

@GuillaumeGomez
Copy link
Member

tmp

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

@steveklabnik
Copy link
Member

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

@steveklabnik
Copy link
Member

@bors: r-

until we sort out what is going on

@GuillaumeGomez
Copy link
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
Copy link
Member Author

lqd commented May 18, 2016

@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
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
Copy link
Member Author

lqd commented May 19, 2016

@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
Copy link
Member

GuillaumeGomez commented May 19, 2016

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
Copy link
Member Author

lqd commented May 19, 2016

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
Member

This would be very appreciated. :)

@lqd
Copy link
Member Author

lqd commented May 19, 2016

No problem.

Here's how it looks
chrome50

@GuillaumeGomez
Copy link
Member

Great, thanks! Can you squash your commits please?

@lqd
Copy link
Member Author

lqd commented May 19, 2016

sure

@lqd
Copy link
Member Author

lqd commented May 19, 2016

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

@steveklabnik
Copy link
Member

@lqd looks good here!

@bors: delegate=guillaumegomez

@bors
Copy link
Contributor

bors commented May 19, 2016

✌️ @GuillaumeGomez can now approve this pull request

@GuillaumeGomez
Copy link
Member

@bors: r+ rollup

Thanks @lqd!

@bors
Copy link
Contributor

bors commented May 19, 2016

📌 Commit 352a70b has been approved by GuillaumeGomez

@lqd
Copy link
Member Author

lqd commented May 19, 2016

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