Generate list instead of div items in sidebar

[GuillaumeGomez]

Fixes #92986.

Surprisingly, we didn't have much CSS for this...

Demo.

r? @jsha

[rust-highfive]

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez,@Folyd,@jsha

[bors]

☔ The latest upstream changes (presumably #93795) made this pull request unmergeable. Please resolve the merge conflicts.

[jsha]

Thanks! Can you put up a demo?

[GuillaumeGomez]

Sorry, forgot to push it... Here you go: https://rustdoc.crud.net/imperio/links-in-sidebar/std/index.html

[jsha]

Generally looks great, thanks for working on this!

I mentioned in the review, but I think each portion of the sidebar that has a heading should get its own <div class="block">. That would make the HTML structure more consistent, and also simplify the styling. If we want to get fancy, the <section> element actually seems like a nice semantic replacement for <div class="block">. But I don't feel strongly about whether we use it or not.

By the way, I notice that wherever we use <div class="block">, there's a second class, like block items, block struct etc. Do we ever actually use that second class? Can we get rid of it?

[jsha]

It's usually best to avoid the :not() selector.

But also, I'm surprised you needed to add this. The existing .block { margin-bottom: 2em } rule should have been enough.

Looking at the HTML, it looks like the whole first part of the sidebar (items on this page) is contained in a single .block. But down below, for items in this module, there's a separate block for each heading. So the structure is like:

<div class="block <foo>">
  <h3>...</h3>
  <ul>...</ul>
</div>

We should make the structure up above the same, so there is a separate block for each heading plus the <ul> underneath it. Then you won't need the extra CSS rule here, and the format of the HTML will be more consistent.

[GuillaumeGomez]

It's what I wanted to do at first but I didn't know how to make the different between item's items and the rest. Maybe using <section> will do the trick.

[jsha]

I think this would be better:

Suggested change

	.sidebar-elems .block.items a {
	.sidebar-elems li {

The rule doesn't need to be specific to .block.items. For instance, it should apply to the "items in this module" section too. Also: selecting on li instead of a is a nice easy way to say "Headings (h2, h3) can wrap, even when they contain an <a> since we generated them and we know they are of a reasonable length. However, list items are Rust item names; they're out of our control, sometimes quite long, and wrap weirdly. Truncate them rather than wrap."

[GuillaumeGomez]

Currently it's not the case, which is why I only applied it on these items specifically. But if you want, I can do it for all.

[GuillaumeGomez]

Now all items in the sidebar are rendered the same way (wrapped in ul itself in a div.block).

I also applied the word-wrap for all li elems in the sidebar making this change:

In the current nightly:

With this PR:

I updated the demo too.

[jsha]

I love the refactoring to add print_sidebar_block. That's a nice abstraction. Rather than taking a callback, I think it should take impl Iterator<Item = &str>. That would be simpler, and AFAICT all of the inputs can be expressed as iterators.

[jsha]

I was thinking that <section> (no class) would replace <div class=\"block\">. Then the relevant CSS selectors would be .sidebar section.

[GuillaumeGomez]

Oh I see. Even better then!

[GuillaumeGomez]

Applied your suggestions! I also updated the demo.

[jsha]

@bors r+ rollup

[bors]

📌 Commit 0d928b6 has been approved by jsha