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.

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

rustdoc: remove class name location from sidebar sibling nav #103211

Merged
merged 1 commit into from
Oct 20, 2022
Merged

rustdoc: remove class name location from sidebar sibling nav #103211

merged 1 commit into from
Oct 20, 2022

Conversation

notriddle
Copy link
Contributor

@notriddle notriddle commented Oct 18, 2022 •
edited
Loading

Preview: https://notriddle.com/notriddle-rustdoc-demos/sidebar-location/std/vec/struct.Vec.html

This change tweaks the CSS to apply most of its styles to .sidebar h2, cleaning up a few redundant rules from .mobile-topbar .location and restoring useful navigation aids in mobile mode.

Before

location-before

After

location-after

@notriddle
rustdoc: remove class name location from sidebar sibling nav
a01b885 
This change tweaks the CSS to apply most of its styles to `.sidebar h2`,
cleaning up a few redundant rules from `.mobile-topbar .location` and
restoring useful navigation aids in mobile mode.
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @GuillaumeGomez (or someone else) soon.

Please see the contribution instructions for more information.

@rustbot rustbot added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Oct 18, 2022
@rustbot
Copy link
Collaborator

rustbot commented Oct 18, 2022

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @Folyd, @jsha

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 18, 2022
@GuillaumeGomez
Copy link
Member

Just a small question about the screenshots: how come there is no In std::vec title in the "Before" screenshot? And also, what are the screenshots supposed to put in evidence? Apart from this one difference I saw none.

@notriddle
Copy link
Contributor Author

notriddle commented Oct 18, 2022 •
edited
Loading

@GuillaumeGomez

Just a small question about the screenshots: how come there is no In std::vec title in the "Before" screenshot?

In current nightly, there are two <h2>s in the sidebar on std::vec::Vec. One of them is <h2 class="location">Vec</h2>, while the other is <h2 class="location">In std::vec</h2>.

In mobile mode, .sidebar .location { display: none }, so both headers are hidden. This seems like a mistake to me, because while <h2 class="location">Vec</h2> in the sidebar is redundant because it has a twin in the top bar, <h2 class="location">In std::vec</h2> feels useful. Everything below the second h2 is a sibling item of std::vec::Vec, while everything above it is a child item.

This commit changes it so that <h2 class="location">Vec</h2> remains as it is, but the second header is changed to just be <h2>In std::vec</h2>. This requires pretty extensive refactoring in the stylesheet, and some test tweaks so that everything can still find what it's looking for. I also removed class="location" from the mobile topbar header, while refactoring the stylesheet to keep the appearance exactly the same.

And also, what are the screenshots supposed to put in evidence? Apart from this one difference I saw none.

That's the only difference. There should be no other user-visible changes other than the In [module] header being visible in the sidebar on mobile.

@GuillaumeGomez
Copy link
Member

Thanks for the extra explanations! Then it looks all good to me.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Oct 19, 2022

📌 Commit a01b885 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 Oct 19, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Oct 19, 2022
@Dylan-DPC
Rollup merge of rust-lang#103211 - notriddle:notriddle/dot-location, …
5e4c91a 
…r=GuillaumeGomez

rustdoc: remove class name `location` from sidebar sibling nav

Preview: https://notriddle.com/notriddle-rustdoc-demos/sidebar-location/std/vec/struct.Vec.html

This change tweaks the CSS to apply most of its styles to `.sidebar h2`, cleaning up a few redundant rules from `.mobile-topbar .location` and restoring useful navigation aids in mobile mode.

## Before

![location-before](https://user-images.githubusercontent.com/1593513/196521014-d8730830-c3a2-4ed7-9266-05454cd31e05.png)

## After

![location-after](https://user-images.githubusercontent.com/1593513/196521020-75ec1fa5-b3dc-4c5d-97b6-afccb5fbe00a.png)
@Dylan-DPC Dylan-DPC mentioned this pull request Oct 19, 2022
Closed
@matthiaskrgr matthiaskrgr mentioned this pull request Oct 19, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 19, 2022
@bors
Auto merge of rust-lang#103264 - matthiaskrgr:rollup-3ja4spo, r=matth…
57781b2 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#103211 (rustdoc: remove class name `location` from sidebar sibling nav)
 - rust-lang#103223 (Use already checked RHS ty for LHS deref suggestions)
 - rust-lang#103237 (Clean up codeblock-tooltip rustdoc-gui test)
 - rust-lang#103239 (Allow #[unstable] impls for fn() with unstable abi.)
 - rust-lang#103246 (Mark `rust-analyzer` as a host-only tool)
 - rust-lang#103257 (rustdoc: move `setting-line` color CSS to settings.css)
 - rust-lang#103258 (Make miri read_dir test a little more robust)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 433f736 into rust-lang:master Oct 20, 2022
@rustbot rustbot added this to the 1.66.0 milestone Oct 20, 2022
@notriddle notriddle deleted the notriddle/dot-location branch October 20, 2022 02:28
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
@bors
Auto merge of rust-lang#103264 - matthiaskrgr:rollup-3ja4spo, r=matth…
299c81a 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#103211 (rustdoc: remove class name `location` from sidebar sibling nav)
 - rust-lang#103223 (Use already checked RHS ty for LHS deref suggestions)
 - rust-lang#103237 (Clean up codeblock-tooltip rustdoc-gui test)
 - rust-lang#103239 (Allow #[unstable] impls for fn() with unstable abi.)
 - rust-lang#103246 (Mark `rust-analyzer` as a host-only tool)
 - rust-lang#103257 (rustdoc: move `setting-line` color CSS to settings.css)
 - rust-lang#103258 (Make miri read_dir test a little more robust)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
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

@GuillaumeGomez GuillaumeGomez

Labels
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
Milestone
1.66.0
Development

Successfully merging this pull request may close these issues.
5 participants
@notriddle @rust-highfive @rustbot @GuillaumeGomez @bors