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

POC of removing jquery #27

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Artiavis
Copy link

Hey, thanks for creating this plugin! It's proven really helpful (still massaging my installation but getting very close)!

I noticed you're using jQuery, and figured I'd send you a POC where it can be removed (although I may have missed 1-2 things, not promising this is perfect). Photoswipe already assumes IE8+; it's only a short jump to not having it. Could be worth considering for those who don't want/need jQuery and don't currently have it.

@liwenyip
Copy link
Owner

Hi @Artiavis,

Thanks for your contribution.

I do understand your point about jQuery not really being necessary, but I haven't coded anything for 2 years, and I don't understand my own code anymore, so I have to take the approach of "if it isn't broken, don't fix it".

You make a good point though :-)

Instead of changing static/js/load-photoswipe.js, could you create a new file called static/js/load-photoswipe-nojquery.js and put comments in the other relevant files to explain how to use the nojquery version if you want to?

Regards,
Li-Wen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants