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

Lazy load images with the light lib bLazy.js instead of jQuery #132

Merged
merged 1 commit into from
Mar 4, 2015

Conversation

ArthurHoaro
Copy link
Member

  • Remove jquery.lazyload lib
  • Add blazy lib
  • Add a bit of CSS animation
  • Delete unused picwall2 template

@nodiscc
Copy link
Member

nodiscc commented Mar 1, 2015

👍

@nodiscc nodiscc added the cleanup code cleanup and refactoring label Mar 1, 2015
@e2jk
Copy link

e2jk commented Mar 1, 2015

  • Please include the non-minified version of the library. We want to have the source included, not just the "compiled/binary" form.
  • Please update the COPYING file to:
    • mention the copyright of this library. (According to https://github.com/dinbror/blazy/ it's MIT licensed)
    • remove mention of jquery-lazyload
    • (tip: as both libraries are similarly licensed, you can just update that line to refer to the new library instead)

Not blocking for this Pull Request, but please in the future submit separate PRs for separate changes (in this example, 3 PRs would have been nice: libraries, CSS, picwall2.html). This makes merging and discussing easier, for example if the library switch would have been controversial (which I don't think it is), you can at least already have the 2 other changes merged while the discussion on the 3rd continues.

  * Remove jquery.lazyload lib
  * Add blazy lib
  * Add a bit of CSS animation
  * Delete unused picwall2 template
@ArthurHoaro
Copy link
Member Author

It's done.

About separate commits, I understand you're point, but picwall2.html wouldn't work without the lib, and the CSS wouldn't apply without the template. I'm not sure it would be really pertinent in this particular PR.

@pikzen
Copy link

pikzen commented Mar 2, 2015

Note: this actually nukes all of jQuery and removes this dependency from Shaarli.
This also means that we need to remove the "Disable jQuery" option if this gets merged, or maybe replace it with "Disable JS"? This would break half of Shaarli, and we probably need to take a look at how it is implemented.

Otherwise this PR is fine.

@ArthurHoaro
Copy link
Member Author

No it doesn't. I just removed lazy loading jQuery plug-in.

@nodiscc
Copy link
Member

nodiscc commented Mar 2, 2015

@pikzen jQuery is still used for tag autocompletion/suggestion. We started discussion about removing it/using another autocompletion system in #49

@nodiscc
Copy link
Member

nodiscc commented Mar 2, 2015

Tested and works perfectly, thanks. I'll merge it tomorrow if nobody disagrees.

nodiscc added a commit that referenced this pull request Mar 4, 2015
Lazy load images with the light lib bLazy.js instead of jQuery
@nodiscc nodiscc merged commit 0e5400e into shaarli:master Mar 4, 2015
@ArthurHoaro ArthurHoaro deleted the picwall branch March 5, 2015 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup code cleanup and refactoring please test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants