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

Remove jQuery loading by default #252

Merged
merged 1 commit into from
Mar 9, 2023

Conversation

lb-
Copy link
Member

@lb- lb- commented Mar 8, 2023

- Sill used as a peer dependency of Bootstrap
- Core jQuery dependency removed though
- Fixes wagtail#250
@lb-
Copy link
Member Author

lb- commented Mar 8, 2023

@vsalvino is this what you were thinking?

Note: I have not dug into JS usage of bootstrap, it appears we are not really using anything but I have left it importing in theme.js (the bootstrap import).

At least we can see a deployment and have a look.

@vsalvino
Copy link
Contributor

vsalvino commented Mar 8, 2023

That should do it. I think the only bootstrap JS feature we may be using is the mobile menu... does that still work correctly with this change?

@vsalvino
Copy link
Contributor

vsalvino commented Mar 8, 2023

P.S... bootstrap 5 did remove jquery as a dependency. Not sure how broken that would be to upgrade, or if it would be a mostly drop-in replacement given our minimal bootstrap usage. https://getbootstrap.com/docs/5.2/migration/

@lb-
Copy link
Member Author

lb- commented Mar 8, 2023

The mobile menu expander works fine in the build but it could be driven by the peer dep.

Yeah maybe BS 5 is an option.

I might not get the chance to look at this one again for a bit though

@vsalvino
Copy link
Contributor

vsalvino commented Mar 8, 2023

I think this looks good then. I'll look into Bootstrap 5 when I get a chance.

@thibaudcolas
Copy link
Member

Thank you both!

@thibaudcolas thibaudcolas merged commit 1e4d8e1 into wagtail:main Mar 9, 2023
@thibaudcolas thibaudcolas added bug Something isn't working dependencies Pull requests that update a dependency file labels Mar 9, 2023
@lb- lb- deleted the chore/250-remove-jquery branch March 9, 2023 20:03
@lb-
Copy link
Member Author

lb- commented Mar 11, 2023

Bootstrap 5 discussion moved to a new issue #254

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JavaScript broken with Sphinx 6
3 participants