Add Warning in Docs about Lazy Loading and LCP#128
Conversation
|
@totesforlife is attempting to deploy a commit to the Cloudinary DevX Team on Vercel. A member of the Team first needs to authorize it. |
|
@eportis-cloudinary anything you want changed? |
eportis-cloudinary
left a comment
There was a problem hiding this comment.
@sccalabr - thank you so much for taking the time to write this thoughtful guide and link to it from the appropriate places in the docs - this is great!
My biggest comment is that I don't think we should document fetchpriority and loading as props, because (as you mention) they are simply passed to the img element. Starting to document everything you could do with props that are "only" img attributes seems like a slippery slope, that I don't want to start going down. Their behavior is already well-documented elsewhere (like MDN).
So, I'm asking you to remove that section and table from the configuration page, and to remove a couple of other things that seemed "extra". Other than that, just minor niggles. The guide is really well written and the examples are great.
| [Learn more about quality](https://cloudinary.com/documentation/transformation_reference#q_quality) on the Cloudinary docs. No newline at end of file | ||
| [Learn more about quality](https://cloudinary.com/documentation/transformation_reference#q_quality) on the Cloudinary docs. | ||
|
|
||
| ## Performance & Loading |
There was a problem hiding this comment.
See my note about removing this section and table. I don't think we should add documentation for "props" that are just being passed through to the img as attributes, without doing anything at the component level.
|
|
||
| The basic props required to use CldImage include: | ||
|
|
||
| :::tip |
There was a problem hiding this comment.
Adding loading and fetchpriority to the list of props feels like a slippery slope that would end with adding all of the img attributes to these docs.
This tip is important and well placed, but can you link to the LCP and performance guide page, and remove the additions to the prop table?
| width="<Width>" | ||
| height="<Height>" | ||
| alt="<Description>" | ||
| fetchpriority="high" <!-- This hurts performance! --> |
There was a problem hiding this comment.
You can't put an HTML comment inside an HTML element, unfortunately. Can you remove this and all instances of that in the examples?
| ``` | ||
| </CodeBlock> | ||
|
|
||
| ### 4. Use Cloudinary Optimizations |
There was a problem hiding this comment.
Because we add f_auto and q_auto to URLs by default, I don't think we need this example, and it could actually confuse people who think they need to add the format and quality props, separately. Can you remove this section?
| - [Measure LCP in JavaScript - web.dev](https://web.dev/articles/lcp#measure-lcp-in-javascript) | ||
| - [Core Web Vitals - web.dev](https://web.dev/articles/vitals) | ||
|
|
||
| ## Comparison with Next.js |
There was a problem hiding this comment.
I know the comparison with NextJS is what led Colby to file the issue, but I think many folks will be coming to the Astro library (and docs) without NextJS experience. That makes this section superfluous for them. Can you remove it?
|
I have some additional tweaks I'd like to make but will merge this and edit from there. @sccalabr are you also @totesforlife? Should I credit both @sccalabr and @totesforlife for this PR? Thanks again for the contribution! |
|
@eportis-cloudinary is the hacktoberfest form sent out at the end of the month |
|
@sccalabr Can you please provide your email so I can send you the swag form? Thank you |
|
@allcontributors please add @sccalabr for docs |
|
I've put up a pull request to add @sccalabr! 🎉 |
|
@sccalabr Thank you for your contribution to Hacktoberfest! As a token of our thanks, we at Cloudinary have emailed you with instructions on how to claim your bespoke 2025 swag pack |
Description
Add Warning in Docs about Lazy Loading and LCP
Issue Ticket Number
Fixes #9
Type of change
Checklist