Skip to content

Add links for impls #43979

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Aug 26, 2017
Merged

Add links for impls #43979

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c8d58a3
Add links for impls
Jouan Aug 18, 2017
4729f22
Fixed changes to .in-band CSS
Jouan Aug 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/librustdoc/html/render.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2925,7 +2925,13 @@ fn render_deref_methods(w: &mut fmt::Formatter, cx: &Context, impl_: &Impl,
fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLink,
render_mode: RenderMode, outer_version: Option<&str>) -> fmt::Result {
if render_mode == RenderMode::Normal {
write!(w, "<h3 class='impl'><span class='in-band'><code>{}</code>", i.inner_impl())?;
let id = derive_id(match i.inner_impl().trait_ {
Some(ref t) => format!("impl-{}", Escape(&format!("{:#}", t))),
None => "impl".to_string(),
});
write!(w, "<h3 id='{}' class='impl'><span class='in-band'><code>{}</code>",
id, i.inner_impl())?;
write!(w, "<a href='#{}' class='anchor'></a>", id)?;
write!(w, "</span><span class='out-of-band'>")?;
let since = i.impl_item.stability.as_ref().map(|s| &s.since[..]);
if let Some(l) = (Item { item: &i.impl_item, cx: cx }).src_href() {
Expand Down
4 changes: 4 additions & 0 deletions src/librustdoc/html/static/rustdoc.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,10 @@ a {
.small-section-header:hover > .anchor {
display: initial;
}

.in-band:hover > .anchor {
display: initial;
}
.anchor {
display: none;
}
Expand Down
3 changes: 0 additions & 3 deletions src/librustdoc/html/static/styles/main.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@ h1.fqn {
h2, h3:not(.impl):not(.method):not(.type):not(.tymethod), h4:not(.method):not(.type):not(.tymethod) {
border-bottom-color: #DDDDDD;
}
.in-band {
background-color: white;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did this style get taken out? Was it interfering with the anchor tag?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the :target style is attempting to change the background to yellow, but the .in-band style is setting it back to white, creating a weird color layout.
Since the background for .in-band is typically white anyway, I thought it was reasonably safe to remove.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, that makes sense, but i'm still gonna call in @GuillaumeGomez for a second opinion. (Usually he's supposed to get pinged on CSS changes but it looks like that one didn't happen this time >_>)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a reason for this behavior. When the text of a function overlaps a version number, the version number isn't visible anymore to avoid weird text overlap. Normally, you could just put your own background-color after this one without issues.

Copy link
Contributor Author

@Jouan Jouan Aug 18, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just reverted my changes to main.css and I made the :target style look like this:
:target { background: #FDFFD3; background-color: #FDFFD3; }

Unfortunately, it doesn't solve the issue. Is this the change that you had in mind?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange to have background and background-color and that your CSS change doesn't work...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have made a change that conserves the .in-band style here: 4729f22


.docblock code, .docblock-short code {
background-color: #F5F5F5;
Expand Down