-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Add width and height attributes to cover images #519
Add width and height attributes to cover images #519
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This are of course my opinions and may not be those of the maintainer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the changes. I'm not sure what I think of the formatting changes, mostly because I'm not sure what the maintainer prefers, but this looks good. Also, there is an issue #499 you might want to reference.
Thank you for the hint! I inserted the link in my initial comment. |
Kudos, SonarCloud Quality Gate passed!
|
- Ensures that the browser can allocate the correct amount of space in the document while the image is loading. - Reduces CLS adityatelange#499
What does this PR change? What problem does it solve?
When using cover images the layout shifts on initial load because the image dimensions can only be determined by the browser after loading the images.
I followed the recommendations from this website linked to from Lighthouse in Chrome and added the width and height attributes to the cover images. From this the browser can determine the aspect ratio before loading any images.
Additionally, in the CSS file I have added the
height: auto
directive to the.entry-cover img
selector. The height is then calculated based on the actual available width.Was the change discussed in an issue or in the Discussions before?
Fix for #499 in the Discussions
PR Checklist