-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
rustdoc: fixes #64305: disable search field instead of hidding it #66298
Conversation
a67c3bd
to
09cfb78
Compare
src/librustdoc/html/static/main.js
Outdated
onEachLazy(document.getElementsByClassName("js-only"), function(e) { | ||
removeClass(e, "js-only"); | ||
onEachLazy([getSearchInput()], function(e) { | ||
e.removeAttribute('disabled'); |
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.
Can you please move this block in the function addSearchOptions
? I just realized that there is no point into enabling the search bar if the content wasn't loaded yet.
src/librustdoc/html/static/main.js
Outdated
@@ -142,8 +142,8 @@ function getSearchElement() { | |||
var TY_PRIMITIVE = itemTypes.indexOf("primitive"); | |||
var TY_KEYWORD = itemTypes.indexOf("keyword"); | |||
|
|||
onEachLazy(document.getElementsByClassName("js-only"), function(e) { | |||
removeClass(e, "js-only"); | |||
onEachLazy([getSearchInput()], function(e) { |
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.
Also: no need anymore for onEachLazy
: just grab the output of getSearchOutput
and use it:
var e = getSearchInput();
if (e) {
e.removeAttribute('disabled');
}
09cfb78
to
0ee15e5
Compare
src/librustdoc/html/static/main.js
Outdated
@@ -2627,6 +2623,11 @@ function getSearchElement() { | |||
option.innerText = crates_text[i]; | |||
elem.appendChild(option); | |||
} | |||
|
|||
var e = getSearchInput(); |
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.
At this stage, I think you can used the search_input
variable created at line 83. Otherwise, seems all good to me, thanks a lot!
0ee15e5
to
2e131ff
Compare
2e131ff
to
221bcfb
Compare
src/librustdoc/html/static/main.js
Outdated
|
||
if (search_input) { | ||
search_input.removeAttribute('disabled'); | ||
}; |
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.
Indent issue. :p
221bcfb
to
8d4d8f3
Compare
…ng it Signed-off-by: Maxime “pep” Buquet <pep@bouah.net>
8d4d8f3
to
5721338
Compare
Thanks! I confirm that it works locally. I added a commit to change the background color if the search input is disabled. @bors: r+ rollup |
📌 Commit 5721338 has been approved by |
…laumeGomez rustdoc: fixes rust-lang#64305: disable search field instead of hidding it The result seems to be ok but I wasn't entirely sure how to get there. I tried to stay generic a bit but maybe it's not required at all. @GuillaumeGomez Signed-off-by: Maxime “pep” Buquet <pep@bouah.net>
Rollup of 7 pull requests Successful merges: - #66060 (Making ICEs and test them in incremental) - #66298 (rustdoc: fixes #64305: disable search field instead of hidding it) - #66457 (Just derive Hashstable in librustc) - #66496 (rustc_metadata: Privatize more things) - #66514 (Fix selected crate search filter) - #66535 (Avoid ICE when `break`ing to an unreachable label) - #66573 (Ignore run-make reproducible-build-2 on Mac) Failed merges: r? @ghost
The result seems to be ok but I wasn't entirely sure how to get there. I tried to stay generic a bit but maybe it's not required at all.
@GuillaumeGomez
Signed-off-by: Maxime “pep” Buquet pep@bouah.net