When arriving on a search result page, we want to focus on the search…

[GuillaumeGomez]

When you arrive on a search result page (for example, any page with a URL parameter "search=..."), you get the search results but since the focus isn't on the search input, you can't navigate using the keyboard, which is a bit annoying. This PR fixes it.

r? @jyn514

[rust-highfive]

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

[pickfire]

Looks good to me.

[GuillaumeGomez]

@bors: r=pickfire rollup

[bors]

📌 Commit ec952a9 has been approved by pickfire

[jyn514]

Wait, won't this break hitting spacebar to scroll down?

[jyn514]

Yeah, with search focused you can no longer scroll with the keyboard.

@bors r-

[GuillaumeGomez]

You never could, so in that regard, it doesn't change anything. The big difference here is that when you normally land on the search results page, you have the focus on the search input, allowing you to go through the items, which is not the case when you land directly on it (and which is why I made this PR).

[jyn514]

I'm not really understanding what this does, then. Can you give a screenshot or an example of a page this changes?

[GuillaumeGomez]

Currently, it doesn't give focus to the search input when you load a page with a search result, which is annoying because you don't have the up/down arrows or the enter either. This PR fixes it and unify the behaviour between loading the search results page directly or when you land on it from a search you made.

[jyn514]

Ok, so an example would be https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir/enum.TyKind.html?search=hir ?

That does have the issue I mentioned: currently you can scroll with page up/down on that page, but after this change you wouldn't be able to without hitting 'escape' first. It does seem a little inconsistent, but I think it's better this way - when you type something into search you want to keep typing, but when you land on a page for the first time you probably want to look at the page.

[GuillaumeGomez]

But then it becomes inconsistent with the rest. But I see your point. Well, let's ask others' opinion then!

cc @rust-lang/rustdoc

[pickfire]

IIRC in the mobile you cannot even open the keyboard if the search bar is not focused in the first place, and even if you focused on the search bar it would not open later, I thought the patch is a fix for it.

[jyn514]

in the mobile you cannot even open the keyboard if the search bar is not focused in the first place, and even if you focused on the search bar it would not open later

That seems like it's a separate bug to fix. Does that mean you can't search at all on mobile?

[pickfire]

That seems like it's a separate bug to fix. Does that mean you can't search at all on mobile?

No, you can search exactly once but not more than that. Once you searched, you need to go back and come back or reload the page IIRC. This patch could have fixed it partially since it auto focused so the keyboard will be raised and users can search.

[GuillaumeGomez]

@pickfire Really?! I never saw this bug when testing on mobile but if it's still around, it definitely needs to be fixed!

[pickfire]

Yes, it's still there. Steps to reproduce (I use firefox on mobile):

1. go to search page
2. type something to search
3. blue the search (keyboard lowers)
4. you cannot click on search bar anymore

Oh, I just found out that it work except the click position is way out, you need to touch the corner of the screen rather than the center of the box itself.

Okay, this pull request does not fix the issue I mentioned.

[jyn514]

Ok, so an example would be https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir/enum.TyKind.html?search=hir ?

That does have the issue I mentioned: currently you can scroll with page up/down on that page, but after this change you wouldn't be able to without hitting 'escape' first. It does seem a little inconsistent, but I think it's better this way - when you type something into search you want to keep typing, but when you land on a page for the first time you probably want to look at the page.

Because of that issue, I don't think this makes sense to merge this PR. I'm happy to discuss if you have other suggestions though!

[GuillaumeGomez]

It's fine, we can keep it as is for now.