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.

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 new crate filter location visually unpolished #93240

Closed
steffahn opened this issue Jan 23, 2022 · 5 comments
Closed

Rustdoc new crate filter location visually unpolished #93240

steffahn opened this issue Jan 23, 2022 · 5 comments
Labels
A-rustdoc-ui Area: rustdoc UI (generated HTML) C-bug Category: This is a bug. C-enhancement Category: An issue proposing an enhancement or a PR with one. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@steffahn
Copy link
Member

steffahn commented Jan 23, 2022

I took a few notes while looking at nightly rustc docs:

Screenshot_20220123_165956

@rustbot label T-rustdoc, C-bug, C-enhancement, A-rustdoc-ui

The sans serif vs. serif is clearly a bug. That’s looking terrible.

The location being in a place that moves horizontally is suboptimal; changing that would be an enhancement. The font size is ridiculously high (because it’s part of a “title”, I guess?). I like the thing no longer being inside of the search bar like it used to be, so I’m not suggesting any reverts, just more polishing work. Maybe this option could be placed e.g. still roughly next to the “Results for …” heading, but right-justified so it doesn’t move around when I change my search term, and also in a normal / more reasonable size. E.g. saying something like “search in [all crates]”.

By the way, something not indicated in the picture that I just noticed: if it says “… in [All crates]”, I don’t think the “All” should be capitalized anymore.

The question of whether or not it’s necessary for rustdoc to parrot my search term at all is one I don’t care all that much about. If there’s any benefit to keeping it, sure, keep it; I don’t really care.


(Minor note, looking back at my image, the arrows are a bit short. The text about font size is in-fact referring to the font size of the dropdown list, nothing else.)

@rustbot rustbot added A-rustdoc-ui Area: rustdoc UI (generated HTML) C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Jan 23, 2022
@the8472
Copy link
Member

the8472 commented Jan 23, 2022

Should this be a dropdown at all? It seems like something that could be a search operator, at least if we had some sort of auto-suggest. E.g. type in:rustc_ in the search bar and it'll offer available creates.

@camelid
Copy link
Member

camelid commented Jan 25, 2022

cc @jsha @GuillaumeGomez

@camelid
Copy link
Member

camelid commented Jan 25, 2022

Should this be a dropdown at all? It seems like something that could be a search operator, at least if we had some sort of auto-suggest. E.g. type in:rustc_ in the search bar and it'll offer available creates.

Yeah, I suggested this a couple weeks ago. I think the idea is to do it once we also have an "advanced search builder".

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Jan 25, 2022

Yes, we still need to discuss things. We will do that once everyone is available. ;)

steffahn added a commit to steffahn/rust that referenced this issue Jul 11, 2022
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
GuillaumeGomez pushed a commit to GuillaumeGomez/rust that referenced this issue Aug 10, 2022
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
@GuillaumeGomez
Copy link
Member

I think this issue was fixed by #100374. Don't hesitate to reopen it if I'm mistaken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-ui Area: rustdoc UI (generated HTML) C-bug Category: This is a bug. C-enhancement Category: An issue proposing an enhancement or a PR with one. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants