馃殌 [Story performance] Disable animations in first page under experiment #35356
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hey @gmajoulet, @newmuis! These files were changed: |
gmajoulet
reviewed
Jul 22, 2021
mszylkowski
changed the title
gmajoulet
reviewed
Jul 22, 2021
| @@ -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.
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.
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.
have you tested (1) or is it working thanks to the render service?
There was a problem hiding this comment.
Tested it, it works
gmajoulet
approved these changes
Jul 22, 2021
newmuis
approved these changes
Jul 27, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
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%