![](/style/images/good.png)
![](/style/images/bad.png)
rustdoc: staggered layout for module contents on mobile by dns2utf8 · Pull Reque...
source link: https://github.com/rust-lang/rust/pull/85651
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
New issue
rustdoc: staggered layout for module contents on mobile #85651
Conversation
This PR adds the container <item-table>
with its two children <item-left>
and <item-right>
.
It uses grid-layout on desktop and flexbox on mobile to make better use of the available space.
Additionally it allows to share parts of the CSS with the search function.
Desktop
Mobile
Some changes occurred in HTML/CSS/JS.
This comment has been hidden.
Note that the item names (e.g. GenericArray
) are supposed to be in sans-serif. See #85621, where we recently fixed this by restoring a CSS rule that applied sans-serif to links in tables. I think you'll want to replace that CSS rule with one that applies sans-serif to this new layout. Probably it would be best to give these items a class of their own, and apply the CSS to that class.
Thanks for the hint! Currently it looks exactly the same (except for 1px more vertical space between the table item and the description and the serif links you mentioned) as the table variant, I added a bit of vertical space already.
My plan is to rebase on master next week and demo with a bigger crate than genneric_array, which I choose because it uses almost all rustdoc features.
If you have a better suited crate I will render that one too :)
Another question: currently the element names item-table
, item-left
and item-right
are very expressive. I could shorten them and potentially reduce the size of the rendered HTML. I guesstimate that the current choice consumes about the same space as the table
variant, but we could save a little bit if we wanted.
What do you think?
I hope it will. :)
This comment has been hidden.
@@ -1756,7 +1786,8 @@ details.undocumented[open] > summary::before {
.search-results .result-name, .search-results div.desc, .search-results .result-description {
width: 100%;
}
.search-results div.desc, .search-results .result-description {
/* Display second row of staggered layouts */
.search-results div.desc, .search-results .result-description, item-right {
jsha on May 30
Contributor
I was thinking about the "ideal" spacing some more. For the staggered layout in search, I suggested 2em
out of the blue. It would be nice for consistency to use a spacing that we use elsewhere. For instance, for docblocks we indent them 24px relative to their heading:
.docblock {
margin-left: 24px;
position: relative;
}
That works out to about 1.5em at our font sizes. So I think we should change padding-left: 2em
to margin-left: 24px
with a comment that we are matching it to the docblock indent.
When I access your demo, I get this message in the Firefox console:
Refused to apply inline style because it violates the following Content Security Policy directive: "default-src 'self'". Either the 'unsafe-inline' keyword, a hash ('sha256-2TDOnxRwDtZllEt9eciuRNF6fRBZiYRlTWKiW4H0dK0='), or a nonce ('nonce-...') is required to enable inline execution. Note also that 'style-src' was not explicitly set, so 'default-src' is used as a fallback.
It probably doesn't make a difference for this particular PR but you might want to exempt your demo paths from that CSP rule, since it could make a difference in rendering.
Other than my comment about margin-left
, and the test failures, this looks good to me. @GuillaumeGomez do you want to give feedback before merge or can I r+ once the tests are fixed?
This comment has been hidden.
Is your intent for item-table / item-left / item-right to be used elsewhere in the markup, or just for the listing of items in the module?
If the former, I'd give this item-table a class "module-items" and change the XPath selector to //item-table[class=module-items]/item-right.
If the latter I'd do the same selector but without the class.
The latest upstream changes (presumably #84703) made this pull request unmergeable. Please resolve the merge conflicts.
This comment has been hidden.
I did plan to use it as a more generic way for all the static tables.
We can not use it for the search results for now, because we rely on the :hover
class and for that we would need a grouping element which is not compatible with grid layout.
So for now, I am going with the item path without the class.
PS: I am using the grid layout because it automatically compacts the first column which would require active JavaScript with flexbox
This comment has been hidden.
I fixed all the tests but there is a gap that is untestable:
- with the current python parser we can not verify sibling relationships
- only top down selectors are supported
There appears to be an alternative: lxml
However, the compatibility page indicates that we might have to put some work into a migration.
What do you think?
Apart from that, I think this PR is pretty much ready please review the demo here because optically it is pretty much the same as before:
https://data.estada.ch/rustdoc-nightly_2336406b38_2021-06-16/rustdoc_demo/
This comment has been hidden.
@bors retry
This comment has been hidden.
The CI really doesn't want this PR (or is very broken).
Any info about this @pietroalbini ?
I rebased the PR on master again, hope this helps
Waiting for CI to confirm then approving again.
Commit 94c84bd has been approved by
GuillaumeGomez
Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 7c3872e to master...
It merged
Congrats! Thanks for sticking through it. This will be a very nice improvement.
Thank you for supporting it!
None yet
Successfully merging this pull request may close these issues.
None yet
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK