feat: added lazyloading for images #775
Conversation
Master > Develop
Codecov Report
@@ Coverage Diff @@
## develop #775 +/- ##
===========================================
Coverage 8.49421% 8.49421%
===========================================
Files 22 22
Lines 259 259
Branches 62 62
===========================================
Hits 22 22
Misses 237 237 Continue to review full report at Codecov.
|
auto-assign
bot
requested review from
divyanshu-rawat,
gittysachin,
Marvin9 and
Paarmita
|
@divyanshu-rawat pls review |
public/index.html
Outdated
| @@ -44,8 +44,9 @@ | |||
| To begin the development, run `npm start` or `yarn start`. | |||
| To create a production bundle, use `npm run build` or `yarn build`. | |||
| --> | |||
| <script src="https://cdnjs.cloudflare.com/ajax/libs/lazysizes/5.2.0/lazysizes.min.js" async></script> | |||
There was a problem hiding this comment.
We can't afford lazy-loading on the cost of lazysizes.min.js.
There was a problem hiding this comment.
Another tip is to add it using NPM https://www.npmjs.com/package/lazysizes instead of CDN in public directory.
There was a problem hiding this comment.
Have you read what is written in #599 you are still making use of package I am mentioning again We can't afford lazy-loading on the cost of lazysizes.min.js.
The tip was just for your information that you should always install a library via NPM. i didn't meant that if you install the same library using NPM then we will merge it.
If you might have read carefully what is written in #599 that you might have already understood what I was trying to convey.
In summary #599 also adds the package through NPM see her https://github.com/Ignitus/Ignitus-client/pull/599/files#diff-b9cfc7f2cdf78a7f4b91a753d10865a2R32, do your PR is not making any difference we closed #599 because of cost of additional package to lazy load images, and the cost of adding an NPM package is it also increase the bundle size so it doesn't provide us much value, so we prefer to load images from GCP instead.
I am closing the PR now, as we already use GCP , and please ask before taking up the issue.
There was a problem hiding this comment.
@divyanshu-rawat seems like I took the wrong idea, sorry for any inconveniences caused. I thought you were asking to add lazymin via npm and not cdn. Duh, a bad misunderstanding.
Fixes part of issue: #584