-
-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[rustdoc] Only generate search DOM elements if the search is actually needed #160639
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
base: main
Are you sure you want to change the base?
Changes from all commits
40952ff
d83d804
730e9a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -245,6 +245,32 @@ function preLoadCss(cssUrl) { | |
| window.searchState = { | ||
| rustdocToolbar: document.querySelector("rustdoc-toolbar"), | ||
| loadingText: "Loading search results...", | ||
| searchLoaded: false, | ||
| loadSearch() { | ||
| // If you're browsing the nightly docs, the page might need to be refreshed for | ||
| // the search to work because the hash of the JS scripts might have changed. | ||
| function sendSearchForm() { | ||
| // @ts-expect-error | ||
| document.getElementsByClassName("search-form")[0].submit(); | ||
| } | ||
| if (!window.searchState.searchLoaded) { | ||
| window.searchState.searchLoaded = true; | ||
| window.rr_ = data => { | ||
| window.searchIndex = data; | ||
| }; | ||
| if (!window.StringdexOnload) { | ||
| window.StringdexOnload = []; | ||
| } | ||
| window.StringdexOnload.push(() => { | ||
| loadScript( | ||
| getVar("static-root-path") + getVar("search-js"), | ||
| sendSearchForm, | ||
| ); | ||
| }); | ||
|
Comment on lines
+261
to
+269
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the structure of this doesn't make a ton of sense to me in combination, shouldn't we just be initializing it to an array with a single element? otherwise we could hypothetically end up with an array of duplicate initializers?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just moved existing code, didn't modify anything. I think it's because you can have multiple search indexes loaded in parallel. |
||
| loadScript(getVar("static-root-path") + getVar("stringdex-js"), sendSearchForm); | ||
| loadScript(resourcePath("search.index/root", ".js"), sendSearchForm); | ||
| } | ||
| }, | ||
| inputElement: () => { | ||
| let el = document.getElementsByClassName("search-input")[0]; | ||
| if (!el) { | ||
|
|
@@ -269,6 +295,10 @@ function preLoadCss(cssUrl) { | |
| </nav><div class="search-switcher"></div>`; | ||
| out.insertBefore(hdr, window.searchState.outputElement()); | ||
| el = document.getElementsByClassName("search-input")[0]; | ||
|
|
||
| el.addEventListener("focus", () => { | ||
| window.searchState.loadSearch(); | ||
| }); | ||
| } | ||
| if (el instanceof HTMLInputElement) { | ||
| return el; | ||
|
|
@@ -391,41 +421,6 @@ function preLoadCss(cssUrl) { | |
| return params; | ||
| }, | ||
| setup: () => { | ||
| let searchLoaded = false; | ||
| const search_input = window.searchState.inputElement(); | ||
| if (!search_input) { | ||
| return; | ||
| } | ||
| // If you're browsing the nightly docs, the page might need to be refreshed for the | ||
| // search to work because the hash of the JS scripts might have changed. | ||
| function sendSearchForm() { | ||
| // @ts-expect-error | ||
| document.getElementsByClassName("search-form")[0].submit(); | ||
| } | ||
| function loadSearch() { | ||
| if (!searchLoaded) { | ||
| searchLoaded = true; | ||
| window.rr_ = data => { | ||
| window.searchIndex = data; | ||
| }; | ||
| if (!window.StringdexOnload) { | ||
| window.StringdexOnload = []; | ||
| } | ||
| window.StringdexOnload.push(() => { | ||
| loadScript( | ||
| getVar("static-root-path") + getVar("search-js"), | ||
| sendSearchForm, | ||
| ); | ||
| }); | ||
| loadScript(getVar("static-root-path") + getVar("stringdex-js"), sendSearchForm); | ||
| loadScript(resourcePath("search.index/root", ".js"), sendSearchForm); | ||
| } | ||
| } | ||
|
|
||
| search_input.addEventListener("focus", () => { | ||
| loadSearch(); | ||
| }); | ||
|
|
||
| const btn = document.getElementById("search-button"); | ||
| if (btn) { | ||
| btn.onclick = event => { | ||
|
|
@@ -434,7 +429,7 @@ function preLoadCss(cssUrl) { | |
| } | ||
| event.preventDefault(); | ||
| window.searchState.toggle(); | ||
| loadSearch(); | ||
| window.searchState.loadSearch(); | ||
| }; | ||
| } | ||
|
|
||
|
|
@@ -455,7 +450,7 @@ function preLoadCss(cssUrl) { | |
| // previous state with nothing in the bar. | ||
| const inputElement = window.searchState.inputElement(); | ||
| if (params.search !== undefined && inputElement !== null) { | ||
| loadSearch(); | ||
| window.searchState.loadSearch(); | ||
| inputElement.value = params.search; | ||
| // Some browsers fire "onpopstate" for every page load | ||
| // (Chrome), while others fire the event only when actually | ||
|
|
@@ -482,29 +477,32 @@ function preLoadCss(cssUrl) { | |
| // that try to sync state between the URL and the search input. To work around it, | ||
| // do a small amount of re-init on page show. | ||
| window.onpageshow = () => { | ||
| const inputElement = window.searchState.inputElement(); | ||
| const qSearch = window.searchState.getQueryStringParams().search; | ||
| if (qSearch !== undefined && inputElement !== null) { | ||
| if (inputElement.value === "") { | ||
| inputElement.value = qSearch; | ||
| } | ||
| window.searchState.showResults(); | ||
| if (qSearch === "") { | ||
| loadSearch(); | ||
| window.searchState.focus(); | ||
| if (qSearch !== undefined) { | ||
| const inputElement = window.searchState.inputElement(); | ||
| if (inputElement !== null) { | ||
| if (inputElement.value === "") { | ||
| inputElement.value = qSearch; | ||
| } | ||
| window.searchState.showResults(); | ||
| if (qSearch === "") { | ||
| window.searchState.loadSearch(); | ||
| window.searchState.focus(); | ||
| } | ||
| } | ||
| } else { | ||
| window.searchState.hideResults(); | ||
| } | ||
| }; | ||
|
|
||
| const params = window.searchState.getQueryStringParams(); | ||
| if (params.search !== undefined) { | ||
| window.searchState.setLoadingSearch(); | ||
| loadSearch(); | ||
| window.searchState.loadSearch(); | ||
| } | ||
| }, | ||
| setLoadingSearch: () => { | ||
| // We set up the search input before adding the other search elements (like | ||
| // "search loading") in case it's not already there yet. | ||
| window.searchState.inputElement(); | ||
| const search = window.searchState.outputElement(); | ||
| nonnull(search).innerHTML = "<h3 class=\"search-loading\">" + | ||
| window.searchState.loadingText + "</h3>"; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| // This test ensures that when clicking on a link which leads to an item inside a collapsed element, | ||
| // the collapsed element will be expanded. | ||
| go-to: "file://" + |DOC_PATH| + "/test_docs/struct.Foo.html" | ||
| include: "utils.goml" | ||
| // We check that the implementors block is expanded. | ||
| assert-property: ("#implementations-list .implementors-toggle", {"open": "true"}) | ||
| // We now collapse the implementors block. | ||
|
|
@@ -16,6 +17,7 @@ define-function: ("collapsed-from-search", [], block { | |
| // Then we collapse the section again... | ||
| set-property: ("#implementations-list .implementors-toggle", {"open": "false"}) | ||
| // Then we run the search. | ||
| call-function: ("open-search", {}) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah that was my original "quest": looking at fixing the new flakyness which happened in #159593 (comment). ^^' |
||
| write-into: (".search-input", "foo::must_use") | ||
| wait-for: "//*[@id='search']//a[@href='../test_docs/struct.Foo.html#method.must_use']" | ||
| click: "//*[@id='search']//a[@href='../test_docs/struct.Foo.html#method.must_use']" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| // This test ensures that the search elements are not created in the DOM before being needed. | ||
|
|
||
| go-to: "file://" + |DOC_PATH| + "/test_docs/index.html" | ||
| store-value: (search_selector, "#search") | ||
|
|
||
| // This selector is not supposed to exist yet. | ||
| assert-false: |search_selector| | ||
|
|
||
| // It should be generated when the search "begins". | ||
| click: "#search-button" | ||
| wait-for: |search_selector| | ||
|
|
||
| // When we arrive on a page with a search query parameter, the element should also be present. | ||
| go-to: "file://" + |DOC_PATH| + "/test_docs/index.html?search=a" | ||
| wait-for: |search_selector| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: technically this doesn't just apply to nightly, it's just way more common there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I copied comment from here. But yeah agreed.