Skip to content

rustdoc: on mobile, make the sidebar full width and linewrap #139831

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
Merged
merged 3 commits into from
May 25, 2025
Merged

rustdoc: on mobile, make the sidebar full width and linewrap #139831

merged 3 commits into from
May 25, 2025

Conversation

lolbinarycat
Copy link
Contributor

this is because the mobile sidebar cannot be resized, unlike on desktop.

@rustbot
Copy link
Collaborator

rustbot commented Apr 15, 2025

r? @notriddle

rustbot has assigned @notriddle.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. labels Apr 15, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@lolbinarycat
Copy link
Contributor Author

@GuillaumeGomez I'm really not sure how changing the size of these elements could cause these test failures, got a clue?

@GuillaumeGomez
Copy link
Member

Please add a screenshot. Also, what's the reasoning behind this change? What would it improve?

@lolbinarycat
Copy link
Contributor Author

Motivation is how much stuff gets needlessly truncated:

before screenshot

will add screenshot for what this looks like after the change once I have my new laptop set up more.

@lolbinarycat
Copy link
Contributor Author

here's what it looks like after the change:

screenshot

@lolbinarycat
Copy link
Contributor Author

This also increases consistency with mdBook, which has a much wider sidebar by default

mdBook screenshot

@GuillaumeGomez GuillaumeGomez removed the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Apr 25, 2025
@GuillaumeGomez
Copy link
Member

The difference is (was?) that mdBook and even the rustdoc file panel are trees whereas the rustdoc sidebar is (was?) a list.

Anyway, I don't mind the change, we'll just do an FCP first to ensure that everyone else agrees with it. Once done, I can check what's wrong with the gui test failure.

@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented Apr 25, 2025 •
edited by camelid
Loading

Team member @GuillaumeGomez has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Apr 25, 2025
@bors
Copy link
Collaborator

bors commented May 10, 2025

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

@lolbinarycat lolbinarycat force-pushed the rustdoc-mobile-sidebar branch from 94558c5 to 59bdc04 Compare May 16, 2025 09:51
@lolbinarycat
rustdoc: on mobile, make the sidebar full width and linewrap
5326fc9 
this is because the mobile sidebar cannot be resized,
unlike on desktop.
@lolbinarycat lolbinarycat force-pushed the rustdoc-mobile-sidebar branch from 59bdc04 to 5326fc9 Compare May 16, 2025 09:53
@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels May 16, 2025
@rfcbot
Copy link
Collaborator

rfcbot commented May 16, 2025

🔔 This is now entering its final comment period, as per the review above. 🔔

@camelid
Copy link
Member

camelid commented May 16, 2025

Is there a reason this is still marked as draft?

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@lolbinarycat lolbinarycat marked this pull request as ready for review May 22, 2025 20:11
@rustbot
Copy link
Collaborator

rustbot commented May 22, 2025

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha, @lolbinarycat

@rustbot
Copy link
Collaborator

rustbot commented May 22, 2025

Some changes occurred in GUI tests.

cc @GuillaumeGomez

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@lolbinarycat
rustdoc: rip out all the gui tests for <100% width mobile sidebar
1eba1b5 
stuff like "clicking the settings menu closes the mobile sidebar"
is now impossible for users to observe, since the mobile sidebar
will always cover the settings menu due to being full-width,
which is good because that behavior is also now impossible for
our testing framework to observe.
@lolbinarycat lolbinarycat force-pushed the rustdoc-mobile-sidebar branch from 7c80c71 to 1eba1b5 Compare May 22, 2025 21:45
GuillaumeGomez
GuillaumeGomez reviewed May 23, 2025
View reviewed changes
GuillaumeGomez
GuillaumeGomez reviewed May 23, 2025
View reviewed changes
@lolbinarycat
Copy link
Contributor Author

@GuillaumeGomez suggestions applied!

@GuillaumeGomez
Copy link
Member

Thanks!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented May 24, 2025

📌 Commit bcca611 has been approved by GuillaumeGomez

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 May 24, 2025
@jhpratt jhpratt mentioned this pull request May 25, 2025
Merged
bors added a commit that referenced this pull request May 25, 2025
@bors
Auto merge of #141529 - jhpratt:rollup-8dle839, r=jhpratt
aa57e46 
Rollup of 4 pull requests

Successful merges:

 - #139831 (rustdoc: on mobile, make the sidebar full width and linewrap)
 - #140950 (More option optimization tests)
 - #141108 (Docs(lib): Fix `extract_if` docs)
 - #141361 (use `cfg_select!` to select the right `VaListImpl` definition)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 045ac21 into rust-lang:master May 25, 2025
6 checks passed
@rustbot rustbot added this to the 1.89.0 milestone May 25, 2025
rust-timer added a commit that referenced this pull request May 25, 2025
@rust-timer
Unrolled build for #139831
1ad0f76 
Rollup merge of #139831 - lolbinarycat:rustdoc-mobile-sidebar, r=GuillaumeGomez

rustdoc: on mobile, make the sidebar full width and linewrap

this is because the mobile sidebar cannot be resized, unlike on desktop.
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels May 26, 2025
@rustbot rustbot mentioned this pull request May 26, 2025
Open
@notriddle notriddle removed the to-announce Announce this issue on triage meeting label May 27, 2025
@lolbinarycat lolbinarycat mentioned this pull request May 29, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

Assignees

@notriddle notriddle

Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.
Projects
None yet
Milestone
1.89.0
Development

Successfully merging this pull request may close these issues.
8 participants
@lolbinarycat @rustbot @rust-log-analyzer @GuillaumeGomez @rfcbot @bors @camelid @notriddle