Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Make impl section headers sticky #133717

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
GuillaumeGomez wants to merge 3 commits into rust-lang:master
base: master
Choose a base branch
Loading
from GuillaumeGomez:sticky-headings

Conversation

Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Dec 1, 2024

Fixes #133506.

It makes impl sections headings sticky, allowing users to know what impl items they are currently going through.

You can test it here.

Some technical details: to make it work (because summary use both ::before and ::after pseudo-elements), I needed to give some left margin and padding so that the other collapse/expand toggles of children items would not appear on the sticky heading's toggle. Hence why the GUI tests new values look so weird.

r? @notriddle

lukas-code and dtolnay reacted with thumbs up emoji RalfJung, jfrimmel, and Kobzol reacted with heart emoji
@rustbot 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 1, 2024
Copy link
Collaborator

rustbot commented Dec 1, 2024

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha

@aDotInTheVoid aDotInTheVoid added A-rustdoc-ui Area: Rustdoc UI (generated HTML) T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. labels Dec 1, 2024

This comment has been minimized.

Copy link
Member Author

Forgot to push the second commit updating GUI tests...

Copy link
Member

joshtriplett commented Dec 2, 2024
edited
Loading

I love it! https://rustdoc.crud.net/imperio/sticky-headings/std/primitive.slice.html looks great.

I can confirm that it works as expected in Firefox on both desktop and mobile.

GuillaumeGomez reacted with heart emoji

Copy link
Contributor

It's messed up when the sidebar is hidden. Otherwise, seems fine.

Copy link
Member Author

Kinda fixed the mess up with the sidebar button. Not perfect but sadly, we can't set an element position based on the page scroll without JS, and with JS, having a listener on the page scroll is terrible for performance. So instead, I simply make the button above the heading. It'll prevent to collapse the heading if the button is present, but I think it's ok.

This comment has been minimized.

This comment has been minimized.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Dec 11, 2024

This comment has been minimized.

This comment has been minimized.

@rfcbot rfcbot removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Dec 11, 2024
@GuillaumeGomez GuillaumeGomez added disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. and removed T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Dec 11, 2024
Copy link
Member Author

@rfcbot fcp merge

Copy link

rfcbot commented Dec 11, 2024
edited
Loading

Team member @GuillaumeGomez has proposed to merge this. The next step is review by the rest of the tagged team members:

Concerns:

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Dec 11, 2024
Copy link
Contributor

The trouble with this sort of thing is that it's really hard to get all the interaction features right. Here's a few cases that act weird:

Copy link
Contributor

@rfcbot concern scroll-margin-top

@rfcbot concern scroll after collapse

@rfcbot concern mobile hidden persistent navigation bar

Copy link
Member

(In favor of this general design, but echo notriddle's concern that this is really tricky to get right, so we should spend some time experimenting and ensuring it works in all scenarios)

Copy link
Collaborator

bors commented Jan 30, 2025

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

Copy link
Contributor

kpreid commented Apr 16, 2025
edited
Loading

I'm interested in seeing this happen to make impls more comprehensible, but I would like to recommend that this sticky header should be optional. I know that optional UI features are a cost in themselves, but I have two specific reasons that unconditional stickiness could be problematic:

  • If the sticky header is tall enough, relative to the viewport (yes, I do use rustdoc on my phone) then there may not be enough remaining space to comfortably read the documentation text.
    • And how much space that is will depend not just on the viewport size but also the content and style of the documentation itself — how tall the function signatures, body text, code examples, etc. are, and how much context is needed from one part to understand another. So, a size threshold would not completely solve the problem.
  • Some people find sticky headers and other content-overlapping fixed elements inherently distressing to view (I have a little bit of this myself) — it's something about losing the consistency that the entire content scrolls as a single unit within a well-defined boundary, combined with the shrinking of the effective usable viewport. Being able to avoid sticky headers should be considered an accessibility feature.
lolbinarycat reacted with thumbs up emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Labels
A-rustdoc-ui Area: Rustdoc UI (generated HTML) disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

rustdoc: Please use sticky headers for impl blocks

AltStyle によって変換されたページ (->オリジナル) /