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

Remove border-bottom from headings in most docblocks. #90036

Merged
merged 2 commits into from
Oct 20, 2021

Conversation

jsha
Copy link
Contributor

@jsha jsha commented Oct 19, 2021

Headings in the top-doc docblock still get a border-bottom due to a rule
that covers all h2, h3, and h4. Method docblocks are generally h5, and
so don't get a border-bottom anymore.

This fixes a problem where a sub-sub-heading within a method would have
a line that went all the way across the page, creating a division that
made that sub-sub-heading look much more important than it really is.

Fixes #90033

Demo at https://jacob.hoffman-andrews.com/rust/less-rule/std/string/struct.String.html

r? @GuillaumeGomez

Headings in the top-doc docblock still get a border-bottom due to a rule
that covers all h2, h3, and h4. Method docblocks are generally h5, and
so don't get a border-bottom anymore.

This fixes a problem where a sub-sub-heading within a method would have
a line that went all the way across the page, creating a division that
made that sub-sub-heading look much more important than it really is.
@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 Oct 19, 2021
@jsha jsha added the A-rustdoc-ui Area: Rustdoc UI (generated HTML) label Oct 19, 2021
@camelid
Copy link
Member

camelid commented Oct 19, 2021

Hmm, the spacing looks a bit funny to me:

image

Now that there's no longer a horizontal rule, I think it might be better to put the heading closer to the next paragraph.

@camelid
Copy link
Member

camelid commented Oct 19, 2021

Also, I think increasing the h5 font size a tad, from 1em to 1.1em, visually separates the heading better from the main body text.

@jsha
Copy link
Contributor Author

jsha commented Oct 19, 2021

I pushed an update with the requested changes, and updated the demo. Let me know what you think!

@GuillaumeGomez
Copy link
Member

Thanks!

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Oct 19, 2021

📌 Commit e399343 has been approved by GuillaumeGomez

@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, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Oct 19, 2021
Remove border-bottom from most docblocks.

Headings in the top-doc docblock still get a border-bottom due to a rule
that covers all h2, h3, and h4. Method docblocks are generally h5, and
so don't get a border-bottom anymore.

This fixes a problem where a sub-sub-heading within a method would have
a line that went all the way across the page, creating a division that
made that sub-sub-heading look much more important than it really is.

Fixes rust-lang#90033

Demo at https://jacob.hoffman-andrews.com/rust/less-rule/std/string/struct.String.html

r? `@GuillaumeGomez`
@camelid
Copy link
Member

camelid commented Oct 19, 2021

I pushed an update with the requested changes, and updated the demo. Let me know what you think!

It looks like you didn't increase the font size though?

@jsha
Copy link
Contributor Author

jsha commented Oct 19, 2021

I did: https://github.com/rust-lang/rust/pull/90036/files#diff-67c3587d759aa9578993149e68eb8d3a9de6b28e5c0b85091b0a71a07f441795R514

Previously h4, h5, and h6 were font-size: 1em. Now h4 and h5 are 1.1em, while h6 is still 1em.

bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 19, 2021
Rollup of 10 pull requests

Successful merges:

 - rust-lang#86479 (Automatic exponential formatting in Debug)
 - rust-lang#87404 (Add support for artifact size profiling)
 - rust-lang#87769 (Alloc features cleanup)
 - rust-lang#88789 (remove unnecessary bound on Zip specialization impl)
 - rust-lang#88860 (Deduplicate panic_fmt)
 - rust-lang#90009 (Make more `From` impls `const` (libcore))
 - rust-lang#90018 (Fix rustdoc UI for very long type names)
 - rust-lang#90025 (Revert rust-lang#86011 to fix an incorrect bound check)
 - rust-lang#90036 (Remove border-bottom from most docblocks.)
 - rust-lang#90060 (Update RELEASES.md)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@camelid
Copy link
Member

camelid commented Oct 20, 2021

I did: https://github.com/rust-lang/rust/pull/90036/files#diff-67c3587d759aa9578993149e68eb8d3a9de6b28e5c0b85091b0a71a07f441795R514

Previously h4, h5, and h6 were font-size: 1em. Now h4 and h5 are 1.1em, while h6 is still 1em.

Ah, maybe you didn't update your demo with this part then? Firefox devtools show that h5 is still 1em. I do see the margin change in the demo, just not the size change.

@bors bors merged commit 570b999 into rust-lang:master Oct 20, 2021
@rustbot rustbot added this to the 1.58.0 milestone Oct 20, 2021
@jsha jsha changed the title Remove border-bottom from most docblocks. Remove border-bottom from headings in most docblocks. Oct 20, 2021
@jsha jsha deleted the less-rule branch October 22, 2021 21:10
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remove horizontal rule from Markdown-generated HTML
6 participants