Skip to content

Commit

Permalink
This commit changes the layout to something a bit less "look at my lo…
Browse files Browse the repository at this point in the history
…go!!!111"

gigantic, and makes it clearer where clicking the logo will actually take you.
It also means the crate name is persistently at the top of the sidebar, even
when in a sub-item page, and clicking that name takes you back to the root.

|         | Short crate name | Long crate name |
|---------|------------------|-----------------|
| Root    | ![short-root]    | ![long-root]
| Subpage | ![short-subpage] | ![long-subpage]

[short-root]: https://github.com/rust-lang/rust/assets/1593513/fe2ce102-d4b8-44e6-9f7b-68636a907f56
[short-subpage]: https://github.com/rust-lang/rust/assets/1593513/29501663-56c0-4151-b7de-d2637e167125
[long-root]: https://github.com/rust-lang/rust/assets/1593513/f6a385c0-b4c5-4a9c-954b-21b38de4192f
[long-subpage]: https://github.com/rust-lang/rust/assets/1593513/97ec47b4-61bf-4ebe-b461-0d2187b8c6ca

https://notriddle.com/rustdoc-html-demo-4/logo-lockup/image/index.html

https://notriddle.com/rustdoc-html-demo-4/logo-lockup/crossbeam_channel/index.html

https://notriddle.com/rustdoc-html-demo-4/logo-lockup/adler/struct.Adler32.html

https://notriddle.com/rustdoc-html-demo-4/logo-lockup/crossbeam_channel/struct.Sender.html

This improves visual information density (the construct with the logo and
crate name is *shorter* than the logo on its own, because it's not
square) and navigation clarity (we can now see what clicking the Rust logo
does, specifically).

Compare this with the layout at [Phoenix's Hexdocs] (which is what this
proposal is closely based on), the old proposal on [Internals Discourse]
(which always says "Rust standard library" in the sidebar, but doesn't do the
side-by-side layout).

[Phoenix's Hexdocs]: https://hexdocs.pm/phoenix/1.7.7/overview.html
[Internals Discourse]: https://internals.rust-lang.org/t/poc-of-a-new-design-for-the-generated-rustdoc/11018

In newer versions of rustdoc, the crate name and version are always shown in
the sidebar, even in subpages. Clicking the crate name does the same thing
clicking the logo always did: return you to the crate root.

While this actually takes up less screen real estate than the old layout on
desktop, it takes up more HTML. It's also a bit more visually complex.

I could do what the Internals POC did and keep the vertically stacked layout
all the time, instead of doing a horizontal stack where possible. It would
take up more screen real estate, though.

This design is lifted almost verbatim from Hexdocs. It seems to work for them.
[`opentelemetry_process_propagator`], for example, has a long application name.

[`opentelemetry_process_propagator`]: https://hexdocs.pm/opentelemetry_process_propagator/OpentelemetryProcessPropagator.html

Has anyone written the rationale on why the Rust logo shows up on projects that
aren't the standard library? If we turned it off on non-standard crates by
default, it would line wrap crate names a lot less often.

Or maybe we should encourage crate authors to include their own logo more
often? It certainly helps give people a better sense of "place."

I'm not sure of anything that directly follows up this one. Plenty of other
changes could be made to improve the layout, like

* coming up with a less cluttered way to do disclosure (there's a lot of `[-]`
  on the page)
* doing a better job of separating lateral navigation (vec::Vec links to
  vec::IntoIter) and the table of contents (vec::Vec links to vec::Vec::new)
* giving readers more control of how much rustdoc hows them, and giving doc
  authors more control of how much it generates
* better search that reduces the need to browse

But those are mostly orthogonal, not future possibilities unlocked by this change.
  • Loading branch information
notriddle committed Sep 18, 2023
1 parent b1575cb commit b91c786
Show file tree
Hide file tree
Showing 13 changed files with 95 additions and 48 deletions.
1 change: 1 addition & 0 deletions src/librustdoc/html/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ pub(crate) struct Layout {
pub(crate) external_html: ExternalHtml,
pub(crate) default_settings: FxHashMap<String, String>,
pub(crate) krate: String,
pub(crate) krate_version: String,
/// The given user css file which allow to customize the generated
/// documentation theme.
pub(crate) css_file_extension: Option<PathBuf>,
Expand Down
6 changes: 3 additions & 3 deletions src/librustdoc/html/render/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
external_html,
default_settings,
krate: krate.name(tcx).to_string(),
krate_version: cache.crate_version.as_deref().unwrap_or_default().to_string(),
css_file_extension: extension_css,
scrape_examples_extension: !call_locations.is_empty(),
};
Expand Down Expand Up @@ -669,10 +670,9 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {

let blocks = sidebar_module_like(all.item_sections());
let bar = Sidebar {
title_prefix: "Crate ",
title: crate_name.as_str(),
title_prefix: "",
title: "",
is_crate: false,
version: "",
blocks: vec![blocks],
path: String::new(),
};
Expand Down
9 changes: 3 additions & 6 deletions src/librustdoc/html/render/sidebar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ pub(super) struct Sidebar<'a> {
pub(super) title_prefix: &'static str,
pub(super) title: &'a str,
pub(super) is_crate: bool,
pub(super) version: &'a str,
pub(super) blocks: Vec<LinkBlock<'a>>,
pub(super) path: String,
}
Expand Down Expand Up @@ -99,12 +98,12 @@ pub(super) fn print_sidebar(cx: &Context<'_>, it: &clean::Item, buffer: &mut Buf
|| it.is_primitive()
|| it.is_union()
|| it.is_enum()
|| it.is_mod()
// crate title is displayed as part of logo lockup
|| (it.is_mod() && !it.is_crate())
|| it.is_type_alias()
{
(
match *it.kind {
clean::ModuleItem(..) if it.is_crate() => "Crate ",
clean::ModuleItem(..) => "Module ",
_ => "",
},
Expand All @@ -113,14 +112,12 @@ pub(super) fn print_sidebar(cx: &Context<'_>, it: &clean::Item, buffer: &mut Buf
} else {
("", "")
};
let version =
if it.is_crate() { cx.cache().crate_version.as_deref().unwrap_or_default() } else { "" };
let path: String = if !it.is_mod() {
cx.current.iter().map(|s| s.as_str()).intersperse("::").collect()
} else {
"".into()
};
let sidebar = Sidebar { title_prefix, title, is_crate: it.is_crate(), version, blocks, path };
let sidebar = Sidebar { title_prefix, title, is_crate: it.is_crate(), blocks, path };
sidebar.render_into(buffer).unwrap();
}

Expand Down
57 changes: 45 additions & 12 deletions src/librustdoc/html/static/css/rustdoc.css
Original file line number Diff line number Diff line change
Expand Up @@ -461,19 +461,9 @@ img {
display: none !important;
}

.sidebar .logo-container {
margin-top: 10px;
margin-bottom: 10px;
text-align: center;
}

.version {
overflow-wrap: break-word;
}

.logo-container > img {
height: 100px;
width: 100px;
height: 48px;
width: 48px;
}

ul.block, .block li {
Expand Down Expand Up @@ -510,6 +500,8 @@ ul.block, .block li {
color: var(--sidebar-link-color);
}
.sidebar .current,
.sidebar .current a,
.sidebar-crate a.logo-container:hover + h2 a,
.sidebar a:hover:not(.logo-container) {
background-color: var(--sidebar-current-link-background-color);
}
Expand All @@ -524,6 +516,47 @@ ul.block, .block li {
overflow: hidden;
}

.sidebar-crate {
display: flex;
align-items: center;
justify-content: center;
margin: 0 32px;
column-gap: 32px;
flex-wrap: wrap;
}

.sidebar-crate h2 {
flex-grow: 1;
/* This setup with the margins and row-gap is designed to make flex-wrap
work the way we want. If they're in the side-by-side lockup, there
should be a 16px margin to the left of the logo (visually the same as
the 24px one on everything else, which are not giant circles) and 8px
between it and the crate's name and version. When they're line wrapped,
the logo needs to have the same margin on both sides of itself (to
center properly) and the crate name and version need 24px on their
left margin. */
margin: 0 -8px;
}

.sidebar-crate .logo-container {
margin: 10px -16px;
text-align: center;
}

.sidebar-crate h2 a {
display: block;
margin-left: -0.25rem;
padding-left: 0.25rem;
margin-right: -24px;
}

.sidebar-crate h2 .version {
display: block;
font-weight: normal;
font-size: 1rem;
overflow-wrap: break-word;
}

.mobile-topbar {
display: none;
}
Expand Down
8 changes: 6 additions & 2 deletions src/librustdoc/html/static/js/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,12 @@ function setMobileTopbar() {
// but with the current code it's hard to get the right information in the right place.
const mobileLocationTitle = document.querySelector(".mobile-topbar h2");
const locationTitle = document.querySelector(".sidebar h2.location");
if (mobileLocationTitle && locationTitle) {
mobileLocationTitle.innerHTML = locationTitle.innerHTML;
if (mobileLocationTitle) {
if (hasClass(document.body, "crate")) {
mobileLocationTitle.innerText = `Crate ${window.currentCrate}`;
} else if (locationTitle) {
mobileLocationTitle.innerHTML = locationTitle.innerHTML;
}
}
}

Expand Down
22 changes: 15 additions & 7 deletions src/librustdoc/html/templates/page.html
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,21 @@ <h2></h2> {# #}
{% endif %}
<nav class="sidebar"> {# #}
{% if page.css_class != "src" %}
<a class="logo-container" href="{{page.root_path|safe}}{{krate_with_trailing_slash|safe}}index.html"> {# #}
{% if !layout.logo.is_empty() %}
<img src="{{layout.logo}}" alt="logo"> {# #}
{% else %}
<img class="rust-logo" src="{{static_root_path|safe}}{{files.rust_logo_svg}}" alt="logo"> {# #}
{% endif %}
</a> {# #}
<div class="sidebar-crate">
<a class="logo-container" href="{{page.root_path|safe}}{{krate_with_trailing_slash|safe}}index.html"> {# #}
{% if !layout.logo.is_empty() %}
<img src="{{layout.logo}}" alt="logo"> {# #}
{% else %}
<img class="rust-logo" src="{{static_root_path|safe}}{{files.rust_logo_svg}}" alt="logo"> {# #}
{% endif %}
</a> {# #}
<h2> {# #}
<a href="{{page.root_path|safe}}{{krate_with_trailing_slash|safe}}index.html">{{layout.krate}}</a> {# #}
{% if !layout.krate_version.is_empty() %}
<span class="version">{{+ layout.krate_version}}</span>
{% endif %}
</h2>
</div>
{% endif %}
{{ sidebar|safe }}
</nav> {# #}
Expand Down
3 changes: 0 additions & 3 deletions src/librustdoc/html/templates/sidebar.html
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@ <h2 class="location"> {# #}
<div class="sidebar-elems">
{% if is_crate %}
<ul class="block">
{% if !version.is_empty() %}
<li class="version">Version {{+ version}}</li>
{% endif %}
<li><a id="all-types" href="all.html">All Items</a></li> {# #}
</ul>
{% endif %}
Expand Down
4 changes: 2 additions & 2 deletions tests/rustdoc-gui/huge-logo.goml
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ go-to: "file://" + |DOC_PATH| + "/huge_logo/index.html"

set-window-size: (1280, 1024)
// offsetWidth = width of sidebar
assert-property: (".sidebar .logo-container", {"offsetWidth": "200", "offsetHeight": 100})
assert-property: (".sidebar .logo-container img", {"offsetWidth": "100", "offsetHeight": 100})
assert-property: (".sidebar-crate .logo-container", {"offsetWidth": "48", "offsetHeight": 48})
assert-property: (".sidebar-crate .logo-container img", {"offsetWidth": "48", "offsetHeight": 48})

set-window-size: (400, 600)
// offset = size + margin
Expand Down
2 changes: 1 addition & 1 deletion tests/rustdoc-gui/sidebar-mobile.goml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ assert-position: ("#method\.must_use", {"y": 46})
// Check that the bottom-most item on the sidebar menu can be scrolled fully into view.
click: ".sidebar-menu-toggle"
scroll-to: ".block.keyword li:nth-child(1)"
compare-elements-position-near: (".block.keyword li:nth-child(1)", ".mobile-topbar", {"y": 543.19})
compare-elements-position-near: (".block.keyword li:nth-child(1)", ".mobile-topbar", {"y": 544})

// Now checking the background color of the sidebar.
show-text: true
Expand Down
24 changes: 15 additions & 9 deletions tests/rustdoc-gui/sidebar.goml
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ set-local-storage: {"rustdoc-theme": "light"}
// We reload the page so the local storage settings are being used.
reload:

assert-text: (".sidebar > .location", "Crate test_docs")
// In modules, we only have one "location" element.
assert-count: (".sidebar .location", 1)
assert-text: (".sidebar > .sidebar-crate > h2 > a", "test_docs")
// Crate root has no "location" element
assert-count: (".sidebar .location", 0)
assert-count: (".sidebar h2", 1)
assert-text: ("#all-types", "All Items")
assert-css: ("#all-types", {"color": "#356da4"})
Expand All @@ -74,8 +74,9 @@ assert-text: ("#structs + .item-table .item-name > a", "Foo")
click: "#structs + .item-table .item-name > a"

// PAGE: struct.Foo.html
assert-count: (".sidebar .sidebar-crate", 1)
assert-count: (".sidebar .location", 1)
assert-count: (".sidebar h2", 2)
assert-count: (".sidebar h2", 3)
// We check that there is no crate listed outside of the top level.
assert-false: ".sidebar-elems > .crate"

Expand All @@ -94,7 +95,8 @@ click: ".sidebar-elems ul.crate > li:first-child > a"
// PAGE: lib2/index.html
go-to: "file://" + |DOC_PATH| + "/lib2/index.html"
assert-property: (".sidebar", {"clientWidth": "200"})
assert-text: (".sidebar > .location", "Crate lib2")
assert-text: (".sidebar > .sidebar-crate > h2 > a", "lib2")
assert-count: (".sidebar .location", 0)
// We check that we have the crates list and that the "current" on is now "lib2".
assert-text: (".sidebar-elems ul.crate > li > a.current", "lib2")
// We now go to the "foobar" function page.
Expand All @@ -108,21 +110,25 @@ click: "#functions + .item-table .item-name > a"

// PAGE: fn.foobar.html
// In items containing no items (like functions or constants) and in modules, we have no
// "location" elements. Only the parent module h2.
// "location" elements. Only the parent module h2 and crate.
assert-text: (".sidebar > .sidebar-crate > h2 > a", "lib2")
assert-count: (".sidebar .location", 0)
assert-count: (".sidebar h2", 1)
assert-count: (".sidebar h2", 2)
assert-text: (".sidebar .sidebar-elems h2", "In lib2")
// We check that we don't have the crate list.
assert-false: ".sidebar-elems > .crate"

go-to: "./module/index.html"
assert-property: (".sidebar", {"clientWidth": "200"})
assert-text: (".sidebar > .sidebar-crate > h2 > a", "lib2")
assert-text: (".sidebar > .location", "Module module")
assert-count: (".sidebar .location", 1)
// We check that we don't have the crate list.
assert-false: ".sidebar-elems > .crate"

go-to: "./sub_module/sub_sub_module/index.html"
assert-property: (".sidebar", {"clientWidth": "200"})
assert-text: (".sidebar > .sidebar-crate > h2 > a", "lib2")
assert-text: (".sidebar > .location", "Module sub_sub_module")
// We check that we don't have the crate list.
assert-false: ".sidebar-elems .crate"
Expand Down Expand Up @@ -152,14 +158,14 @@ assert-property: (".sidebar", {"clientWidth": "200"})

// Checks that all.html and index.html have their sidebar link in the same place.
go-to: "file://" + |DOC_PATH| + "/test_docs/index.html"
store-property: (".sidebar .location a", {
store-property: (".sidebar .sidebar-crate h2 a", {
"clientWidth": index_sidebar_width,
"clientHeight": index_sidebar_height,
"offsetTop": index_sidebar_y,
"offsetLeft": index_sidebar_x,
})
go-to: "file://" + |DOC_PATH| + "/test_docs/all.html"
assert-property: (".sidebar .location a", {
assert-property: (".sidebar .sidebar-crate h2 a", {
"clientWidth": |index_sidebar_width|,
"clientHeight": |index_sidebar_height|,
"offsetTop": |index_sidebar_y|,
Expand Down
2 changes: 1 addition & 1 deletion tests/rustdoc/crate-version-escape.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@

#![crate_name = "foo"]

// @has 'foo/index.html' '//li[@class="version"]' 'Version <script>alert("hi")</script>'
// @has 'foo/index.html' '//*[@class="version"]' '<script>alert("hi")</script>'
2 changes: 1 addition & 1 deletion tests/rustdoc/crate-version.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
// compile-flags: --crate-version=1.3.37

// @has 'crate_version/index.html' '//*[@class="version"]' 'Version 1.3.37'
// @has 'crate_version/index.html' '//*[@class="version"]' '1.3.37'
3 changes: 2 additions & 1 deletion tests/rustdoc/titles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
#![feature(rustc_attrs)]

// @matches 'foo/index.html' '//h1' 'Crate foo'
// @matches 'foo/index.html' '//h2[@class="location"]' 'Crate foo'
// @matches 'foo/index.html' '//div[@class="sidebar-crate"]/h2/a' 'foo'
// @count 'foo/index.html' '//h2[@class="location"]' 0

// @matches 'foo/foo_mod/index.html' '//h1' 'Module foo::foo_mod'
// @matches 'foo/foo_mod/index.html' '//h2[@class="location"]' 'Module foo_mod'
Expand Down

0 comments on commit b91c786

Please sign in to comment.