-
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
Move crate drop-down to search results page #92490
Conversation
Some changes occurred in HTML/CSS/JS. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@@ -1126,9 +1126,16 @@ window.initSearch = function(rawSearchIndex) { | |||
} | |||
} | |||
|
|||
var output = "<h1>Results for " + escape(query.query) + | |||
let crates = `<select id="crate-search"><option value="All crates">All crates</option>`; |
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.
Isn't eslint complaining when you use backticks without ${}
content?
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.
Nope! I wouldn't expect it to. Backticks do a bunch of things besides template expansion. They also allow multiline strings, and serve as a different delimiter so you don't have to backslash double quotes (\"
)
As said previously, I really like it! The display needs a bit of improvement: the The style of the dropdown is incomplete I think: the border is looking strange. In any case, it's a nice start. :) |
This was on purpose, since
Good idea. |
I also had an idea recently that could help with this (I forget if I mentioned it or not). We could add a |
It looks very strange to me. The The bezel seems a bit extreme. |
Interesting! What platform is that?
…On Sun, Jan 2, 2022, 15:34 Noah Lev ***@***.***> wrote:
This was on purpose, since in ____ isn't part of the title (<h1>). But I
could make it the same size as the <h1>, just outside the tag? I don't
have a strong opinion about what's better.
It looks very strange to me. The select also renders differently from how
it used to:
[image: image]
<https://user-images.githubusercontent.com/37223377/147888752-2fb95576-3e30-46b7-a88c-7b21b31b1043.png>
The bezel seems a bit extreme.
—
Reply to this email directly, view it on GitHub
<#92490 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABVYLO4MM3EECR7FYWDTDLUUCZDXANCNFSM5LDI6S3A>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Firefox on macOS. |
6958519
to
3f3b976
Compare
Alright, I've increased the size of |
This comment has been minimized.
This comment has been minimized.
3f3b976
to
8abe0ab
Compare
This comment has been minimized.
This comment has been minimized.
8abe0ab
to
5c3e82a
Compare
This comment has been minimized.
This comment has been minimized.
(query.type ? " (type: " + escape(query.type) + ")" : "") + "</h1>" + | ||
"<div id=\"titles\">" + | ||
` in ${crates} ` + | ||
"</div><div id=\"titles\">" + |
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 think backticks would be better here as well.
@@ -40,5 +40,5 @@ ENV SCRIPT python3 ../x.py --stage 2 test src/tools/expand-yaml-anchors && \ | |||
/scripts/validate-toolstate.sh && \ | |||
/scripts/validate-error-codes.sh && \ | |||
# Runs checks to ensure that there are no ES5 issues in our JS code. | |||
es-check es5 ../src/librustdoc/html/static/js/*.js && \ | |||
es-check es6 ../src/librustdoc/html/static/js/*.js && \ |
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.
We wanted to make this change in bigger PR but I guess it's fine... Don't forget to update the comment just above please.
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 actually think a giant PR adding ES6 features wherever possible, and updating the linter at the same time would be a bad idea. The way we should do it is like I'm doing here:
- update the linter to accept es6 as a standalone commit
- start using es6 features where they are useful
- do a series of standalone commits that do one of two things:
- adopt an ES6 feature where it is useful (e.g. const everything that can be consted), or
- migrate a discrete chunk of code to using various useful ES6 features.
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.
Like I said, it's fine. 😉
Also update Node to v16.9.0, es-check to 6.1.1, and eslint to 8.6.0.
This reduces clutter on doc pages.
5c3e82a
to
8abb4bb
Compare
Thanks! @bors: r+ |
📌 Commit 8abb4bb has been approved by |
…Gomez Move crate drop-down to search results page This reduces clutter on doc pages. Part of rust-lang#59840 r? `@GuillaumeGomez` Demo: https://rustdoc.crud.net/jsha/crates-in-results/std/index.html?search=str
…Gomez Move crate drop-down to search results page This reduces clutter on doc pages. Part of rust-lang#59840 r? ``@GuillaumeGomez`` Demo: https://rustdoc.crud.net/jsha/crates-in-results/std/index.html?search=str
…askrgr Rollup of 8 pull requests Successful merges: - rust-lang#92055 (Add release notes for 1.58) - rust-lang#92490 (Move crate drop-down to search results page) - rust-lang#92510 (Don't resolve blocks in foreign functions) - rust-lang#92573 (expand: Refactor InvocationCollector visitor for better code reuse) - rust-lang#92608 (rustdoc: Introduce a resolver cache for sharing data between early doc link resolution and later passes) - rust-lang#92657 (Implemented const casts of raw pointers) - rust-lang#92671 (Make `Atomic*::from_mut` return `&mut Atomic*`) - rust-lang#92673 (Remove useless collapse toggle on "all items" page) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This reduces clutter on doc pages.
Part of #59840
r? @GuillaumeGomez
Demo: https://rustdoc.crud.net/jsha/crates-in-results/std/index.html?search=str