-
Notifications
You must be signed in to change notification settings - Fork 30
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
Fix images in cards for old articles in Onwards Content AB test #13006
Conversation
@@ -25,6 +26,8 @@ const containerStyles = css` | |||
|
|||
${from.leftCol} { | |||
flex-direction: row; | |||
} | |||
${from.wide} { |
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.
To align cards with the other containers at leftCol
screen sizes
@@ -126,7 +133,7 @@ export const BigSixOnwardsContent = ({ url, discussionApiUrl }: Props) => { | |||
imagePositionOnDesktop="right" | |||
imagePositionOnMobile="top" | |||
imageSize="large" | |||
imageLoading="lazy" | |||
imageLoading="eager" |
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.
We only load this component when the user has scrolled to it, so we should load eagerly. Mirrors functionality in Carousel component where the four non-hidden cards images are eagerly loaded.
Size Change: +18 B (0%) Total Size: 942 kB ℹ️ View Unchanged
|
749bb68
to
dcdc75a
Compare
? trail.masterImage | ||
: trail.image | ||
? getSourceImageUrl(trail.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.
Nested ternary 🤢. If the AB test is successful, then it will be worth extracting this to a function (and not using the deprecated getSourceImageUrl
function).
Seen on PROD (merged by @domlander 8 minutes and 51 seconds ago) Please check your changes! |
What does this change?
Show images on cards containing old articles
Why?
Images did not display on some old articles
Screenshots