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

rustdoc: remove outdated CSS .content table etc #101773

Merged
merged 1 commit into from
Sep 14, 2022

Conversation

notriddle
Copy link
Contributor

Screenshot before

image

Screenshot after

image

Description

The .content table / .content td / .content tr family of selectors date back to 4fd061c, when module indexes and other parts of rustdoc used <table> tags for layout and content presentation. The .content td h1, .content td h2 has only been changed since then to tweak the font size in dd5ff42.

.content table {
border-spacing: 0 5px;
border-collapse: separate;
}
.content td { vertical-align: top; }
.content td:first-child { padding-right: 20px; }
.content td p:first-child { margin-top: 0; }
.content td h1, .content td h2 { margin-left: 0; font-size: 1.1em; }

This CSS would have affected:

  • search result tables, which were removed in b615c0c
  • module item tables, which were removed in 6020c79
  • docblock tables from markdown, which still exist

It may also have affected a few other tables over the last decade, but they've been gradually replaced with grid layouts and flexbox to make layouts that work better on narrow viewports. For example, 34bd2b8.

These rules have no affect on the appearance of docblock tables

.content table {
    border-spacing: 0 5px;
}

According to MDN, border-spacing only has an effect when border-collapse is separate. However, border-collapse: collapse is set globally for all tables, so this rule does nothing.

.content td p:first-child { margin-top: 0; }

Tables with paragraphs in them are impossible without dropping down to raw HTML. Also, the rustdoc stylesheet sets paragraphs to have no top margin anyway, so this rule is a no-op.

.content td h1, .content td h2 { margin-left: 0; font-size: 1.125rem; }

Tables with headers in them are impossible without dropping down to raw HTML. This is considered unlikely, especially since it looks weird right now (.docblock h2 has an underline that is redundant with the table cell's own border).

.content tr:first-child td { border-top: 0; }

This has no effect because of border collapsing.

This rule is removed, because tables look fine without it

.content td:first-child { padding-right: 20px; }

By removing this rule, the first cell in each row has the same padding as all other cells in the row.

This rule is kept, and converted to directly target .docblock

.content td { vertical-align: top; }

Removing this rule would cause it to be aligned to the middle instead.

The `.content table` / `.content td` / `.content tr` family of selectors date
back to 4fd061c, when module indexes and
other parts of rustdoc used `<table>` tags for layout and content
presentation. The `.content td h1, .content td h2` has only been changed
since then to tweak the font size in
dd5ff42.

https://github.com/rust-lang/rust/blob/4fd061c426902b0904c65e64a3780b21f9ab3afb/src/rustdoc_ng/html/static/main.css#L155-L162

This CSS would have affected:

  * search result tables, which were removed in
    b615c0c
  * module item tables, which were removed in
    6020c79
  * docblock tables from markdown, which still exist

It may also have affected a few other tables over the last decade, but
they've been gradually replaced with grid layouts and flexbox to make layouts
that work better on narrow viewports. For example,
34bd2b8.

These rules have no affect on the appearance of docblock tables
===============================================================

    .content table {
        border-spacing: 0 5px;
    }

According to MDN, [border-spacing] only has an effect when `border-collapse`
is `separate`. However, `border-collapse: collapse` is set globally for all
tables, so this rule does nothing.

[border-spacing]: https://developer.mozilla.org/en-US/docs/Web/CSS/border-spacing

    .content td p:first-child { margin-top: 0; }

Tables with paragraphs in them are impossible without dropping down to raw
HTML. Also, the rustdoc stylesheet sets paragraphs to have no top margin
anyway, so this rule is a no-op.

    .content td h1, .content td h2 { margin-left: 0; font-size: 1.125rem; }

Tables with headers in them are impossible without dropping down to raw HTML.
This is considered unlikely, especially since it looks weird right now
(`.docblock h2` has an underline that is redundant with the table cell's own
border).

    .content tr:first-child td { border-top: 0; }

This has no effect because of border collapsing.

This rule is removed, because tables look fine without it
=========================================================

    .content td:first-child { padding-right: 20px; }

By removing this rule, the first cell in each row has the same padding as all
other cells in the row.

This rule is kept, and converted to directly target `.docblock`
===============================================================

    .content td { vertical-align: top; }

Removing this rule would cause it to be aligned to the middle instead.
@rustbot rustbot added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Sep 13, 2022
@rustbot
Copy link
Collaborator

rustbot commented Sep 13, 2022

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @Folyd, @jsha

@rust-highfive
Copy link
Collaborator

r? @CraftSpider

(rust-highfive has picked a reviewer for you, use r? to override)

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

Thanks! r=me once CI pass

@notriddle
Copy link
Contributor Author

@bors r=GuillaumeGomez

@bors
Copy link
Contributor

bors commented Sep 13, 2022

📌 Commit c950d82 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 Sep 13, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 14, 2022
Rollup of 6 pull requests

Successful merges:

 - rust-lang#101433 (Emit a note that static bounds from HRTBs are a bug)
 - rust-lang#101684 (smol grammar changes to README.md)
 - rust-lang#101769 (rustdoc: remove redundant CSS `.out-of-band > span.since { position }`)
 - rust-lang#101772 (Also replace the placeholder for the stable_features lint)
 - rust-lang#101773 (rustdoc: remove outdated CSS `.content table` etc)
 - rust-lang#101779 (Update test output for drop tracking)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3cb8806 into rust-lang:master Sep 14, 2022
@rustbot rustbot added this to the 1.65.0 milestone Sep 14, 2022
@notriddle notriddle deleted the notriddle/content-table branch September 14, 2022 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

6 participants