Skip to content

Fix search section position on small devices #80412

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

Merged
merged 1 commit into from
Jan 2, 2021

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Dec 27, 2020

Fixes #79526.

This is exactly the same issue fixed in 9c36491 (in #79936) but applied to the search section. When the width becomes too small, the search input goes on its own line to get more space, making it go "under" the section following (so either "main" or "search"). The fix is to simply make the section go more under so that it doesn't go over the search input.

r? @jyn514

@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) labels Dec 27, 2020
@rust-highfive
Copy link
Contributor

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 27, 2020
@@ -1570,7 +1570,8 @@ h4 > .notable-traits {
height: 73px;
}

#main {
/* This is to prevent the search bar to be under the corresponding section. */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way this comment is worded confuses me. What does 'under the corresponding section' mean?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wording isn't great indeed. What about "the section following it"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something like

Suggested change
/* This is to prevent the search bar to be under the corresponding section. */
/* This is to prevent the search bar from being underneath the following section. */

then? What's the ID of the following section? It might be good to add that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More like "the element following it". Might be better this way haha.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be better this way haha.

Which way is "this way"? ;)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated! Tell me if the new wording is good for you. :)

@jyn514
Copy link
Member

jyn514 commented Dec 30, 2020

@GuillaumeGomez can you add a screenshot before and after?

@jyn514 jyn514 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 30, 2020
@GuillaumeGomez
Copy link
Member Author

@jyn514 The display doesn't change at all.

@jyn514
Copy link
Member

jyn514 commented Dec 30, 2020

I'm confused. If the display is the same, why make the change? In #79936 the display changed to have half the screen dedicated to the item name and the other half to the description; is that different from what's happening here?

@GuillaumeGomez
Copy link
Member Author

I linked to the specific commit in my first comment, but extra info time! The section following the search input (which is included in a form) is in fact over the search input. Which is why I had to make the first fix and I'm adding this one. However, due to layout, the display of this section seems to be under the search input. You can see it easily like this: make a search to see the search results, then reduce the width of the screen (with the debugger tools, since you'll need to use them :p) so that the search input is on its own line. Then check the layout (by simply going over the element in the html tree view in the debugger tool), the one you're want to look for is <section id="search". That should appear more obviously this way because I realize that my explanations might not be super easy or obvious...

@jyn514
Copy link
Member

jyn514 commented Jan 2, 2021

I managed to reproduce this. I was confused because the problem is different from #79936, only the fix is the same. This is fixing #79526, not anything to do with the width of the names.

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 2, 2021

📌 Commit 2ab3651 has been approved by jyn514

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 2, 2021
@bors
Copy link
Collaborator

bors commented Jan 2, 2021

⌛ Testing commit 2ab3651 with merge fde6927...

@bors
Copy link
Collaborator

bors commented Jan 2, 2021

☀️ Test successful - checks-actions
Approved by: jyn514
Pushing fde6927 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 2, 2021
@bors bors merged commit fde6927 into rust-lang:master Jan 2, 2021
@rustbot rustbot added this to the 1.51.0 milestone Jan 2, 2021
@GuillaumeGomez GuillaumeGomez deleted the fix-search-section-pos branch January 2, 2021 21:10
@memoryruins
Copy link
Contributor

Thank y'all for looking into this! Mobile rustdoc has been much friendlier to use recently ❤️

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) merged-by-bors This PR was explicitly merged by bors. 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
Development

Successfully merging this pull request may close these issues.

Rustdoc search is broken on mobile
7 participants