[rustdoc] Fix display of features #118677

GuillaumeGomez · 2023-12-06T14:46:24Z

It now looks like this:

We can't use flex without breaking the flow, meaning we can't vertically align items as we want. Because of that, the min-height was problematic as it rendered weirdly and therefore needed to be removed.

r? @notriddle

rustbot · 2023-12-06T14:46:33Z

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha

notriddle · 2023-12-06T14:58:01Z

Would line-height: 36px fix the uneven height of these elements, or does it mess up something else?

GuillaumeGomez · 2023-12-06T15:00:21Z

With line-height: 36px;:

notriddle · 2023-12-06T16:03:05Z

Right, but the point of me asking is that the ones with the 👎 should be the same height as the ones without any emoji in them, as well as items like this:

notriddle · 2023-12-06T16:05:23Z

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

@@ -1100,11 +1094,9 @@ so that we can apply CSS-filters to change the arrow color in themes */
 	font-weight: normal;
 	color: var(--main-color);
 	background-color: var(--stab-background-color);
-	width: fit-content;


Why? This seems to still work, even when it's display: block.

It wasn't in use said firefox, so I removed it.

Weird. I'm not seeing this when I try it.

Ah? Is the badge still taking the full width then?

No it's not.

Ah then we know why I changed it. :D

Do you want the badge to take up the full width even when it isn't full?

Currently, it only takes up the width that it actually needs, which is what width: fit-content is for. That's intentional, so that these badges are less prominent. Did you mean to change it?

Oh you're right. No I didn't meant to change it. Might be worth doing it though but I'll open another PR for that. Let me revert this change.

Thanks!

I don’t mean to be a pain, but can you also add in the line-height: 36px? With that, the changes should be very minimal other than fixing the bug.

No don't worry. Just curious about the why. It looks very bad with it. Even more when there are no emoji.

GuillaumeGomez · 2023-12-06T16:08:07Z

Right, but the point of me asking is that the ones with the 👎 should be the same height as the ones without any emoji in them, as well as items like this:

I explained my reasoning here:

We can't use flex without breaking the flow, meaning we can't vertically align items as we want. Because of that, the min-height was problematic as it rendered weirdly and therefore needed to be removed.

GuillaumeGomez · 2023-12-06T19:00:29Z

I reverted the unwanted width change. I think it's worth considering it but let's do that in another PR.

notriddle · 2023-12-06T19:23:49Z

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

-	*/
-	min-height: 36px;
-	display: flex;
+	display: block;
 	padding: 3px;


Oh, I see why it looks different. Try this.

Suggested change

padding: 3px;

padding: 0 3px;

line-height: 36px;

Still looks bad when there is more than one line:

Yeah, that is more line spacing than I'd like. Here's one more attempt to get a decent rendering (where ones with both emoji and non-emoji have the same height, but there's not excessive line spacing).

.item-info .stab { display:block; padding:3px; margin-bottom:5px; } .item-name .stab { margin-left:0.3125em; } .stab { padding:0 2px; font-size:0.875rem; font-weight:normal; color:var(--main-color); background-color:var(--stab-background-color); width:fit-content; white-space:pre-wrap; border-radius:3px; display:inline; vertical-align:baseline; } .stab.portability>code { background:none; color:var(--stab-code-color); } .stab .emoji, .item-info .stab::before { font-size:1.25rem; } .stab .emoji { margin-right:0.3rem; } .item-info .stab::before { /* ensure badges with emoji and without it have same height */ content: "\0"; width: 0; display: inline-block; color: transparent; } .emoji { text-shadow:1px 0 0 black,-1px 0 0 black,0 1px 0 black,0 -1px 0 black; }

Wow. That's quite impressive, congrats! I updated as suggested. :)

Can you update the big screenshot at the top, too?

notriddle · 2023-12-06T23:40:24Z

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

-
-/* Black one-pixel outline around emoji shapes */


Probably shouldn't remove this comment.

notriddle · 2023-12-06T23:40:35Z

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

-.stab.portability > code {
+.stab.portability>code {


Probably shouldn't change the formatting here.

GuillaumeGomez · 2023-12-07T09:45:19Z

Updated!

notriddle · 2023-12-08T00:22:14Z

@bors r+ rollup

bors · 2023-12-08T00:22:16Z

📌 Commit 9e1797b has been approved by notriddle

It is now in the queue for this repository.

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#118505 (Elaborate on ip_addr bit conversion endianness) - rust-lang#118581 (OnceLock: Add note about drop and statics) - rust-lang#118677 ([rustdoc] Fix display of features) - rust-lang#118690 (coverage: Avoid unnecessary macros in unit tests) - rust-lang#118693 (Tell MirUsedCollector that the pointer alignment checks calls its panic symbol) - rust-lang#118695 (coverage: Merge refined spans in a separate final pass) - rust-lang#118709 (fix jobserver GLOBAL_CLIENT_CHECKED uninitialized before use) - rust-lang#118722 (rustdoc: remove unused parameter `reversed` from onEach(Lazy)) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#118677 - GuillaumeGomez:doc_cfg-display, r=notriddle [rustdoc] Fix display of features Fixes rust-lang#118615. It now looks like this: ![image](https://github.com/rust-lang/rust/assets/3050060/6e77204e-0706-44a3-89ae-2dbd1934ebbc) We can't use flex without breaking the flow, meaning we can't vertically align items as we want. Because of that, the `min-height` was problematic as it rendered weirdly and therefore needed to be removed. r? `@notriddle`

rustbot assigned notriddle Dec 6, 2023

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 Dec 6, 2023

notriddle reviewed Dec 6, 2023

View reviewed changes

GuillaumeGomez force-pushed the doc_cfg-display branch from 42989b7 to 075fbae Compare December 6, 2023 18:59

notriddle reviewed Dec 6, 2023

View reviewed changes

GuillaumeGomez force-pushed the doc_cfg-display branch 2 times, most recently from 28230fb to 5d689dc Compare December 6, 2023 23:22

notriddle requested changes Dec 6, 2023

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 Dec 6, 2023

GuillaumeGomez force-pushed the doc_cfg-display branch from 5d689dc to 535d7d7 Compare December 7, 2023 09:43

GuillaumeGomez added 2 commits December 7, 2023 10:44

Fix display of features in rustdoc

bf681dc

Extend GUI tests for doc_cfg

9e1797b

GuillaumeGomez force-pushed the doc_cfg-display branch from 535d7d7 to 9e1797b Compare December 7, 2023 09:45

notriddle approved these changes Dec 8, 2023

View reviewed changes

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 8, 2023

matthiaskrgr mentioned this pull request Dec 8, 2023

Rollup of 8 pull requests #118732

Merged

bors merged commit fa724cc into rust-lang:master Dec 8, 2023
11 checks passed

rustbot added this to the 1.76.0 milestone Dec 8, 2023

GuillaumeGomez deleted the doc_cfg-display branch August 19, 2024 12:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[rustdoc] Fix display of features #118677

[rustdoc] Fix display of features #118677

GuillaumeGomez commented Dec 6, 2023 •

edited

Loading

rustbot commented Dec 6, 2023

notriddle commented Dec 6, 2023

GuillaumeGomez commented Dec 6, 2023

notriddle commented Dec 6, 2023 •

edited

Loading

notriddle Dec 6, 2023

GuillaumeGomez Dec 6, 2023

notriddle Dec 6, 2023

GuillaumeGomez Dec 6, 2023

notriddle Dec 6, 2023

GuillaumeGomez Dec 6, 2023

notriddle Dec 6, 2023

GuillaumeGomez Dec 6, 2023

notriddle Dec 6, 2023

GuillaumeGomez Dec 6, 2023

GuillaumeGomez commented Dec 6, 2023

GuillaumeGomez commented Dec 6, 2023

notriddle Dec 6, 2023

GuillaumeGomez Dec 6, 2023

notriddle Dec 6, 2023 •

edited

Loading

GuillaumeGomez Dec 6, 2023

notriddle Dec 6, 2023

GuillaumeGomez Dec 7, 2023

notriddle Dec 6, 2023

notriddle Dec 6, 2023

GuillaumeGomez commented Dec 7, 2023

notriddle commented Dec 8, 2023

bors commented Dec 8, 2023

[rustdoc] Fix display of features #118677

[rustdoc] Fix display of features #118677

Conversation

GuillaumeGomez commented Dec 6, 2023 • edited Loading

rustbot commented Dec 6, 2023

notriddle commented Dec 6, 2023

GuillaumeGomez commented Dec 6, 2023

notriddle commented Dec 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez commented Dec 6, 2023

GuillaumeGomez commented Dec 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

notriddle Dec 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuillaumeGomez commented Dec 7, 2023

notriddle commented Dec 8, 2023

bors commented Dec 8, 2023

GuillaumeGomez commented Dec 6, 2023 •

edited

Loading

notriddle commented Dec 6, 2023 •

edited

Loading

notriddle Dec 6, 2023 •

edited

Loading