Skip to content
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

Fix expand/collapse all on site, make highlightjs lazier #14038

Merged
merged 1 commit into from
Feb 1, 2025

Conversation

Alexendoo
Copy link
Member

@Alexendoo Alexendoo commented Jan 20, 2025

This fixes the buttons that expand/collapse all the lints in the list, it also makes code block syntax highlighting only happen when the specific lint enters the viewport since highlighting them all at once was fairly heavy

There's also a few miscellaneous inline event handler removals

script.js and highlightjs are now loaded with defer so that the download can start earlier

Also fixes #14048, we were calling highlight on the pre in <pre><code>...</code></pre> but highlightjs wants us to call it on the code element

changelog: none

@rustbot
Copy link
Collaborator

rustbot commented Jan 20, 2025

r? @blyxyas

rustbot has assigned @blyxyas.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 20, 2025
@Alexendoo Alexendoo force-pushed the site-expand-collapse branch from 84dcb9a to bb746e1 Compare January 22, 2025 18:55
@tommy-gilligan
Copy link

ty for this :D just tried to use expand all button at https://rust-lang.github.io/rust-clippy/master/index.html but can't :(

Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a single nit, it's looking pretty good! (Tested manually)

document.getElementById("expand-all").addEventListener("click", () => toggleExpansion(true));
document.getElementById("collapse-all").addEventListener("click", () => toggleExpansion(false));

document.addEventListener("click", event => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be more consistent with the code above it if we just used document.getElementsByClassName. it does exactly what the code looks for, but in a built-in method.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bit is using event delegation in order to avoid adding ~1500 listeners

Maybe not a noticeable difference but it shaves off a few ms

if (!'IntersectionObserver' in window) {
return;
}
const observer = new IntersectionObserver((entries) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really smart move here! 👏

@Alexendoo Alexendoo force-pushed the site-expand-collapse branch from bb746e1 to 33bb8af Compare January 30, 2025 18:01
Copy link
Member

@blyxyas blyxyas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks! ❤️

@blyxyas blyxyas added this pull request to the merge queue Feb 1, 2025
Merged via the queue into rust-lang:master with commit c607f78 Feb 1, 2025
8 checks passed
@Alexendoo Alexendoo deleted the site-expand-collapse branch February 2, 2025 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Code highlighting on lints website using SCSS language a lot somehow?
4 participants