Improve crate selection on rustdoc search results page

[steffahn]

Improves some problems pointed out in #93240.

In particular the in being serif-font makes sense once this is in a new line, and the font-size is better so the dropdown menu isn’t this huge anymore. This also addresses my concern that the selection moves around horizontally as the search term changes.

The formatting for the #search-settings CSS is not necessary for the title, because h1 has the same CSS, too, anyways.

Currently on nightly:

After this PR: (outdated, see below)

---

With my latest changes added, I'd say this PR can close #93240 entirely. See #98855 (comment) below to see renderings of the current status.

[rustbot]

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @Folyd, @jsha

[rust-highfive]

r? @jsha

(rust-highfive has picked a reviewer for you, use r? to override)

[GuillaumeGomez]

I uploaded an online version of this change here. I'm personally not a big fan of putting the "in X" part in its own line but let's see what others think about it.

cc @jsha @Manishearth

[steffahn]

The thing you uploaded only has a single crate, so the list doesn’t show up in the first place. It did however remind me that in that case, the <br/> probably shouldn’t appear in the HTML (even though it doesn’t appear to get rendered as a new line anyway), so after re-inspecting the logic in the JS, I’ve moved the line break into the crates variable to make it conditional like that.

[GuillaumeGomez]

Didn't even pay attention to that, sorry. Fixed it.

[jsha]

I agree with @GuillaumeGomez that we shouldn't use a second line of vertical space. @steffahn you commented in #93240 that you are indifferent to whether the results page echoes the search query. I think that might be a better solution than using a second line; let's remove the "for search_term" part. Then the dropdown can be on the same line but not move around as you edit your search.

[steffahn]

Okay, keeping it in the title and removing the search term; making the in part of the title font (but not the selection, that looks weird in bold). I would still like to keep the selection dropdown smaller, if that’s stylistically acceptable. So how about something like this for a look? (I just manually edited this in the browser so far, not implemented it yet into the source files.)

With a single crate, the thing would just say Results.

[steffahn]

On second thought, in e.g. the white theme where the dropdown is lower-contrast, keeping matching fonts seems kind-of essential actually. Compare:

the second looks better IMO.

[steffahn]

theme where the dropdown is lower-contrast

Makes me reconsider perhaps also addressing the other point I made in #93240 here, the high contrast in the dark theme currently. Is there any other dropdown-selections in rustdoc that should be taken into consideration for consistency?

---

I’m also noticing that “all crates” is probably more sensible than “All crates” in this position.

[steffahn]

I added an implementation of a style along those lines, i.e. as explained in my previous two comments.

Small font size in dropdown:

As you might be able to tell by looking into the CSS, I had a hard time adapting the color of that arrow to the field; I’m open for suggestions for cleaner approaches if an inline svg like this is not acceptable.

[rustbot]

A change occurred in the Ayu theme.

cc @Cldfire

Some changes occurred in HTML/CSS themes.

cc @GuillaumeGomez

[GuillaumeGomez]

Just realized that if you don't need to use background-image in all themes, we already have it in rustdoc.css.

[notriddle]

Potential extreme alternative: use the sidebar?

(just a mockup)

[GuillaumeGomez]

That seems very weird to switch the "focus" from the main content to the sidebar, no?

[steffahn]

I have yet to find out why the thing has so much more visual padding in the <select> on Firefox than Chrome, and how to avoid this. (I prefer the way it looks on Chrome / that’s what I looked at when changing/choosing the padding in the first place.)

Chrome

Firefox

If anyone has an idea, feel free to share it.

[steffahn]

Some screenshots of the hover effects I’ve added yesterday:

For light and ayu, colors of outline (both with and without focus, respectively) are the same as the settings and help button; the border color without focus is also the same as the outline of the settings menu on all themes, so that’s how I chose the outline color for the dark theme; the blue hover color of the dark theme is the same as the hover color for the radio buttons and the activate switches in the settings menu, admitted there are a lot of slightly different shades of blue, i.e. the searchbar highlight above (if it’s focused) is a different blue, and the active tab below (“In Names”/“In Parameters”/“In Return Types”) has yet-another shade of blue.

[steffahn]

I’ve disabled the abovementioned failing (still rather new) tests for now¹; if it’s okay to have them only come back in a future PR, then this PR should be ready to be reviewed and merged from my end, provided CI passes now 🚀

Footnotes

1. see the collapsed rust-log-analyzer comment above this one to see the error message it was producing ↩

[GuillaumeGomez]

When does it happen? We don't use appearance: none ourselves after all.

[steffahn]

I think we effectively are, kind-of… I guess… at least with appearance: none it looks the same, IDK about backwards-compatibility with older versions of Firefox where it might’ve been different..?

In any case, what this is supposed to say is that Firefox has some (empirically, 4px) padding in an "unstyled" (i.e. when the native arrow is removed) that Chrome doesn't have. I don't have any other browsers myself ATM to test / compare. Admitted, given that appearance: none is not literally what’s used here, the comment could be clarified.

[steffahn]

I changed the wording a bit now.

I’m not sure about the pros and cons of using the “actual” appearance: none property, I haven’t researched too much what distinguishes this property from those -moz-appearance / -webkit-appearance ones; consequently I kept everything as-is with regards to these properties, just to be safe, e.g. since I haven’t tested other browsers, etc…

[GuillaumeGomez]

So about the goml format. It's something I wrote to make it easy to write/read. You can see more information about it (with the full list of commands) here.

[GuillaumeGomez]

We can take a look at the goml tests at the end if you're still stuck. Not really the hard part normally. ;)

[steffahn]

We can take a look at the goml tests at the end if you're still stuck. Not really the hard part normally. ;)

Well for one thing, I’d need to get those tests running locally in the first place, otherwise it’s impossible to work with.

It just gives me a completely cryptic error message

Running 66 rustdoc-gui (8 concurrently) ...
ERROR: puppeteer failed when trying to create a new page. Please try again with `--no-sandbox`

I’m also noticing this warning on the npm install browser-ui-test:

npm WARN deprecated puppeteer@3.3.0: Version no longer supported. Upgrade to @latest

I have no idea if that’s related to the problem and if yes how to fix it.

[GuillaumeGomez]

Ah I see, like that:

x.py test src/test/rustdoc-gui --test-args --no-sandbox

Normally you have the explanations in src/test/rustdoc-gui/README.md. :)

[steffahn]

W.r.t. rewriting/adapting the tests, they currently (judging by the comments) seem to work with the implicit assumption that the <select> lives in its own line and just test for its size. What’s actually interesting is presumably that its right-hand-side border is positioned within-bounds of, say, the search section, I have no idea how one would write this.

I also don’t know what this compare-elements-position-near-stuff means or what the error message talking about "#search-settings .search-results-title" means, in particular what the "#search-settings .search-results-title" syntax itself means. Looks like two CSS selectors separated by space; IDK how those compose.

---

In terms of tests, one could perhaps also want to test that #crate-search and #crate-search-div are always (i.e. both in normal cases and when the contents are too long) exactly on top of each other and the same size (or plus one pixel in all directions for the border, I’d have to check that…), so that the dropdown arrow appears in the right position.

[steffahn]

Ah I see, like that:

x.py test src/test/rustdoc-gui --test-args --no-sandbox

Normally you have the explanations in src/test/rustdoc-gui/README.md. :)

Ah, that works… but it gives an unrelated failure of a test that doesn’t appear to fail in CI

/home/frank/forks/rust/src/test/rustdoc-gui/type-declation-overflow.goml type-declation-overflow... FAILED
[ERROR] (line 35) Error: Evaluation failed: property `scrollWidth` (`491`) isn't equal to `492` for selector `.mobile-topbar .location`: for command `assert-property: (".mobile-topbar .location", {"scrollWidth": "492"})`
[ERROR] (line 36) Error: Evaluation failed: property `clientWidth` (`491`) isn't equal to `492` for selector `.mobile-topbar .location`: for command `assert-property: (".mobile-topbar .location", {"clientWidth": "492"})`

oh, well, I guess I can just ignore that problem.

[steffahn]

TBH, I don’t have much more time I can spend on this today. I would highly appreciate if I didn’t have to learn this whole testing infrastructure now, too. This PR is already so much more that I initially wanted to do. Learning new HTML and CSS fundamentals I didn’t know before is enough commitment for my liking, I don’t feel like going too deep into this whole testing framework, too, now.

[GuillaumeGomez]

I can do this part then.

[bors]

☔ The latest upstream changes (presumably #99346) made this pull request unmergeable. Please resolve the merge conflicts.

[GuillaumeGomez]

I took the commits from here, fixed the conflicts and added the missing GUI tests in #100374.

[steffahn]

Looks like at some point Firefox must have changed, at least my Firefox browser no longer shows the differences that used to be compensated with the

/* cancel stylistic differences in padding in firefox
for "appearance: none"-style (or equivalent) <select>s */
@-moz-document url-prefix() {
	#crate-search {
		padding-left: 0px; /* == 4px - 4px */
		padding-right: 19px; /* == 23px - 4px */
	}
}

part, hence that can (and should) probably be removed. I’ll need to look into this more, and create an appropriate new issue and PR if this is really the case. (Makes me wonder whether there are good ways to do something like “display a website for a bunch of different versions of Firefox” in order to confirm that there really was a change at some version throughout the last year, and I’m not the one that changed a factor.

[GuillaumeGomez]

I was wondering the same thing a few days ago. Don't hesitate to send a PR in any case!

As for checking on different firefox versions, I remember that websites allow it, but don't remember the name. ^^'

[steffahn]

I didn’t find any websites easily. I’ll just ignore the urge to conclusively “prove” something in Firefox has changed, and just go with the observation that both on my PC and phone, the current Firefox seems to be displaying it differently than intended and removing the special case workaround CSS code seems like the way to fix it. So I’ll just open a PR tomorrow. :-)

[GuillaumeGomez]

Perfect, thanks!