weixin_39551611
weixin_39551611
2020-12-28 14:24

lazy load images using lazyload-rails gem

Fixes #7919

Lazy loaded images by using lazyload-rails gem. Converted the img tags into image_tag helpers for the gem to function properly.

  • [x] PR is descriptively titled 📑 and links the original issue above 🔗
  • [x] tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
  • [x] code is in uniquely-named feature branch and has no merge conflicts 📁
  • [x] screenshots/GIFs are attached 📎 in case of UI updation
  • [x] ask /reviewers for help, in a comment below

GIFs

lazy1 lazy2 lazy3 lazy4 lazy5

Thanks!

该提问来源于开源项目:publiclab/plots2

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享
  • 邀请回答

8条回答

  • weixin_39754142 weixin_39754142 3月前

    Reading y'all vet this just made me so happpppy! Good work. Excellent, nuanced points <3

    点赞 评论 复制链接分享
  • weixin_39913422 weixin_39913422 3月前

    Hey, i just want to say the discussion and priorities of maintainability and the vetting and everything that happened in this PR is AWESOME 🙌🙌🙌🙌 ❤️❤️❤️

    点赞 评论 复制链接分享
  • weixin_39551611 weixin_39551611 3月前

    Hey I have added some more routes now can you please review this? :sweat_smile:

    点赞 评论 复制链接分享
  • weixin_39865277 weixin_39865277 3月前

    cool :+1:

    点赞 评论 复制链接分享
  • weixin_39551611 weixin_39551611 3月前

    Hey I totally understand your concerns over this :sweat_smile: and I actually did check it out against the vetting points by so the thing is that although this gem is not updated as frequently, the current implementation is something that won't need updates in the sense it is sufficient in itself. It is also the only implementation for lazy-loading in rails applications. And after doing a lot of searches, I could only find this tool being mentioned in blogs to speed up rails apps. The documentation is sufficient, there aren't any rollbacks in the commits as such and the license is MIT. Since, it is the only option available we might just have to go with it

    点赞 评论 复制链接分享
  • weixin_39865277 weixin_39865277 3月前

    great job finding the gem. I have a few concerns with it though...its seems that its not frequently updated(last updated 14 months ago), it is crucial we look at this as it could hinder our future updates..also guidelines on vetting libraries https://github.com/publiclab/plots2/issues/8019 ...I am not discarding it just giving you smth to think about/ consider . Thanks

    点赞 评论 复制链接分享
  • weixin_39551611 weixin_39551611 3月前

    just checking in to ask if for the placeholder should we have like an animated spinner my only concern is that the size of it should be small. Or we can leave it at the gray placeholder as well :sweat_smile: Thought? :v:

    点赞 评论 复制链接分享
  • weixin_39945679 weixin_39945679 3月前

    Codecov Report

    Merging #8043 into master will increase coverage by 0.15%. The diff coverage is n/a.

    Impacted file tree graph

    diff
    @@            Coverage Diff             @@
    ##           master    #8043      +/-   ##
    ==========================================
    + Coverage   82.34%   82.49%   +0.15%     
    ==========================================
      Files          99       99              
      Lines        5737     5737              
    ==========================================
    + Hits         4724     4733       +9     
    + Misses       1013     1004       -9     
    

    | Impacted Files | Coverage Δ | | |---|---|---| | app/controllers/admin_controller.rb | 81.85% <0.00%> (+3.79%) | :arrow_up: |

    点赞 评论 复制链接分享

为你推荐