Fix image priority on PLP to improve lighthouse #429
Conversation
|
@bookernath is attempting to deploy a commit to the Makeswift Team on Vercel. A member of the Team first needs to authorize it. |
| @@ -69,6 +69,7 @@ export function ProductsList({ | |||
| key={product.id} | |||
| product={product} | |||
| showCompare={showCompare} | |||
| imagePriority={index === 0} | |||
There was a problem hiding this comment.
What is instead we made it so that ListProduct was CardProduct & { imagePriority?: boolean } and then from Catalyst we could pick exactly which images we'd like to prioritize?
There was a problem hiding this comment.
Yep I had similar thoughts about the layering... counterargument is that this change within the primitive helps lighthouse for anyone that uses these components.
I think your argument is overall better
There was a problem hiding this comment.
The primitive might be rendered below the fold, in which case we don't want the first image to have priority. Keep in mind that this primitive might be used in places other than the PLP
There was a problem hiding this comment.
So what do you think @bookernath? Are you down to try my suggestion or did you want to move forward with the changes you have right now?
There was a problem hiding this comment.
I prefer if we keep the primitive agnostic of this logic and just pass default from section or, like @migueloller suggested, optional config that can be passed from Catalyst.
There was a problem hiding this comment.
And just to be more explicit. My suggested changes would involve changing this line: https://github.com/makeswift/vibes/pull/429/files/a8b79ac5fcc35fa1f32eefc8892f7617782ae09d#diff-b96599c391d195af1a5bd0759ff7add174153291e25192fd05136b0505bf31d1R12
To this:
export type ListProduct = CardProduct & { imagePriority?: boolean };Then, from Catalyst we can pass the products as follows:
<ProductsListSection products={products.map((product, index) => ({ ...product, imagePriority: index <= 3 })} />
Mirroring bigcommerce/catalyst#1781
What/Why?
Improve lighthouse finding around above-the-fold image being loaded lazily; prioritizing just the first image is enough to eliminate the finding and maps to the mobile experience where you can only see one image. On desktop this might ideally be more like the first 3-4 images, but the finding on desktop goes away also so I'm fine to start with 1 image.
Testing
Before:
After (finding gone):