-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
🚀 [Story performance] Disable animations in first page under experiment #35356
🚀 [Story performance] Disable animations in first page under experiment #35356
Conversation
Hey @gmajoulet, @newmuis! These files were changed:
|
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.
Please report the experiment to the CSI pipeline otherwise we can't collect data
@@ -465,6 +465,11 @@ export class AmpStory extends AMP.BaseElement { | |||
'story-load-first-page-only' | |||
); | |||
} | |||
if (isExperimentOn(this.win, 'story-disable-animations-first-page')) { |
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.
Some thoughts on this:
- The experiment needs to be enabled for CSI tracking BEFORE the LCP metric is sent. Is it the case here?
- There are more cases where the animations are disabled (prefers-reduced-motion), I think it'd be interesting to capture this as well
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.
Good Qs
For 1: This is added on the buildCallback
of a story (which happens before layoutStory_()
), so it should happen before the LCP metric is sent.
For 2: We can trigger the CSI flag if prefers-reduced-motion
is enabled, since that will also "disable animations on the first page". I don't think it'd be accurate if some of the users that have the reduced motion activated count towards this flag being OFF.
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.
have you tested (1) or is it working thanks to the render service?
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.
Tested it, it works
There's multiple ways LCP can get affected by animations on the first page:
There could potentially be more causes for LCP problems through animations, which is why we'll disable the animations on the first page. Launching the experiment for
5%