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

Optimize the documentation JS files #20075

Merged
merged 3 commits into from
Apr 4, 2024
Merged

Optimize the documentation JS files #20075

merged 3 commits into from
Apr 4, 2024

Conversation

RedYetiDev
Copy link
Contributor

This PR optimizes the three files in the /docs/_assets/js directory for performance.

In toolbar.js, jQuery selectors have been cached as variables to prevent constant calls to the DOM.
In sidebar.js, the multiple toggleClass(...).toggleClass(...) have been merged into one
In api-search.js:

  • Defined functions have been switched to newer ES format (const myFunc = () => {})
  • regexForTerm has been switched to ternary
  • JSON has been switched to JSON ({"key":"value"} -> {key:"value"})

@Gedochao Gedochao requested a review from jchyb April 3, 2024 10:35
Copy link
Contributor

@jchyb jchyb left a comment

Choose a reason for hiding this comment

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

Hi @RedYetiDev! I should mention now that I am very far from the .js expert, so I may not be up-to-date on the features, or more importantly, the convention used for js code (like which function syntax should be used/is more popular). With that said I'm going to look at this from the scala perspective. Also since the bulk of the changes are concerning syntax changes without any performance improvements, I'm going to be a bit more picky with those (apologies in advance!).

docs/_assets/js/api-search.js Outdated Show resolved Hide resolved
docs/_assets/js/toolbar.js Outdated Show resolved Hide resolved
@RedYetiDev RedYetiDev requested a review from jchyb April 3, 2024 14:11
@RedYetiDev
Copy link
Contributor Author

Once the changes are approved, If you'd like, I'll squash before merging

@RedYetiDev
Copy link
Contributor Author

Okay, would you like me to squash before we merge?

@jchyb
Copy link
Contributor

jchyb commented Apr 3, 2024

That's not necessary, I can squash while merging using GitHub UI

@RedYetiDev
Copy link
Contributor Author

Sounds good!

@jchyb jchyb merged commit fa2f7bf into scala:main Apr 4, 2024
19 checks passed
@Kordyjan Kordyjan added this to the 3.5.0 milestone May 10, 2024
WojciechMazur pushed a commit that referenced this pull request Jul 4, 2024
This PR optimizes the three files in the `/docs/_assets/js` directory
for performance.

In `toolbar.js`, jQuery selectors have been cached as variables to
prevent constant calls to the DOM.
In `sidebar.js`, the multiple `toggleClass(...).toggleClass(...)` have
been merged into one
In `api-search.js`:
- Defined functions have been switched to newer ES format (`const myFunc
= () => {}`)
- JSON has been switched to **JS**ON (`{"key":"value"}` ->
`{key:"value"}`)
[Cherry-picked fa2f7bf]
WojciechMazur added a commit that referenced this pull request Jul 5, 2024
Backports #20075 to the LTS branch.

PR submitted by the release tooling.
[skip ci]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants