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.

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

Fix missing bottom border for headings in sidebar #90571

Merged

Conversation

GuillaumeGomez
Copy link
Member

Fixes #90568.

r? @jsha

@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 Nov 4, 2021
@rust-highfive
Copy link
Collaborator

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 Nov 4, 2021
Copy link
Contributor

@jsha jsha left a comment

Choose a reason for hiding this comment

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

Thanks for catching and fixing this!

@@ -156,7 +156,9 @@ h1.fqn > .in-band > a:hover {
section hierarchies. */
h2,
.top-doc h3,
.top-doc h4 {
.top-doc h4,
.sidebar .sidebar-elems h3:not(.sidebar-title),
Copy link
Contributor

Choose a reason for hiding this comment

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

I find the not selectors are pretty hard to maintain and reason about. Can we do this with a positive selector instead? For instance, it looks like the headings we need underlines on here are nested under .block, so maybe we could do .sidebar .block h3?

Also it's not clear to me why both .sidebar and .sidebar-elems are needed in this selector.

Copy link
Member Author

Choose a reason for hiding this comment

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

Except that the .sidebar-title are in .block as well. :3

It was more complicated than I expected to actually add this rule without breaking something.

Copy link
Contributor

Choose a reason for hiding this comment

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

What about selecting the specific block types that get the underline treatment? Like:

.sidebar .block.struct h3, .sidebar .block.trait h3, .sidebar .block.type, ... {

Alternately, we could change the HTML structure slightly so everything after "Other items in ..." is nested under a specific class, like .other-items. Then we could make this selector .sidebar .other-items h3.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I'd prefer to have a CSS class in the generated HTML, seems much simpler.

@GuillaumeGomez
Copy link
Member Author

@jsha It now looks much better, thanks for the suggestion!

@jsha
Copy link
Contributor

jsha commented Nov 4, 2021

Looks great. r=me once CI passes

@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez
Copy link
Member Author

@bors: r=jsha rollup

@bors
Copy link
Contributor

bors commented Nov 4, 2021

📌 Commit aa17e1c has been approved by jsha

@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 Nov 4, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 4, 2021
…r-sidebar, r=jsha

Fix missing bottom border for headings in sidebar

Fixes rust-lang#90568.

r? `@jsha`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 4, 2021
…r-sidebar, r=jsha

Fix missing bottom border for headings in sidebar

Fixes rust-lang#90568.

r? ``@jsha``
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 5, 2021
Rollup of 9 pull requests

Successful merges:

 - rust-lang#90507 (Suggest `extern crate alloc` when using undeclared module `alloc`)
 - rust-lang#90530 (Simplify js tester a bit)
 - rust-lang#90533 (Add note about x86 instruction prefixes in asm! to unstable book)
 - rust-lang#90537 (Update aarch64 `target_feature` list for LLVM 12.)
 - rust-lang#90544 (Demote metadata load warning to "info".)
 - rust-lang#90554 (Clean up some `-Z unstable-options` in tests.)
 - rust-lang#90556 (Add more text and examples to `carrying_{add|mul}`)
 - rust-lang#90563 (rustbot allow labels)
 - rust-lang#90571 (Fix missing bottom border for headings in sidebar)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3821ab2 into rust-lang:master Nov 5, 2021
@rustbot rustbot added this to the 1.58.0 milestone Nov 5, 2021
@GuillaumeGomez GuillaumeGomez deleted the missing-bottom-border-sidebar branch November 5, 2021 09:27
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) 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.

Titles in the sidebar aren't underlined anymore
6 participants