Skip to content

Commit

Permalink
Rollup merge of rust-lang#56874 - JohnHeitmann:docs-spacing, r=Guilla…
Browse files Browse the repository at this point in the history
…umeGomez

Simplify foreign type rendering.

Simplified foreign type rendering by switching from tables to flexbox. Also, removed some seemingly extraneous elements like “ghost” spans.

Reduces element count on the `std::iter::Iterator` page by 30%. On my laptop it drops Iterator page load time from ~15s to ~10s. Frame times during scrolling are a hair lower too.

Known visual changes (happy to tweak based on feedback):
* The main `impl ...` headers are now getting the default, larger, h3 font size. This was an accident, but I liked how it turned out so I didn't fix it.
* There's a hair less vertical spacing between the end of a where block and the start of the next fn. Now, all spacing is consistent. I think this looks a bit worse. I may tweak vertical spacing more here or in a follow-up that cleans up vertical spacing more broadly.
* "[src]" links are all sized at 17px. A few were 19px in the original.

I haven't yet done heavy cross-browser or cross-crate testing. I was hoping to get a quick thumbs up or thumbs down here at this first draft, then if this is on the right track I'll spend some time on that testing.

TODO:

- [x] Test on Chrome
- [x] Test on Firefox
- [ ] ~~Test on UC Android~~
- [x] Test on Edge
- [x] Test on iOS safari
- [x] Test on desktop safari
- [x] Update automated tests
- [x] Increase vertical margin
- [x] Fix "Important traits for" hover overlap
- [x] Wait for rust-lang#55798 to land & merge it
  • Loading branch information
Centril committed Jan 13, 2019
2 parents 2cf736f + 34bd2b8 commit 6d00124
Show file tree
Hide file tree
Showing 19 changed files with 95 additions and 93 deletions.
70 changes: 19 additions & 51 deletions src/librustdoc/html/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3059,7 +3059,7 @@ fn item_trait(
let item_type = m.type_();
let id = cx.derive_id(format!("{}.{}", item_type, name));
let ns_id = cx.derive_id(format!("{}.{}", name, item_type.name_space()));
write!(w, "{extra}<h3 id='{id}' class='method'><code id='{ns_id}'>",
write!(w, "<h3 id='{id}' class='method'>{extra}<code id='{ns_id}'>",
extra = render_spotlight_traits(m)?,
id = id,
ns_id = ns_id)?;
Expand Down Expand Up @@ -3444,7 +3444,7 @@ fn item_union(w: &mut fmt::Formatter, cx: &Context, it: &clean::Item,
let id = format!("{}.{}", ItemType::StructField, name);
write!(w, "<span id=\"{id}\" class=\"{shortty} small-section-header\">\
<a href=\"#{id}\" class=\"anchor field\"></a>\
<span class='invisible'><code>{name}: {ty}</code></span>\
<code>{name}: {ty}</code>\
</span>",
id = id,
name = name,
Expand Down Expand Up @@ -3999,8 +3999,7 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi
None => "impl".to_string(),
});
if let Some(use_absolute) = use_absolute {
write!(w, "<h3 id='{}' class='impl'><span class='in-band'><table class='table-display'>\
<tbody><tr><td><code>", id)?;
write!(w, "<h3 id='{}' class='impl'><code class='in-band'>", id)?;
fmt_impl_for_trait_page(&i.inner_impl(), w, use_absolute)?;
if show_def_docs {
for it in &i.inner_impl().items {
Expand All @@ -4014,22 +4013,18 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi
}
write!(w, "</code>")?;
} else {
write!(w, "<h3 id='{}' class='impl'><span class='in-band'><table class='table-display'>\
<tbody><tr><td><code>{}</code>",
id, i.inner_impl())?;
write!(w, "<h3 id='{}' class='impl'><code class='in-band'>{}</code>",
id, i.inner_impl()
)?;
}
write!(w, "<a href='#{}' class='anchor'></a>", id)?;
write!(w, "</td><td><span class='out-of-band'>")?;
let since = i.impl_item.stability.as_ref().map(|s| &s.since[..]);
render_stability_since_raw(w, since, outer_version)?;
if let Some(l) = (Item { item: &i.impl_item, cx: cx }).src_href() {
write!(w, "<div class='ghost'></div>")?;
render_stability_since_raw(w, since, outer_version)?;
write!(w, "<a class='srclink' href='{}' title='{}'>[src]</a>",
l, "goto source code")?;
} else {
render_stability_since_raw(w, since, outer_version)?;
}
write!(w, "</span></td></tr></tbody></table></span></h3>")?;
write!(w, "</h3>")?;
if let Some(ref dox) = cx.shared.maybe_collapsed_doc_value(&i.impl_item) {
let mut ids = cx.id_map.borrow_mut();
write!(w, "<div class='docblock'>{}</div>",
Expand Down Expand Up @@ -4065,20 +4060,15 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi
let ns_id = cx.derive_id(format!("{}.{}", name, item_type.name_space()));
write!(w, "<h4 id='{}' class=\"{}{}\">", id, item_type, extra_class)?;
write!(w, "{}", spotlight_decl(decl)?)?;
write!(w, "<table id='{}' class='table-display'><tbody><tr><td><code>", ns_id)?;
write!(w, "<code id='{}'>", ns_id)?;
render_assoc_item(w, item, link.anchor(&id), ItemType::Impl)?;
write!(w, "</code>")?;
render_stability_since_raw(w, item.stable_since(), outer_version)?;
if let Some(l) = (Item { cx, item }).src_href() {
write!(w, "</td><td><span class='out-of-band'>")?;
write!(w, "<div class='ghost'></div>")?;
render_stability_since_raw(w, item.stable_since(), outer_version)?;
write!(w, "<a class='srclink' href='{}' title='{}'>[src]</a></span>",
write!(w, "<a class='srclink' href='{}' title='{}'>[src]</a>",
l, "goto source code")?;
} else {
write!(w, "</td><td>")?;
render_stability_since_raw(w, item.stable_since(), outer_version)?;
}
write!(w, "</td></tr></tbody></table></h4>")?;
write!(w, "</h4>")?;
}
}
clean::TypedefItem(ref tydef, _) => {
Expand All @@ -4090,40 +4080,18 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi
write!(w, "</code></h4>")?;
}
clean::AssociatedConstItem(ref ty, ref default) => {
let mut version = String::new();

render_stability_since_raw(&mut version, item.stable_since(), outer_version)?;

let id = cx.derive_id(format!("{}.{}", item_type, name));
let ns_id = cx.derive_id(format!("{}.{}", name, item_type.name_space()));
write!(w, "<h4 id='{}' class=\"{}{}\">", id, item_type, extra_class)?;
if !version.is_empty() {
write!(w, "<table id='{}' class='table-display'><tbody><tr><td><code>", ns_id)?;
} else {
write!(w, "<code id='{}'>", ns_id)?;
}
write!(w, "<code id='{}'>", ns_id)?;
assoc_const(w, item, ty, default.as_ref(), link.anchor(&id))?;
if !version.is_empty() {
write!(w, "</code>")?;
}
let src = if let Some(l) = (Item { cx, item }).src_href() {
if !version.is_empty() {
write!(w, "</td><td><span class='out-of-band'>")?;
write!(w, "<div class='ghost'></div>{}", version)?;
}
format!("<a class='srclink' href='{}' title='{}'>[src]</a>",
l, "goto source code")
} else {
if !version.is_empty() {
write!(w, "</td><td>{}", version)?;
}
String::new()
};
if version.is_empty() {
write!(w, "</code>{}</h4>", src)?;
} else {
write!(w, "{}</span></td></tr></tbody></table></h4>", src)?;
write!(w, "</code>")?;
render_stability_since_raw(w, item.stable_since(), outer_version)?;
if let Some(l) = (Item { cx, item }).src_href() {
write!(w, "<a class='srclink' href='{}' title='{}'>[src]</a>",
l, "goto source code")?;
}
write!(w, "</h4>")?;
}
clean::AssociatedTypeItem(ref bounds, ref default) => {
let id = cx.derive_id(format!("{}.{}", item_type, name));
Expand Down
11 changes: 10 additions & 1 deletion src/librustdoc/html/static/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -2409,8 +2409,17 @@ if (!DOMTokenList.prototype.remove) {
e.remove();
});
onEachLazy(main.childNodes, function(e) {
// Unhide the actual content once loading is complete. Headers get
// flex treatment for their horizontal layout, divs get block treatment
// for vertical layout (column-oriented flex layout for divs caused
// errors in mobile browsers).
if (e.tagName === "H2" || e.tagName === "H3") {
e.nextElementSibling.style.display = "block";
let nextTagName = e.nextElementSibling.tagName;
if (nextTagName == "H2" || nextTagName == "H3") {
e.nextElementSibling.style.display = "flex";
} else {
e.nextElementSibling.style.display = "block";
}
}
});
}
Expand Down
53 changes: 39 additions & 14 deletions src/librustdoc/html/static/rustdoc.css
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,9 @@ h2, h3:not(.impl):not(.method):not(.type):not(.tymethod), h4:not(.method):not(.t
border-bottom: 1px solid;
}
h3.impl, h3.method, h4.method, h3.type, h4.type, h4.associatedconstant {
flex-basis: 100%;
font-weight: 600;
margin-top: 10px;
margin-top: 16px;
margin-bottom: 10px;
position: relative;
}
Expand Down Expand Up @@ -356,7 +357,8 @@ nav.sub {
#main > .docblock h3, #main > .docblock h4, #main > .docblock h5 { font-size: 1em; }

#main > h2 + div, #main > h2 + h3, #main > h3 + div {
display: none;
display: none; /* Changed to flex or block via js once the page is loaded */
flex-wrap: wrap;
}

.docblock h1 { font-size: 1em; }
Expand Down Expand Up @@ -390,7 +392,7 @@ h4 > code, h3 > code, .invisible > code {
}

.in-band, code {
z-index: 5;
z-index: -5;
}

.invisible {
Expand Down Expand Up @@ -534,6 +536,10 @@ h4 > code, h3 > code, .invisible > code {
margin-top: -8px;
}

.impl-items {
flex-basis: 100%;
}

#main > .stability {
margin-top: 0;
}
Expand Down Expand Up @@ -784,6 +790,33 @@ body.blur > :not(#help) {
top: 0;
}

.impl-items .since, .impl .since {
flex-grow: 0;
padding-left: 12px;
padding-right: 2px;
position: initial;
}

.impl-items .srclink, .impl .srclink {
flex-grow: 0;
/* Override header settings otherwise it's too bold */
font-size: 17px;
font-weight: normal;
}

.impl-items code, .impl code {
flex-grow: 1;
}

.impl-items h4, h4.impl, h3.impl {
display: flex;
flex-basis: 100%;
font-size: 16px;
margin-bottom: 12px;
/* Push the src link out to the right edge consistently */
justify-content: space-between;
}

.variants_table {
width: 100%;
}
Expand Down Expand Up @@ -871,15 +904,6 @@ h3 > .collapse-toggle, h4 > .collapse-toggle {
margin-left: 20px;
}

.ghost {
display: none;
}

.ghost + .since {
position: initial;
display: table-cell;
}

.since + .srclink {
display: table-cell;
padding-left: 10px;
Expand Down Expand Up @@ -1119,7 +1143,7 @@ span.since {
margin-left: 5px;
top: -5px;
left: 105%;
z-index: 1;
z-index: 10;
}

.tooltip:hover .tooltiptext {
Expand Down Expand Up @@ -1361,8 +1385,9 @@ h3.important {
margin-top: 16px;
}

.content > .methods > div.important-traits {
.content > .methods > .method > div.important-traits {
position: absolute;
font-weight: 400;
left: -42px;
margin-top: 2px;
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/rustdoc/assoc-consts-version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
pub struct SomeStruct;

impl SomeStruct {
// @has 'foo/struct.SomeStruct.html' '//*[@id="SOME_CONST.v"]//div[@class="since"]' '1.1.2'
// @has 'foo/struct.SomeStruct.html' '//*[@id="associatedconstant.SOME_CONST"]//div[@class="since"]' '1.1.2'
#[stable(since="1.1.2", feature="rust2")]
pub const SOME_CONST: usize = 0;
}
2 changes: 1 addition & 1 deletion src/test/rustdoc/const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
pub struct Foo;

impl Foo {
// @has const/struct.Foo.html '//*[@id="new.v"]//code' 'const unsafe fn new'
// @has const/struct.Foo.html '//code[@id="new.v"]' 'const unsafe fn new'
pub const unsafe fn new() -> Foo {
Foo
}
Expand Down
6 changes: 3 additions & 3 deletions src/test/rustdoc/issue-25001.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@ pub trait Bar {

impl Foo<u8> {
// @has - '//*[@id="method.pass"]//code' 'fn pass()'
// @has - '//*[@id="pass.v"]//code' 'fn pass()'
// @has - '//code[@id="pass.v"]' 'fn pass()'
pub fn pass() {}
}
impl Foo<u16> {
// @has - '//*[@id="method.pass-1"]//code' 'fn pass() -> usize'
// @has - '//*[@id="pass.v-1"]//code' 'fn pass() -> usize'
// @has - '//code[@id="pass.v-1"]' 'fn pass() -> usize'
pub fn pass() -> usize { 42 }
}
impl Foo<u32> {
// @has - '//*[@id="method.pass-2"]//code' 'fn pass() -> isize'
// @has - '//*[@id="pass.v-2"]//code' 'fn pass() -> isize'
// @has - '//code[@id="pass.v-2"]' 'fn pass() -> isize'
pub fn pass() -> isize { 42 }
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/rustdoc/issue-51236.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ pub mod traits {
}

// @has issue_51236/struct.Owned.html
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//*/code' "impl<T> Send for \
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//code' "impl<T> Send for \
// Owned<T> where <T as Owned<'static>>::Reader: Send"
pub struct Owned<T> where T: for<'a> ::traits::Owned<'a> {
marker: PhantomData<<T as ::traits::Owned<'static>>::Reader>,
Expand Down
4 changes: 2 additions & 2 deletions src/test/rustdoc/issue-54705.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ pub trait ScopeHandle<'scope> {}


// @has issue_54705/struct.ScopeFutureContents.html
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//*/code' "impl<'scope, S> \
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//code' "impl<'scope, S> \
// Send for ScopeFutureContents<'scope, S> where S: Sync"
//
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//*/code' "impl<'scope, S> \
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//code' "impl<'scope, S> \
// Sync for ScopeFutureContents<'scope, S> where S: Sync"
pub struct ScopeFutureContents<'scope, S>
where S: ScopeHandle<'scope>,
Expand Down
8 changes: 4 additions & 4 deletions src/test/rustdoc/issue-55321.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
#![feature(optin_builtin_traits)]

// @has issue_55321/struct.A.html
// @has - '//*[@id="implementations-list"]/*[@class="impl"]//*/code' "impl !Send for A"
// @has - '//*[@id="implementations-list"]/*[@class="impl"]//*/code' "impl !Sync for A"
// @has - '//*[@id="implementations-list"]/*[@class="impl"]//code' "impl !Send for A"
// @has - '//*[@id="implementations-list"]/*[@class="impl"]//code' "impl !Sync for A"
pub struct A();

impl !Send for A {}
impl !Sync for A {}

// @has issue_55321/struct.B.html
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//*/code' "impl<T> !Send for \
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//code' "impl<T> !Send for \
// B<T>"
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//*/code' "impl<T> !Sync for \
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//code' "impl<T> !Sync for \
// B<T>"
pub struct B<T: ?Sized>(A, Box<T>);
2 changes: 1 addition & 1 deletion src/test/rustdoc/issue-56822.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ impl<'a, T> MyTrait for Inner<'a, T> {
}

// @has issue_56822/struct.Parser.html
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//*/code' "impl<'a> Send for \
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//code' "impl<'a> Send for \
// Parser<'a>"
pub struct Parser<'a> {
field: <Wrapper<Inner<'a, u8>> as MyTrait>::Output
Expand Down
2 changes: 1 addition & 1 deletion src/test/rustdoc/synthetic_auto/complex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ mod foo {
}

// @has complex/struct.NotOuter.html
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//*/code' "impl<'a, T, K: \
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//code' "impl<'a, T, K: \
// ?Sized> Send for NotOuter<'a, T, K> where K: for<'b> Fn((&'b bool, &'a u8)) \
// -> &'b i8, T: MyTrait<'a>, <T as MyTrait<'a>>::MyItem: Copy, 'a: 'static"

Expand Down
4 changes: 2 additions & 2 deletions src/test/rustdoc/synthetic_auto/lifetimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ where
{}

// @has lifetimes/struct.Foo.html
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//*/code' "impl<'c, K> Send \
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//code' "impl<'c, K> Send \
// for Foo<'c, K> where K: for<'b> Fn(&'b bool) -> &'c u8, 'c: 'static"
//
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//*/code' "impl<'c, K> Sync \
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//code' "impl<'c, K> Sync \
// for Foo<'c, K> where K: Sync"
pub struct Foo<'c, K: 'c> {
inner_field: Inner<'c, K>,
Expand Down
4 changes: 2 additions & 2 deletions src/test/rustdoc/synthetic_auto/manual.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// @has manual/struct.Foo.html
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//*/code' 'impl<T> Sync for \
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//code' 'impl<T> Sync for \
// Foo<T> where T: Sync'
//
// @has - '//*[@id="implementations-list"]/*[@class="impl"]//*/code' \
// @has - '//*[@id="implementations-list"]/*[@class="impl"]//code' \
// 'impl<T> Send for Foo<T>'
//
// @count - '//*[@id="implementations-list"]/*[@class="impl"]' 1
Expand Down
4 changes: 2 additions & 2 deletions src/test/rustdoc/synthetic_auto/negative.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ pub struct Inner<T: Copy> {
}

// @has negative/struct.Outer.html
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//*/code' "impl<T> !Send for \
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//code' "impl<T> !Send for \
// Outer<T>"
//
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//*/code' "impl<T> \
// @has - '//*[@id="synthetic-implementations-list"]/*[@class="impl"]//code' "impl<T> \
// !Sync for Outer<T>"
pub struct Outer<T: Copy> {
inner_field: Inner<T>,
Expand Down
Loading

0 comments on commit 6d00124

Please sign in to comment.