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.

Sign up for GitHub

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

Jump to bottom

Remove jQuery from mkdocs theme #3649

Merged
merged 2 commits into from
Apr 20, 2024
Merged

Remove jQuery from mkdocs theme #3649

merged 2 commits into from
Apr 20, 2024

Conversation

squidfunk
Copy link
Contributor

Since the mkdocs theme updated to Bootstrap 5, jQuery is not necessary anymore. I've replaced all uses of jQuery with common browser APIs and adapted the use of the modals to Bootstrap 5. My testing shows that master behaves exactly as if this PR were merged. If not, please report errors here and I'll fix them.

I don't think jQuery can be removed from the readthedocs theme.

Closes #3648

@squidfunk squidfunk mentioned this pull request Apr 17, 2024
Merged
@squidfunk squidfunk requested a review from oprypin April 17, 2024 01:45
@squidfunk squidfunk mentioned this pull request Apr 17, 2024
Closed
@squidfunk
Copy link
Contributor Author

Side note: merging this PR will reduce the payload that the browser needs to download by 90kb (30kb gzipp'd), which is an improvement of -14% (currently 213kb gzipp'd). Thus, the MkDocs core theme should load faster!

@squidfunk squidfunk requested review from waylan and tomchristie April 18, 2024 01:21
waylan
waylan approved these changes Apr 18, 2024
View reviewed changes
Copy link
Member

@waylan waylan left a comment

Choose a reason for hiding this comment

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

Looks good to me. It is great to see this being worked on. I looked through the changes and they look good to me, but I'm weakest with JavaScript so I may have missed something.

@squidfunk
Copy link
Contributor Author

squidfunk commented Apr 18, 2024 •
edited
Loading

Thanks for taking the time to look at this! ❤️

Do we want to release this with MkDocs 1.6.0, or afterwards? I've tested all that was apparent to me from reading the code and have found no problems (except for the visual regression mentioned in #3493 (comment), which are not related to the changes in this PR), but I'm also not a heavy user of the mkdocs theme.

If it should go into 1.6.0, we can merge it.

@waylan
Copy link
Member

waylan commented Apr 18, 2024

I would include this in 1.6.0 if we can.

@oprypin oprypin added this to the 1.6.0 milestone Apr 18, 2024
@oprypin oprypin linked an issue Apr 18, 2024 that may be closed by this pull request
update all jquery usage to remove quarantining #2467
Closed
@tomchristie
Copy link
Member

I would include this in 1.6.0 if we can.

Seems reasonable. See also... #3648 (comment)

@squidfunk
Copy link
Contributor Author

Great! Since everybody seems to agree that we want this in 1.6.0, let's merge it.

@squidfunk squidfunk merged commit 6244500 into master Apr 20, 2024
34 checks passed
@squidfunk squidfunk deleted the refactor/remove-jquery-mkdocs-theme branch April 20, 2024 00:41
@squidfunk squidfunk mentioned this pull request Apr 20, 2024
Closed
@pawamoy pawamoy mentioned this pull request May 2, 2024
Closed
@squidfunk squidfunk mentioned this pull request Jul 21, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@waylan waylan waylan approved these changes

@oprypin oprypin Awaiting requested review from oprypin

@tomchristie tomchristie Awaiting requested review from tomchristie

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.6.0
Development

Successfully merging this pull request may close these issues.

Remove jQuery dependency from mkdocs theme update all jquery usage to remove quarantining
4 participants
@squidfunk @waylan @tomchristie @oprypin