Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Ensure that first content image is not lazy-loaded in block themes either #3560
Ensure that first content image is not lazy-loaded in block themes either #3560
Changes from 15 commits
2be5c02
0f2b239
6f61ab5
af3dc5e
6fe3831
6230509
9597731
9ab8c44
ed2b3ef
061565a
bf56e30
b5ba312
36fb703
f4ccd96
eaa4d61
30b9443
bdb0121
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Query: Should this have "first_featured_image", or just "featured_image"?
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.
"first_featured_image" is correct, as other featured images further down in the page should still be lazy-loaded.
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.
In addition to testing:
Should we also cover this?
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.
@costdev
Hmm, this is already being tested here I think. The final assertion tests that the featured image is not lazy-loaded, while the first image afterwards (an in-content image) is lazy-loaded.
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.
Should we also add another test to ensure that images in a footer template part are always lazy loaded? I'm thinking in case something is later changed that accidentally excludes footer images from lazy loading.
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.
@costdev This is already being tested by this method, the final assertion checks the entire template which includes both a header and footer template part, each with an image. Only the one in the header template part is expected to not be lazy-loaded, while the one in the footer should be.