Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Improve crate selection on rustdoc search results page #100374

Merged
merged 4 commits into from
Aug 12, 2022
Merged

Improve crate selection on rustdoc search results page #100374

merged 4 commits into from
Aug 12, 2022

Conversation

GuillaumeGomez
Copy link
Member

Take over of #98855 (screenshots and explanations are there).

You can test it here.

cc @steffahn @jsha
r? @notriddle

@GuillaumeGomez GuillaumeGomez added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-rustdoc-ui Area: Rustdoc UI (generated HTML) A-rustdoc-js Area: Rustdoc's JS front-end labels Aug 10, 2022
@rustbot
Copy link
Collaborator

rustbot commented Aug 10, 2022

A change occurred in the Ayu theme.

cc @Cldfire

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 10, 2022
@GuillaumeGomez GuillaumeGomez changed the title Improve rustdoc search results page crates selection Improve crate selection on rustdoc search results page Aug 10, 2022
@GuillaumeGomez GuillaumeGomez mentioned this pull request Aug 10, 2022
Closed
steffahn and others added 3 commits August 10, 2022 15:55
@steffahn @GuillaumeGomez
Improve crate selection on rustdoc search results page
16bcc18 
Resolves all of issue rust-lang#93240

Reproduces a similar change as rust-lang#99086, but with improvements

In particular, this PR inlcludes:
* redesigning the crate-search selector so the background color matches its surroundings
* decrease the font of the dropdown menu to a reaonable size
* add a hover effect
* make the color of the arrow theme-dependent, using a surrounding div, with :after pseudo-element
  that can then be transformed using CSS filters to approximate the desired color
* fix the text "in" to match the title font
* remove the "for xyz" in the "Results for xyz in [All crates]" title when
  searching for search term "xyz"; you can already see what you're searching for
  as it's typed in the search bar!
* in line with rust-lang#99086, handle super-long crate names appropriately without a long <select>
  element escaping the screen area; the improvement is that we also keep the title
  within a single line now; uses some flex layout shenanigans...
* the margins / paddings are adjusted so the selected label of the <select> fits within
  the rest of that title nicely; also some inconsistency in the way that Firefox renders
  a <select> with "appearance: none" (roughly 4px more padding left and right of the text
  than e.g. Chrome) is worked around, and it now produces a result that looks (essentially)
  identical to Chrome
* the color of the help menu and settings menu border in light theme is made to match with
  the color of the corresponding buttons, like they do (match) in the ayu theme
* the casing of "All crates" changes to "all crates"
* the new tests from rust-lang#99086 are temporarily disabled, until they can be adapted later
@steffahn @GuillaumeGomez
Two small improvements:
e957480 
Fix oversight duplicate property left in CSS (dark theme).

Improve wording in comment that mentions `appearance: none`
@GuillaumeGomez
Add missing ID into the ID map
107e039
@GuillaumeGomez GuillaumeGomez force-pushed the improve_rustdoc_search_results_page_crates_selection branch from 1c40594 to da1a10d Compare August 10, 2022 13:55
@GuillaumeGomez GuillaumeGomez force-pushed the improve_rustdoc_search_results_page_crates_selection branch from da1a10d to ea05be2 Compare August 10, 2022 13:57
@notriddle
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Aug 10, 2022

📌 Commit ea05be2 has been approved by notriddle

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 10, 2022
@steffahn
Copy link
Member

Thanks for writing the tests!

Looking at the “ID map” commit, I feel like that file needing to be updated is quite subtle; I never could’ve known that myself. (Also, even looking at that thing and skimming over its usages, I still don’t even understand what that IdMap is used for…) I think there’s some potential for improving documentation here, e.g. adding comments in places where those HTML elements are defined or generated that the init_id_map needs to be kept up-to-date. Or maybe even an automated test could be possible? (I don’t know what that IdMap does, so no idea whether testing it is possible.) That’s just a general comment, not really related to this PR; maybe I should open an issue on that?

@GuillaumeGomez
Copy link
Member Author

Don't worry, it's normal. We're hoping to remove the need for it in a hopefully not so far future (the implementation for that is #92043).

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Aug 11, 2022
@Dylan-DPC
Rollup merge of rust-lang#100374 - GuillaumeGomez:improve_rustdoc_sea…
5a5cd6b 
…rch_results_page_crates_selection, r=notriddle

Improve crate selection on rustdoc search results page

Take over of rust-lang#98855 (screenshots and explanations are there).

You can test it [here](https://rustdoc.crud.net/imperio/improve_rustdoc_search_results_page_crates_selection/std/index.html?search=test).

cc `@steffahn` `@jsha`
r? `@notriddle`
@Dylan-DPC Dylan-DPC mentioned this pull request Aug 11, 2022
Merged
@matthiaskrgr matthiaskrgr mentioned this pull request Aug 11, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 11, 2022
@bors
Auto merge of rust-lang#100426 - matthiaskrgr:rollup-0ks4dou, r=matth…
2ed0f29 
…iaskrgr

Rollup of 13 pull requests

Successful merges:

 - rust-lang#93896 (rustdoc: make item-infos dimmer on dark theme)
 - rust-lang#99337 (rustdoc: simplify highlight.rs)
 - rust-lang#99421 (add crt-static for android)
 - rust-lang#99500 (Fix flags when using clang as linker for Fuchsia)
 - rust-lang#99511 (make raw_eq precondition more restrictive)
 - rust-lang#99992 (Add `x.sh` and `x.ps1` shell scripts)
 - rust-lang#100112 (Fix test: chunks_mut_are_send_and_sync)
 - rust-lang#100203 (provide correct size hint for unsupported platform `CommandArgs`)
 - rust-lang#100307 (Fix rust-lang#96847)
 - rust-lang#100350 (Stringify non-shorthand visibility correctly)
 - rust-lang#100374 (Improve crate selection on rustdoc search results page)
 - rust-lang#100392 (Simplify visitors)
 - rust-lang#100418 (Add stability attributes to BacktraceStatus variants)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit cefcfda into rust-lang:master Aug 12, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 12, 2022
@GuillaumeGomez GuillaumeGomez deleted the improve_rustdoc_search_results_page_crates_selection branch August 12, 2022 08:38
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 2, 2022
@bors
Auto merge of rust-lang#100395 - Dylan-DPC:rollup-ajrwo1s, r=Dylan-DPC
3013480 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#92744 (Check if enum from foreign crate has any non exhaustive variants when attempting a cast)
 - rust-lang#99337 (rustdoc: simplify highlight.rs)
 - rust-lang#100007 (Never inline Windows dtor access)
 - rust-lang#100030 (cleanup code w/ pointers in std a little)
 - rust-lang#100192 ( Remove duplicated temporaries creating during box derefs elaboration)
 - rust-lang#100247 (Generalize trait object generic param check to aliases.)
 - rust-lang#100374 (Improve crate selection on rustdoc search results page)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@GuillaumeGomez GuillaumeGomez mentioned this pull request Oct 27, 2022
Closed
@steffahn steffahn mentioned this pull request Sep 28, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@notriddle notriddle

Labels
A-rustdoc-js Area: Rustdoc's JS front-end A-rustdoc-ui Area: Rustdoc UI (generated HTML) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.65.0
Development

Successfully merging this pull request may close these issues.
6 participants
@GuillaumeGomez @rustbot @notriddle @bors @steffahn @rust-highfive