Skip to content

Fix title heading overlap in rust doc - #45450

Merged
bors merged 2 commits into
rust-lang:masterfrom
GuillaumeGomez:overlap-link
Nov 1, 2017
Merged

Fix title heading overlap in rust doc#45450
bors merged 2 commits into
rust-lang:masterfrom
GuillaumeGomez:overlap-link

Conversation

@GuillaumeGomez

Copy link
Copy Markdown
Member

Fixes#45158.

To be noted that this margin only appears when a title is the first element.

screen shot 2017-10-22 at 16 08 44

r? @rust-lang/docs

@GuillaumeGomez

Copy link
Copy Markdown
MemberAuthor

The second commit removes the top margin when a title is the first element of the text:

screen shot 2017-10-22 at 16 11 17

@kennytmkennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 22, 2017
@killercup

Copy link
Copy Markdown
Contributor

Hm, this is not how I would would fix. Why give this header a margin-left, when all other headers with collapse buttons next to them don't have one? It makes the layout look quite unbalanced to me.

In addition, it still has weird spacing on hover as the collapse button and the section link marker overlap:

bildschirmfoto 2017-10-22 um 19 17 00

I'd suggest getting rid of the "§" pseudo-element for the first header (.docblock > .section-header:first-child a:before { display: none; }) and adjusting the line height and spacing of the toggle button so it is one the same visual line as the headline (h1.fqn + .toggle-wrapper a { line-height: 33px; padding-top: 3px; }).

@GuillaumeGomez

Copy link
Copy Markdown
MemberAuthor

I thought at first about removing all "§" but it seemed inconsistent with the rest. Therefore, I came up with this solution which still seems the best for me. I don't feel like the rendering is unbalanced, and this is something we quite often around. However, if a majority of people thinks the same as you, I'll just update this way but I really don't like this idea.

@steveklabnik

Copy link
Copy Markdown
Contributor

I told @GuillaumeGomez I'd check this out, but I am terrible at CSS, so I don't have any strong opinions here.

@GuillaumeGomez

Copy link
Copy Markdown
MemberAuthor

Well, talk about the design then. Does it seem good for you from this perspective?

@QuietMisdreavus

Copy link
Copy Markdown
Contributor

Plain:

image

Hover:

image

This is Chrome on Windows. Unless there's something else going on in other browsers on other platforms, this looks Good Enough for me. Farther tiny CSS PRs are easy to review.

@bors r+ rollup

@bors

bors commented Oct 31, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit 237ad82 has been approved by QuietMisdreavus

@kennytmkennytm 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 1, 2017
kennytm added a commit to kennytm/rust that referenced this pull request Nov 1, 2017
…tMisdreavus
Fix title heading overlap in rust doc
Fixesrust-lang#45158.
To be noted that this margin only appears when a title is the first element.
<img width="1440" alt="screen shot 2017-10-22 at 16 08 44" src="https://user-images.githubusercontent.com/3050060/31862746-6411070e-b743-11e7-9a75-4159e1f7f1d6.png">
r? @rust-lang/docs
@kennytmkennytm mentioned this pull request Nov 1, 2017
bors added a commit that referenced this pull request Nov 1, 2017
Rollup of 14 pull requests
- Successful merges: #45450, #45579, #45602, #45619, #45624, #45644, #45646, #45648, #45649, #45650, #45652, #45660, #45664, #45671
- Failed merges:
@bors
bors merged commit 237ad82 into rust-lang:masterNov 1, 2017
@GuillaumeGomez
GuillaumeGomez deleted the overlap-link branch November 2, 2017 08:52
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: 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.

6 participants

@GuillaumeGomez@killercup@steveklabnik@QuietMisdreavus@bors@kennytm