-
-
Notifications
You must be signed in to change notification settings - Fork 92
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
Rewrite theme.js to remove jQuery #565
base: main
Are you sure you want to change the base?
Conversation
jQuery quite frankly is a burdensome and weighty library in today's ES6 world. This file is one of the only files in bootscore that still depends on jQuery to function.
That's actually not completely true, there is also woocommerce/js/woocommerce.js and the insides of woocommerce/inc/ajax-add-to-cart.php. So these changes will not completely remove jQuery as dependency, at least not when using woocommerce. |
Ah yeah forgot as we don't use woocommerce. Nonetheless this commit allows us to remove jquery on most pages, and as it's a 45kb library that's a nice bonus. |
I'm not entirely comfortable rewriting those files to remove jquery as we cannot test it. Still, this PR is a good starting point for people like us that may only have one or two plugins that still use jquery. |
I had this on my list as well. It's good that already someone want to do this. Tested it and found 2 issues. Readded the jQuery again to show how this snippets should work:
About the other 2 files @justinkruit mentioned. I'm a huge friend to have a standard and Vanilla JS there would be nice as well. But both files gets only enqueued if WooCommerce plugin is installed and entire WooCommerce' JS is jQuery. It will not make a huge different if we change them to Vanilla JS too. So, in this case I'm fine using Vanilla in theme.js and jQuery in WC scripts. So users who haven't installed WooCommerce have a theme without jQuery dependency when this is merged. Just for info:
Simply setup a testing site, install WooCommerce and you can test them well. But as I wrote above, I think this is not important for now. And while we are here, Internet Explorer warning seems to be outdated and many other browsers are not supported by Bootstrap 5 as well. Think we can delete this in a further update. If we get those 2 issues fixed, I'm in. Let me know what you think. |
And when all is done and fixed, please request both me and @crftwrk as reviewer (if you're able to, not entirely sure if you can as PR author so please test it). I'd like to fully test this when the bugs mentioned above are fixed. |
Set this to draft to avoid accidentally merging. |
If WC depends largely on jquery (which, honestly, i suspected it did but couldn't confirm) there's not much benefit in rewriting it's associated JS in bootscore, as the overall idea here is more optimized code + removing the need for jQuery. It's honestly disappointing such a major plugin still depends on jQuery but so does WordPress so it only makes sense. The other two points i'll check as I have time, unfortunately a bit busy today and tomorrow haha. |
@crftwrk ready to merge when you are |
Nearly ;-)
Changed some code and everything (IE warning not tested but not important) works fine. Is already live on bootscore.me. /*--------------------------------------------------------------
Theme JS
--------------------------------------------------------------*/
// Close offcanvas on click a, keep .dropdown-menu open
// See https://github.com/bootscore/bootscore/discussions/347
document.addEventListener('DOMContentLoaded', function () { // Fire after bootstrap.bundle.min.js is loaded
const offcanvasLinks = document.querySelectorAll('.offcanvas a:not(.dropdown-toggle):not(.remove_from_cart_button)');
const offcanvas = new bootstrap.Offcanvas(document.querySelector('.offcanvas'));
offcanvasLinks.forEach(function (link) {
link.addEventListener('click', function () {
offcanvas.hide();
});
});
});
// Remove search collapse button hide if widget is empty
// Deprecated v5.2.3.4, done by php if (is_active_sidebar('top-nav-search')) in header.php
// Remove in v6
const collapseSearch = document.getElementById('collapse-search');
if (collapseSearch && collapseSearch.children.length === 0) {
document.querySelectorAll('.top-nav-search-md, .top-nav-search-lg').forEach((element) => element.remove());
}
// Focus searchform on click search-toggler
document.querySelector('#collapse-search').addEventListener('shown.bs.collapse', function () {
document.querySelector('.top-nav-search input:first-of-type').focus();
});
// Close search collapse if click outside the searchform
document.addEventListener('click', function (event) {
const target = event.target;
const collapseSearch = document.querySelector('#collapse-search');
const collapseButton = document.querySelector('[data-bs-target="#collapse-search"]');
if (!collapseSearch.contains(target) && collapseSearch.classList.contains('show')) {
collapseButton.click(); // Simulate a click on the collapse button to hide the collapse
}
});
// To-top button
const topButton = document.querySelector('.top-button');
if (topButton) {
window.addEventListener('scroll', function () {
topButton.classList.toggle('visible', window.scrollY >= 500);
});
}
// Height classes, add class to your content
['height-50', 'height-75', 'height-85', 'height-100'].forEach((cls) => {
const factor = parseFloat(cls.split('-')[1]) / 100;
document.querySelectorAll(`.${cls}`).forEach((element) => {
element.style.height = `${window.innerHeight * factor}px`;
});
});
// IE warning
if (window.document.documentMode && typeof bootscore !== 'undefined') {
const IEWarningDiv = document.createElement('div');
IEWarningDiv.className = 'position-fixed top-0 end-0 bottom-0 start-0 d-flex justify-content-center align-items-center';
IEWarningDiv.style.backgroundColor = 'white';
IEWarningDiv.style.zIndex = 1999;
IEWarningDiv.innerHTML = `
<div style="max-width: 90vw;">
<h1>${bootscore.ie_title}</h1>
<p class="lead">${bootscore.ie_limited_functionality}</p>
<p class="lead">${bootscore.ie_modern_browsers_1}${bootscore.ie_modern_browsers_2}${bootscore.ie_modern_browsers_3}${bootscore.ie_modern_browsers_4}${bootscore.ie_modern_browsers_5}</p>
</div>`;
document.body.appendChild(IEWarningDiv);
} How to test
If you want to change this, I'm in 👍 |
1/ just overlooked tbh |
The thing is that each snippet has a reason why it's there and has a job to do. If one of this is not working, it may create issues on existing installations. I recommend to setup a test environment instead doing changes on your live site. Agree that we can skip to test the IE warning, but all other snippets have to work properly. Comments are mandatory as well to help other contributors to understand what each snippet does. Added in #565 (comment) a working script for the self-closing offcanvas. Set this to draft again to avoid accidentally merging. If we can fix that, I'm in 👍 |
@androidacy-user FYI, IE warning has been removed #620. You can skip this snippet. |
jQuery quite frankly is a burdensome and weighty library in today's ES6 world. This file is one of the only files in bootscore that still depends on jQuery to function.