-
Notifications
You must be signed in to change notification settings - Fork 890
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 #74
Comments
Hi Actually JQuery is used for the gallery page, the portfolio page and also to make the image in the post header to move as it is scrolled. It could be possible to add a condition but I don't think that's worth the change (The file is charged after a couple of microseconds here for 84.6 KB) while the picture is a couple MB. |
what about using CDN instead of loading our own copy, so that we can possibly use a cache from other site? |
Yeah, I had something like that in mind, a flag for everything to be loaded locally (every vendor scripts if enabled) and a flag for everything loaded from the CDN. That could be a good feature 👍 |
@sylhare yes it's small compared to the images (I'm having a look at those at the moment), but I wanted to see if I could remove it ;) I did not see the gallery dependency on jquery when I did it locally. |
@mathieubrun, true the masonry library seems not to need JQuery to work, however I used JQuery to initialize it. Probably by converting all of the JQuery to vanilla javascript you would kill the JQuery dependancy apart from the bootstrap portfolio. That could be an even better solution 🤔 wouldn't it? |
@sylhare yes this could be done. I had the snippet for the scrolling and the css. Regarding the portfolio, it looks like you're not using any bootstrap component that is using JS. The bootstrap js could be removed then. Also, you could switch to flexbox layout ( https://caniuse.com/#feat=flexbox ) and ditch bootstrap as well ? |
Yup, I was thinking toward making that move. So here are the tasks that I see to remove JQuery (I'll create another for the bootstrap to felxbox):
If you want to help on some, let me know. 😉It is greatly appreciated! |
Done on my fork, just need to do some checks with IE ! |
Amazing! I will review the PR once you are ready 😁 |
Removed jquery dependency from navbar #74
JQuery is now removed from the theme 🎉 |
Add Adam Morris as a JavaScript contributing maintainer
Removed jquery dependency from navbar sylhare#74
Removed jquery dependency from navbar sylhare#74
FIrst, thanks for the great theme !
JQuery does not seem to be required, removing it may shave a few KB and improve page load speed :)
I had it working on my site here : https://github.com/mathieubrun/mathieubrun.github.io
If you want I can fork and make a PR ?
The text was updated successfully, but these errors were encountered: