From 47c46324aa28a53893f600437a2f911b8e28ef6e Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Sat, 23 Sep 2023 12:59:58 -0700 Subject: [PATCH] rustdoc: clean up the In [name] up-pointer This commit makes three changes for consistency and readability: - It shows the sibling navigation on module pages. It's weird that it didn't work before, and is inconsistent with everything else (even Crates have sibling navigation with other Crates). - It hides the "In [parent]" header if it's the same as the current crate, and if there's no other header between them. We need to keep it on modules and types, since they have their own header and data between them, and we don't want to show siblings under a header implying that they're children. - It adds a margin to deal with the headers butting directly into the branding lockup. --- src/librustdoc/html/render/context.rs | 3 ++- src/librustdoc/html/render/sidebar.rs | 19 ++++++++++++++++--- src/librustdoc/html/render/write_shared.rs | 2 +- src/librustdoc/html/static/css/rustdoc.css | 2 +- src/librustdoc/html/static/js/main.js | 15 ++++++++++----- src/librustdoc/html/templates/page.html | 2 ++ src/librustdoc/html/templates/sidebar.html | 2 +- tests/rustdoc-gui/sidebar-mobile.goml | 2 +- tests/rustdoc-gui/sidebar.goml | 19 ++++++++++++++++--- 9 files changed, 50 insertions(+), 16 deletions(-) diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index da826054e66..bf8d1a80337 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -662,7 +662,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { let shared = Rc::clone(&self.shared); let mut page = layout::Page { title: "List of all items in this crate", - css_class: "mod", + css_class: "mod sys", root_path: "../", static_root_path: shared.static_root_path.as_deref(), description: "List of all items in this crate", @@ -677,6 +677,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { title_prefix: "", title: "", is_crate: false, + is_mod: false, blocks: vec![blocks], path: String::new(), }; diff --git a/src/librustdoc/html/render/sidebar.rs b/src/librustdoc/html/render/sidebar.rs index 417c572fd77..117a13f0975 100644 --- a/src/librustdoc/html/render/sidebar.rs +++ b/src/librustdoc/html/render/sidebar.rs @@ -19,6 +19,7 @@ pub(super) struct Sidebar<'a> { pub(super) title_prefix: &'static str, pub(super) title: &'a str, pub(super) is_crate: bool, + pub(super) is_mod: bool, pub(super) blocks: Vec>, pub(super) path: String, } @@ -112,12 +113,24 @@ pub(super) fn print_sidebar(cx: &Context<'_>, it: &clean::Item, buffer: &mut Buf } else { ("", "") }; - let path: String = if !it.is_mod() { - cx.current.iter().map(|s| s.as_str()).intersperse("::").collect() + // need to show parent path header if: + // - it's a child module, instead of the crate root + // - there's a sidebar section for the item itself + // + // otherwise, the parent path header is redundant with the big crate + // branding area at the top of the sidebar + let sidebar_path = if it.is_mod() { &cx.current[..cx.current.len() - 1] } else { &cx.current[..] }; + let path: String = if sidebar_path.len() > 1 || !title.is_empty() { + let path = sidebar_path.iter().map(|s| s.as_str()).intersperse("::").collect(); + if sidebar_path.len() == 1 { + format!("crate {path}") + } else { + path + } } else { "".into() }; - let sidebar = Sidebar { title_prefix, title, is_crate: it.is_crate(), blocks, path }; + let sidebar = Sidebar { title_prefix, title, is_mod: it.is_mod(), is_crate: it.is_crate(), blocks, path }; sidebar.render_into(buffer).unwrap(); } diff --git a/src/librustdoc/html/render/write_shared.rs b/src/librustdoc/html/render/write_shared.rs index eea7193ccbe..e68d5ab2fbd 100644 --- a/src/librustdoc/html/render/write_shared.rs +++ b/src/librustdoc/html/render/write_shared.rs @@ -336,7 +336,7 @@ if (typeof exports !== 'undefined') {exports.searchIndex = searchIndex}; let dst = cx.dst.join("index.html"); let page = layout::Page { title: "Index of crates", - css_class: "mod", + css_class: "mod sys", root_path: "./", static_root_path: shared.static_root_path.as_deref(), description: "List of crates", diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 0d9cb076087..de7586da62c 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -523,7 +523,7 @@ ul.block, .block li { justify-content: center; /* there's a 10px padding at the top of
, and a 4px margin at the top of the search form. To line them up, add them. */ - margin: 14px 32px 0; + margin: 14px 32px 1rem; row-gap: 10px; column-gap: 32px; flex-wrap: wrap; diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index 25e9da57a05..49b8644b076 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -457,22 +457,27 @@ function preLoadCss(cssUrl) { return; } + const modpath = hasClass(document.body, "mod") ? "../" : ""; + const h3 = document.createElement("h3"); - h3.innerHTML = `${longty}`; + h3.innerHTML = `${longty}`; const ul = document.createElement("ul"); ul.className = "block " + shortty; for (const name of filtered) { let path; if (shortty === "mod") { - path = name + "/index.html"; + path = `${modpath}${name}/index.html`; } else { - path = shortty + "." + name + ".html"; + path = `${modpath}${shortty}.${name}.html`; + } + let current_page = document.location.href.toString(); + if (current_page.endsWith("/")) { + current_page += "index.html"; } - const current_page = document.location.href.split("/").pop(); const link = document.createElement("a"); link.href = path; - if (path === current_page) { + if (link.href === current_page) { link.className = "current"; } link.textContent = name; diff --git a/src/librustdoc/html/templates/page.html b/src/librustdoc/html/templates/page.html index 612f8613482..ebf817673bf 100644 --- a/src/librustdoc/html/templates/page.html +++ b/src/librustdoc/html/templates/page.html @@ -42,6 +42,8 @@ {# #} {% else if !page.css_class.contains("mod") %} {# #} + {% else if !page.css_class.contains("sys") %} + {# #} {% endif %} {# #} {% if layout.scrape_examples_extension %} diff --git a/src/librustdoc/html/templates/sidebar.html b/src/librustdoc/html/templates/sidebar.html index 14a61f3643e..a99198141e2 100644 --- a/src/librustdoc/html/templates/sidebar.html +++ b/src/librustdoc/html/templates/sidebar.html @@ -29,6 +29,6 @@ {% endif %} {% if !path.is_empty() %} -

In {{+ path}}

+

In {{+ path}}

{% endif %} diff --git a/tests/rustdoc-gui/sidebar-mobile.goml b/tests/rustdoc-gui/sidebar-mobile.goml index beff07b555d..d3a82d9ebe6 100644 --- a/tests/rustdoc-gui/sidebar-mobile.goml +++ b/tests/rustdoc-gui/sidebar-mobile.goml @@ -26,7 +26,7 @@ assert-css: (".sidebar", {"left": "0px"}) // Make sure the "struct Foo" header is hidden, since the mobile topbar already does it. assert-css: ("//nav[contains(@class, 'sidebar')]//h2/a[text()='Foo']/parent::h2", {"display": "none"}) // Make sure the global navigation is still here. -assert-css: ("//nav[contains(@class, 'sidebar')]//h2/a[text()='In test_docs']/parent::h2", {"display": "block"}) +assert-css: ("//nav[contains(@class, 'sidebar')]//h2/a[text()='In crate test_docs']/parent::h2", {"display": "block"}) // Click elsewhere. click: "body" diff --git a/tests/rustdoc-gui/sidebar.goml b/tests/rustdoc-gui/sidebar.goml index 7050ee902a5..eff66d803d2 100644 --- a/tests/rustdoc-gui/sidebar.goml +++ b/tests/rustdoc-gui/sidebar.goml @@ -110,11 +110,11 @@ 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 and crate. +// "location" elements. Only the crate and optional parent module. +// This page, being directly below the crate, only has its heading. assert-text: (".sidebar > .sidebar-crate > h2 > a", "lib2") assert-count: (".sidebar .location", 0) -assert-count: (".sidebar h2", 2) -assert-text: (".sidebar .sidebar-elems h2", "In lib2") +assert-count: (".sidebar h2", 1) // We check that we don't have the crate list. assert-false: ".sidebar-elems > .crate" @@ -123,6 +123,15 @@ assert-property: (".sidebar", {"clientWidth": "200"}) assert-text: (".sidebar > .sidebar-crate > h2 > a", "lib2") assert-text: (".sidebar > .location", "Module module") assert-count: (".sidebar .location", 1) +// Module page requires three headings: +// - Presistent crate branding (name and version) +// - Module name, followed by TOC for module headings +// - "In crate [name]" parent pointer, followed by sibling navigation +assert-count: (".sidebar h2", 3) +assert-text: (".sidebar > .sidebar-elems > h2", "In crate lib2") +assert-property: (".sidebar > .sidebar-elems > h2 > a", { + "href": "/lib2/index.html", +}, ENDS_WITH) // We check that we don't have the crate list. assert-false: ".sidebar-elems > .crate" @@ -130,6 +139,10 @@ 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") +assert-text: (".sidebar > .sidebar-elems > h2", "In lib2::module::sub_module") +assert-property: (".sidebar > .sidebar-elems > h2 > a", { + "href": "/module/sub_module/index.html", +}, ENDS_WITH) // We check that we don't have the crate list. assert-false: ".sidebar-elems .crate" assert-text: (".sidebar-elems > section ul > li:nth-child(1)", "Functions")