Skip to content

Commit

Permalink
Rollup merge of #93067 - jsha:fix-scroll-padding-top, r=GuillaumeGomez
Browse files Browse the repository at this point in the history
rustdoc mobile: fix scroll offset when jumping to internal id

Followup to #92692. The semantics of `scroll-margin-top` are a little surprising - the attribute needs to be applied to the element that gets scrolled into the viewport, not the scrolling element.

This fixes an issue where clicking on a method (or other item) from the sidebar takes you to a scroll position where the topbar covers up the method name.

I'm interested in ideas for how to test this with browser-ui-test, but I think it doesn't yet have what I need. What I need is an assert that `<element>.getBoundingClientRect().y` is > 45.

Demo: https://rustdoc.crud.net/jsha/fix-scroll-padding-top/std/string/struct.String.html#method.extend_from_within

r? `@GuillaumeGomez`
  • Loading branch information
matthiaskrgr committed Jan 20, 2022
2 parents dc393b2 + 801ac0e commit 35a53b2
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 1 deletion.
9 changes: 8 additions & 1 deletion src/librustdoc/html/static/css/rustdoc.css
Original file line number Diff line number Diff line change
Expand Up @@ -1735,12 +1735,19 @@ details.rustdoc-toggle[open] > summary.hideme::after {
}

@media (max-width: 700px) {
/* When linking to an item with an `id` (for instance, by clicking a link in the sidebar,
or visiting a URL with a fragment like `#method.new`, we don't want the item to be obscured
by the topbar. Anything with an `id` gets scroll-margin-top equal to .mobile-topbar's size.
*/
*[id] {
scroll-margin-top: 45px;
}

.rustdoc {
padding-top: 0px;
/* Sidebar should overlay main content, rather than pushing main content to the right.
Turn off `display: flex` on the body element. */
display: block;
scroll-margin-top: 45px;
}

main {
Expand Down
6 changes: 6 additions & 0 deletions src/test/rustdoc-gui/sidebar-mobile.goml
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,9 @@ assert-css: (".sidebar", {"display": "block", "left": "-1000px"})

// Check that the topbar is visible
assert-property: (".mobile-topbar", {"clientHeight": "45"})

// Check that clicking an element from the sidebar scrolls to the right place
// so the target is not obscured by the topbar.
click: ".sidebar-menu-toggle"
click: ".sidebar-links a"
assert-position: ("#method\.must_use", {"y": 45})

0 comments on commit 35a53b2

Please sign in to comment.