🐛 [Story performance] Disabling animations on initial page when first loaded #36668
Conversation
…l; branch 'main' of github.com:ampproject/amphtml into noanimation_firstpage
|
Hey @gmajoulet, @newmuis! These files were changed: |
|
I know this would make LCP better for documents using animations, but I think as a first step we should run this ONLY for transformed / optimized documents, as doing it for ALL documents would be a larger decision. |
| /** | ||
| * Whether to skip the animations on the page, due to reduced motion or first page. | ||
| * @return {boolean} | ||
| */ |
| skipAnimations_() { | ||
| return ( | ||
| prefersReducedMotion(this.ampdoc_.win) || | ||
| (isExperimentOn( |
There was a problem hiding this comment.
I don't think we're planning on launching this experiment, and we should not repurpose them it makes old data, dashboards, and documents very confusing.
| ) && | ||
| matches(this.page_, DISABLE_ANIMATIONS_FIRST_PAGE_SELECTOR)) | ||
| ); | ||
| } |
There was a problem hiding this comment.
This still disables animations on non transformed documents
| // desktop view. | ||
| switchPromise.then(() => { | ||
| this.pages_.forEach((page) => | ||
| removeAttributeInMutate(page, Attributes.VISITED) |
There was a problem hiding this comment.
replay_() is really trying to reset everything, including this, like a page reload. We should still remove the attributes
There was a problem hiding this comment.
In this PR I want to make the removal of animations more targeted:
- Current: first page of each story (independently of whether it was visited before or not).
- New: initial page of the story on the first visit of the page.
This means that when the story is replayed we don't need to skip the animations, especially because we're not on the initial page any more on some cases.
However, I understand your point so I'll re-implement this feature with better semantics (and probably remove this attribute altogether).
| `amp-story-page#${escapeCssSelectorIdent(pageId)}` | ||
| ); | ||
| page.setAttribute('active', ''); | ||
| page.setAttribute('i-amphtml-initial', ''); |
There was a problem hiding this comment.
This attribute is very broad and could mean literally anything, please be a bit more specific and/or consistent with how things are named in the JavaScript to make it easier
mszylkowski
changed the title
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. |
2 participants
On the experiment to
story-disable-animations-first-pageit would always prevent the first page from running the animations, but we only want to disable them if the initial page is being initially loaded (when you return to the first page, it should play the animations).This PR only disables the animations when you initially open the story, but when you navigate to the first page it should play the animation.
I'm adding an attribute
i-amphtml-initialfor the initial page, and usingi-amphtml-visitedto know if this is the first time that the page is viewed (we don't want to disable animations if you're coming back to the page).The attribute
i-amphtml-visitedused to guide what side on the screen an inactive page is (visited pages would be on the left). However, with the one panel layout, this is unnecessary so I removed that part of the logic, so that the attribute now informs whether the user has visited to the page (regardless of whether it's on the left or right).