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

Fix docblock <table> overflow #87230

Merged
merged 3 commits into from
Jul 21, 2021
Merged

Fix docblock <table> overflow #87230

merged 3 commits into from
Jul 21, 2021

Conversation

GuillaumeGomez
Copy link
Member

Before:

Screenshot from 2021-07-18 00-05-23

After:

Screenshot from 2021-07-18 00-03-55

cc @SergioBenitez

r? @notriddle

@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 Jul 17, 2021
@GuillaumeGomez
Copy link
Member Author

GuillaumeGomez commented Jul 17, 2021 •
edited
Loading

A potential "extension" to this fix all potential similar issues would then to add overflow-x: auto; on .docblock directly. However it has "side-effects" which need to be investigated before (padding and margin are working weirdly with it). But that's for another day in another PR.

@GuillaumeGomez GuillaumeGomez changed the title Fix ocblock <table> overflow Fix docblock <table> overflow Jul 17, 2021
@GuillaumeGomez GuillaumeGomez mentioned this pull request Jul 17, 2021
Closed
@GuillaumeGomez
Copy link
Member Author

ping @notriddle :)

@GuillaumeGomez
Copy link
Member Author

@bors: r=notriddle rollup

@bors
Copy link
Contributor

bors commented Jul 21, 2021

📌 Commit 3f1a120 has been approved by notriddle

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 21, 2021
@GuillaumeGomez GuillaumeGomez mentioned this pull request Jul 21, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 21, 2021
@bors
Auto merge of rust-lang#87347 - GuillaumeGomez:rollup-ke92xxc, r=Guil…
05f2326 
…laumeGomez

Rollup of 9 pull requests

Successful merges:

 - rust-lang#87187 (Fix NixOS detection)
 - rust-lang#87206 (avoid temporary vectors/reuse iterators)
 - rust-lang#87230 (Fix docblock <table> overflow)
 - rust-lang#87273 (Recognize bounds on impls as const bounds)
 - rust-lang#87279 (Add comments explaining the unix command-line argument support.)
 - rust-lang#87301 (Fix typo in compile.rs)
 - rust-lang#87311 (Get back the more precise suggestion spans of old regionck)
 - rust-lang#87321 (Add long explanation for E0722)
 - rust-lang#87342 (Add long explanation for E0757)

Failed merges:

 - rust-lang#87270 (Don't display <table> in item summary)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e6380a6 into rust-lang:master Jul 21, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jul 21, 2021
@GuillaumeGomez GuillaumeGomez deleted the docblock-table-overflow branch July 21, 2021 17:11
notriddle added a commit to notriddle/rust that referenced this pull request Dec 8, 2022
@notriddle
rustdoc: clean up docblock table CSS
a09e8c5 
* The rule `display: block` had no noticeable effect. Technically, because
  markdown tables have a tbody and thead, they get wrapped in an [anonymous
  table box] in the CSS tree, nested within the `<table>` element's block
  layout box.

  This rule was added in rust-lang#87230 to make the table side-scrolling, but
  this same issue was doubly fixed in rust-lang#88742 by wrapping it in an explicit
  `<div>` tag. Since accessibility advocates recommend the wrapper div over
  marking the table as `display: block`, we'll stick with that.

  https://adrianroselli.com/2020/11/under-engineered-responsive-tables.html

* The rule `width: calc(100% - 2px)` had no visible effect, because the
  anonymous table box was not affected.

* The style is tweaked to basically be the same style GitHub uses.
  In particular, it adds zebra stripes, and removes dotted borders.

[anonymous table box]: https://www.w3.org/TR/CSS2/tables.html#anonymous-boxes
@notriddle notriddle mentioned this pull request Dec 8, 2022
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 9, 2022
@matthiaskrgr
Rollup merge of rust-lang#105442 - notriddle:notriddle/docblock-table…
441669c 
…-css, r=GuillaumeGomez

rustdoc: clean up docblock table CSS

# Preview

http://notriddle.com/notriddle-rustdoc-demos/table-2/test_dingus/fn.test.html

# Before

![image](https://user-images.githubusercontent.com/1593513/206364287-1b80eaaf-2e0e-4138-8b56-4aa8ff39abac.png)

# After

![image](https://user-images.githubusercontent.com/1593513/206364209-d287d165-31be-4de1-9b43-05b35ce2a86b.png)

# Details

* The rule `display: block` had no noticeable effect. Technically, because markdown tables have a tbody and thead, they get wrapped in an [anonymous table box] in the CSS tree, nested within the `<table>` element's block layout box.

  This rule was added in rust-lang#87230 to make the table side-scrolling, but this same issue was doubly fixed in rust-lang#88742 by wrapping it in an explicit `<div>` tag. Since accessibility advocates recommend the wrapper div over marking the table as `display: block`, we'll stick with that.

  https://adrianroselli.com/2020/11/under-engineered-responsive-tables.html

* The rule `width: calc(100% - 2px)` had no visible effect, because the anonymous table box was not affected.

* The style is tweaked to basically be the same style GitHub uses. In particular, it adds zebra stripes, and removes dotted borders.

  https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/Markdown.20table.20styling

[anonymous table box]: https://www.w3.org/TR/CSS2/tables.html#anonymous-boxes
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
@matthiaskrgr
Rollup merge of rust-lang#105442 - notriddle:notriddle/docblock-table…
a2fc275 
…-css, r=GuillaumeGomez

rustdoc: clean up docblock table CSS

# Preview

http://notriddle.com/notriddle-rustdoc-demos/table-2/test_dingus/fn.test.html

# Before

![image](https://user-images.githubusercontent.com/1593513/206364287-1b80eaaf-2e0e-4138-8b56-4aa8ff39abac.png)

# After

![image](https://user-images.githubusercontent.com/1593513/206364209-d287d165-31be-4de1-9b43-05b35ce2a86b.png)

# Details

* The rule `display: block` had no noticeable effect. Technically, because markdown tables have a tbody and thead, they get wrapped in an [anonymous table box] in the CSS tree, nested within the `<table>` element's block layout box.

  This rule was added in rust-lang#87230 to make the table side-scrolling, but this same issue was doubly fixed in rust-lang#88742 by wrapping it in an explicit `<div>` tag. Since accessibility advocates recommend the wrapper div over marking the table as `display: block`, we'll stick with that.

  https://adrianroselli.com/2020/11/under-engineered-responsive-tables.html

* The rule `width: calc(100% - 2px)` had no visible effect, because the anonymous table box was not affected.

* The style is tweaked to basically be the same style GitHub uses. In particular, it adds zebra stripes, and removes dotted borders.

  https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/Markdown.20table.20styling

[anonymous table box]: https://www.w3.org/TR/CSS2/tables.html#anonymous-boxes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@notriddle notriddle notriddle approved these changes

Assignees
No one assigned
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
Milestone
1.55.0
Development

Successfully merging this pull request may close these issues.
4 participants
@GuillaumeGomez @bors @notriddle @rustbot