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

Changed lazyloading with highest fetchpriority in category header #587

Merged
merged 1 commit into from
Feb 9, 2024

Conversation

buggyzap
Copy link
Contributor

@buggyzap buggyzap commented Feb 5, 2024

Questions Answers
Description? I have changed the lazyloading with a more accurate property for this element, the fetchpriority=high.
Type? improvement
BC breaks? no
Deprecations? no
How to test? Open Chrome devtools in Performance tab and measure that image is loading first.

Common scenario

We have to load an header image, the LCP of the page depends on it, so we cannot load it in lazy loading mode but in fetchpriority=high, so in network order load of resources we get an highest priority and a fastest rendering.

Additional Resources

https://web.dev/articles/fetch-priority

Copy link
Contributor

@kpodemski kpodemski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fetchpriority is not available in Firefox, but I don't think it's a blocker, since the image is above the fold I think the change is legit

edit: I wonder if we could mix loading="eager" and this? 🤔

I'll ping other people to get some other opinions
cc @ga-devfront @tblivet @Hlavtox @SharakPL

@ga-devfront
Copy link
Collaborator

Interesting, does this really benefit all devices? Do we really feel a change in performance? as @kpodemski suggests I think we can mix that with loading="eager" . @tblivet an opinion?

Copy link
Contributor

@Hlavtox Hlavtox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tblivet
Copy link
Contributor

tblivet commented Feb 9, 2024

I also agree with adding loading eager 👍

@buggyzap
Copy link
Contributor Author

buggyzap commented Feb 9, 2024

https://stackoverflow.com/questions/77744344/is-it-okay-to-use-both-fetchpriority-high-and-loading-eager-in-img-tag

We can also add loading eager 👍

Hi guys, loading eager is the default value so its redundant to add it: https://www.w3schools.com/tags/att_img_loading.asp

We can just add fetchpriority.

@Hlavtox Hlavtox merged commit 00320b5 into PrestaShop:develop Feb 9, 2024
6 checks passed
@SharakPL SharakPL added this to the Beta milestone Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants