Wrap too long type name#127418

GuillaumeGomez · 2024-07-06T10:15:20Z

Fixes #120595.

Takeover of #126209.

cc @BradMarr
r? @notriddle

rustbot · 2024-07-06T10:15:29Z

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha

GuillaumeGomez · 2024-07-06T14:54:42Z

Weird, different size locally vs in CI. Oh well...

notriddle

Okay, it's really, really important to include a screenshot on a change that's this drastic:

Don't like it.

Also:

If the text is going to be aligned to the right, it should be consistently aligned to the right. Notice, in the above screenshot, array is ragged-right, but slice is ragged-left.
Even if it is consistent, that's merely better. It's not actually good. Large amounts of English, like these descriptions, should always be written left-to-right.

notriddle · 2024-07-06T15:45:44Z

src/librustdoc/html/static/css/rustdoc.css

+	justify-content: space-between;
 }
 .item-table > li > div {
 	display: table-cell;


This can't actually do anything any more.

BradMarr · 2024-07-06T16:13:39Z

Yeah that's what I figured, I couldn't figure out how to do it the way it was and having a max width.

the8472 · 2024-07-07T10:58:45Z

#120595 (comment) cites table layout intrinsic width issues.

I'm not sure if that's correct. The issue seems to be that the table columns have different overflow-wrap rules which I think leads one column to hog all the space at the expense of the other. If both are set to anywhere then it balances the available space and it looks fine(-ish). It could use some polish to indicate that wrapping happened, a dash or an indent on the 2nd line.

GuillaumeGomez · 2024-07-17T12:35:09Z

Finally took the time to update. I reverted back to the table display but with some small differences: there is now a padding at the bottom to make it easier to spot which item's short doc we're reading and also set the width of the items name fixed (to 50% of the parent) so that whatever their size, the display remains the same. Screenshots:

GuillaumeGomez · 2024-07-17T12:35:37Z

I'll add GUI tests once we agree with the UI changes.

@rustbot ready

notriddle · 2024-07-17T15:38:50Z

Does @the8472's solution work? It might look better than the fixed-width setup with huge gaps.

GuillaumeGomez · 2024-07-18T12:35:17Z

If one item is super long and the others are "normal", then the rendering is the same. One thing I dislike with the current display is that the position of the short documentation changes depending on the width of the biggest item name. That's why I decided to go for fixed width (with still handling too long name).

notriddle · 2024-07-18T13:46:11Z

Okay, that makes sense.

Next question: instead of making the name 50% of the size, make it 33%? Item names are usually a lot shorter than descriptions, and the huge blank space between them makes it hard to match them up (especially since there’s no zebra striping on this table).

GuillaumeGomez · 2024-07-18T13:49:34Z

That's a good point. Gonna add UI tests to this new size then.

GuillaumeGomez · 2024-07-18T18:48:47Z

Reduced the width of items name to 33% and added a GUI test.

notriddle · 2024-07-18T20:09:50Z

@bors r+

bors · 2024-07-18T20:09:53Z

📌 Commit c820a23 has been approved by notriddle

It is now in the queue for this repository.

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#127418 (Wrap too long type name) - rust-lang#127594 (Fuchsia status code match arm) - rust-lang#127835 (Fix ICE in suggestion caused by `⩵` being recovered as `==`) - rust-lang#127858 (match lowering: Rename `MatchPair` to `MatchPairTree`) - rust-lang#127871 (Mention that type parameters are used recursively on bivariance error) - rust-lang#127913 (remove `debug-logging` default from tools profile) - rust-lang#127925 (Remove tag field from `Relation`s) - rust-lang#127929 (Use more accurate span for `addr_of!` suggestion) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 8 pull requests Successful merges: - rust-lang#127418 (Wrap too long type name) - rust-lang#127594 (Fuchsia status code match arm) - rust-lang#127835 (Fix ICE in suggestion caused by `⩵` being recovered as `==`) - rust-lang#127858 (match lowering: Rename `MatchPair` to `MatchPairTree`) - rust-lang#127871 (Mention that type parameters are used recursively on bivariance error) - rust-lang#127913 (remove `debug-logging` default from tools profile) - rust-lang#127925 (Remove tag field from `Relation`s) - rust-lang#127929 (Use more accurate span for `addr_of!` suggestion) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#127418 - GuillaumeGomez:wrap-too-long-type-name, r=notriddle Wrap too long type name Fixes rust-lang#120595. Takeover of rust-lang#126209. cc `@BradMarr` r? `@notriddle`

cynecx · 2024-07-22T01:26:25Z

src/librustdoc/html/static/css/rustdoc.css

+		width: 33%;
+	}
+	.item-table > li > div {
+		padding-bottom: 5px;


@GuillaumeGomez Maybe it's just me but this makes the spacing in the y-axis look so "wasteful" and a bit unnatural. Is this really necessary? Or why can't we keep the old behavior.

…llaumeGomez rustdoc: revert spacing change in item-table It really wasn't necessary for the bug fix, and could reasonably be considered a functional regression. In response to rust-lang#127418 (comment)

Rollup merge of rust-lang#128051 - notriddle:notriddle/spacing, r=GuillaumeGomez rustdoc: revert spacing change in item-table It really wasn't necessary for the bug fix, and could reasonably be considered a functional regression. In response to rust-lang#127418 (comment)

rustbot assigned notriddle Jul 6, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jul 6, 2024

GuillaumeGomez mentioned this pull request Jul 6, 2024

Update rustdoc.css #126209

Closed

This comment has been minimized.

Sign in to view

GuillaumeGomez force-pushed the wrap-too-long-type-name branch from 06b735e to 587dff9 Compare July 6, 2024 11:16

notriddle requested changes Jul 6, 2024

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 6, 2024

GuillaumeGomez force-pushed the wrap-too-long-type-name branch from 587dff9 to 35fb71f Compare July 17, 2024 12:33

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 17, 2024

This comment has been minimized.

Sign in to view

notriddle added T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. and removed T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jul 17, 2024

GuillaumeGomez added 2 commits July 18, 2024 20:48

Wrap too long item name and improve the item list display a bit

fed0592

Add test for size of items in the items list

c820a23

GuillaumeGomez force-pushed the wrap-too-long-type-name branch from 35fb71f to c820a23 Compare July 18, 2024 18:48

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 Jul 18, 2024

matthiaskrgr mentioned this pull request Jul 18, 2024

Rollup of 8 pull requests #127936

Merged

bors merged commit 7c1bf86 into rust-lang:master Jul 19, 2024

rustbot added this to the 1.81.0 milestone Jul 19, 2024

GuillaumeGomez deleted the wrap-too-long-type-name branch July 19, 2024 08:52

cynecx reviewed Jul 22, 2024

View reviewed changes

notriddle mentioned this pull request Jul 22, 2024

rustdoc: revert spacing change in item-table #128051

Merged

Uh oh!

Conversation

GuillaumeGomez commented Jul 6, 2024

Uh oh!

rustbot commented Jul 6, 2024

Uh oh!

This comment has been minimized.

GuillaumeGomez commented Jul 6, 2024

Uh oh!

notriddle left a comment

Choose a reason for hiding this comment

Uh oh!

notriddle Jul 6, 2024

Choose a reason for hiding this comment

Uh oh!

BradMarr commented Jul 6, 2024

Uh oh!

the8472 commented Jul 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GuillaumeGomez commented Jul 17, 2024

Uh oh!

GuillaumeGomez commented Jul 17, 2024

Uh oh!

This comment has been minimized.

notriddle commented Jul 17, 2024

Uh oh!

GuillaumeGomez commented Jul 18, 2024

Uh oh!

notriddle commented Jul 18, 2024

Uh oh!

GuillaumeGomez commented Jul 18, 2024

Uh oh!

GuillaumeGomez commented Jul 18, 2024

Uh oh!

notriddle commented Jul 18, 2024

Uh oh!

bors commented Jul 18, 2024

Uh oh!

cynecx Jul 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

notriddle Jul 22, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

the8472 commented Jul 7, 2024 •

edited

Loading

cynecx Jul 22, 2024 •

edited

Loading