🐛 [Story a11y] Reduced motion should apply the animation's last frame…

… to the elements (#34466) * Created e2e test * Added tests * Fixed e2e tests * Removed comment * Removed unused import * Addede e2es * Moved field to html * More work on e2e * Added load unload fixture * Fixed tests * Added more tests and cleaned up hooks * Added multiple sources on first videos to test flexible-bitrate * Changed e2es to work * Removed whitespaces * Adding cache fetch test * Add e2e fixture to disallowlist for validate * fixed host_url in e2e * Updated tests * Changed order of test conditions for consistency * Update extensions/amp-video/0.1/test-e2e/test-amp-video-flexible-bitrate.js * Update extensions/amp-video/0.1/test-e2e/test-amp-video-flexible-bitrate.js * Fixed reduced motion animation * Fixed reduced motion * Removed unnecessary ? * Reverted example html * Added anim to bot rendering * Remove false test * Fixed linting * Fix visual diff * Fixed paused devAssert on touch pause * Fail gracefully * StartOrFinishAnimation * Fixed animation now working * Removed console logs * Added maybePause or maybeResume * Using maybeInit * Fixed bot rendering visstest * Fixed a pause test and removed unused method * Removed finishOrPause * Merge pt2 * Fixed animation.js * Removed unused property prefersReducedMotion * Fixed error when switching user prefs * Fixed animation tests * Added delay to bot rendering so the animations are applied * removed unused maybe and isinitialized * Simplify code * Simplify code * Simplify a bit more * Added timer to visual test * Removed timer Co-authored-by: Gabriel Majoulet <gmajoulet@google.com>
ampproject · Jul 2, 2021 · 9520909 · 9520909
1 parent a9150b7
commit 9520909
Show file tree

Hide file tree

Showing 9 changed files with 140 additions and 59 deletions.
diff --git a/examples/amp-story/amp-story-animation.html b/examples/amp-story/amp-story-animation.html
@@ -15,6 +15,16 @@
       custom-element="amp-story"
       src="https://cdn.ampproject.org/v0/amp-story-1.0.js"
     ></script>
+    <script
+      async
+      custom-element="amp-video"
+      src="https://cdn.ampproject.org/v0/amp-video-0.1.js"
+    ></script>
+    <script
+      async
+      custom-element="amp-animation"
+      src="https://cdn.ampproject.org/v0/amp-animation-0.1.js"
+    ></script>
     <style amp-custom>
       body {
         font-family: sans-serif;
@@ -117,6 +127,39 @@
           </script>
         </amp-story-animation>
       </amp-story-page>
+
+      <amp-story-page id="hidden-animations">
+        <amp-story-animation layout="nodisplay" trigger="visibility">
+          <script type="application/json">
+            [
+              {
+                "selector": ".anim",
+                "keyframes": {
+                  "transform": ["translate3d(-100%, 0px, 0)", "translate3d(0px, 0px, 0)"]
+                },
+                "delay": 100,
+                "direction": "normal",
+                "duration": 1000,
+                "easing": "cubic-bezier(0.19, 1, 0.22, 1)",
+                "fill": "both"
+              }
+            ]
+          </script>
+        </amp-story-animation>
+        <amp-story-grid-layer template="vertical">
+          <amp-video autoplay="autoplay" layout="fill" loop="loop">
+            <source type="video/mp4" src="./video/stamp.mp4">
+          </amp-video>
+        </amp-story-grid-layer>
+        <amp-story-grid-layer template="vertical" aspect-ratio="412:618">
+          <div class="anim" style="width:80%;height:20%;position:absolute;font-size:30px;background:white;left:10%;top:20%;transform:translate3d(-100%, 0px, 0)">
+            This is the animated text with initial frame in CSS
+          </div>
+          <div class="anim" style="width:80%;height:20%;position:absolute;font-size:30px;background:white;left:10%;top:60%">
+            This is the animated text with final frame in CSS
+          </div>
+        </amp-story-grid-layer>
+      </amp-story-page>
     </amp-story>
   </body>
 </html>
diff --git a/examples/visual-tests/amp-story/amp-story-bot-rendering.html b/examples/visual-tests/amp-story/amp-story-bot-rendering.html
@@ -73,11 +73,26 @@
       </amp-story-page>
 
       <amp-story-page id="page-link">
+        <amp-story-animation layout="nodisplay" trigger="visibility">
+          <script type="application/json">
+            [
+              {
+                "selector": ".anim",
+                "keyframes": {
+                  "opacity": ["0", "1"]
+                },
+                "direction": "normal",
+                "fill": "both",
+                "duration": 1
+              }
+            ]
+          </script>
+        </amp-story-animation>
         <amp-story-grid-layer template="fill">
           <amp-img layout="fill" src="./img/cat2.jpg"></amp-img>
         </amp-story-grid-layer>
         <amp-story-grid-layer template="vertical">
-          <p>
+          <p class="anim">
             <span data-text-background-color="crimson">i can fit in that box but inspect anything brought into the house wack the mini furry mouse intently stare at the same spot. Flee in terror at cucumber discovered on floor shake treat bag</span>
           </p>
         </amp-story-grid-layer>

diff --git a/extensions/amp-animation/0.1/runners/animation-runner.js b/extensions/amp-animation/0.1/runners/animation-runner.js
@@ -76,8 +76,9 @@ export class AnimationRunner {
   seekToPercent(unusedPercent) {}
 
   /**
+   * @param {bool} unusedPauseOnError
    */
-  finish() {}
+  finish(unusedPauseOnError = false) {}
 
   /**
    */

diff --git a/extensions/amp-animation/0.1/runners/native-web-animation-runner.js b/extensions/amp-animation/0.1/runners/native-web-animation-runner.js
@@ -173,7 +173,9 @@ export class NativeWebAnimationRunner extends AnimationRunner {
    * @param {time} time
    */
   seekTo(time) {
-    devAssert(this.players_);
+    if (!this.players_) {
+      return;
+    }
     this.setPlayState_(WebAnimationPlayState.PAUSED);
     this.players_.forEach((player) => {
       player.pause();
@@ -197,15 +199,24 @@ export class NativeWebAnimationRunner extends AnimationRunner {
   /**
    * @override
    */
-  finish() {
+  finish(pauseOnError = false) {
     if (!this.players_) {
       return;
     }
     const players = this.players_;
     this.players_ = null;
     this.setPlayState_(WebAnimationPlayState.FINISHED);
     players.forEach((player) => {
-      player.finish();
+      if (pauseOnError) {
+        try {
+          // Will fail if animation is infinite, in that case we pause it.
+          player.finish();
+        } catch (error) {
+          player.pause();
+        }
+      } else {
+        player.finish();
+      }
     });
   }
 

diff --git a/extensions/amp-story/1.0/amp-story-page.js b/extensions/amp-story/1.0/amp-story-page.js
@@ -75,7 +75,6 @@ import {isPageAttachmentUiV2ExperimentOn} from './amp-story-page-attachment-ui-v
 import {isPrerenderActivePage} from './prerender-active-page';
 import {listen, listenOnce} from '../../../src/event-helper';
 import {CSS as pageAttachmentCSS} from '../../../build/amp-story-open-page-attachment-0.1.css';
-import {prefersReducedMotion} from '#core/dom/media-query-props';
 import {propagateAttributes} from '#core/dom/propagate-attributes';
 import {px, toggle} from '#core/dom/style';
 import {renderPageAttachmentUI} from './amp-story-open-page-attachment';
@@ -318,9 +317,6 @@ export class AmpStoryPage extends AMP.BaseElement {
     if (this.animationManager_) {
       return;
     }
-    if (prefersReducedMotion(this.win)) {
-      return;
-    }
     if (!hasAnimations(this.element)) {
       return;
     }
@@ -725,7 +721,7 @@ export class AmpStoryPage extends AMP.BaseElement {
 
   /** @return {!Promise} */
   beforeVisible() {
-    return this.maybeApplyFirstAnimationFrame();
+    return this.maybeApplyFirstAnimationFrameOrFinish();
   }
 
   /**
@@ -1260,7 +1256,10 @@ export class AmpStoryPage extends AMP.BaseElement {
    * @private
    */
   maybeStartAnimations_() {
-    this.animationManager_?.animateIn();
+    if (!this.animationManager_) {
+      return;
+    }
+    this.animationManager_.animateIn();
   }
 
   /**
@@ -1274,17 +1273,14 @@ export class AmpStoryPage extends AMP.BaseElement {
     }
     this.signals()
       .whenSignal(CommonSignals.LOAD_END)
-      .then(() => this.maybeApplyFirstAnimationFrame())
-      .then(() => {
-        this.animationManager_.finishAll();
-      });
+      .then(() => this.animationManager_.applyLastFrame());
   }
 
   /**
    * @return {!Promise}
    */
-  maybeApplyFirstAnimationFrame() {
-    return Promise.resolve(this.animationManager_?.applyFirstFrame());
+  maybeApplyFirstAnimationFrameOrFinish() {
+    return Promise.resolve(this.animationManager_?.applyFirstFrameOrFinish());
   }
 
   /**

diff --git a/extensions/amp-story/1.0/animation.js b/extensions/amp-story/1.0/animation.js
@@ -37,6 +37,7 @@ import {dev, devAssert, user, userAssert} from '../../../src/log';
 import {escapeCssSelectorIdent} from '#core/dom/css-selectors';
 import {getChildJsonConfig} from '../../../src/json';
 import {map, omit} from '#core/types/object';
+import {prefersReducedMotion} from '#core/dom/media-query-props';
 import {scopedQuerySelector, scopedQuerySelectorAll} from '#core/dom/query';
 import {setStyles} from '#core/dom/style';
 import {timeStrToMillis, unscaledClientRect} from './utils';
@@ -326,6 +327,20 @@ export class AnimationRunner {
     });
   }
 
+  /**
+   * Applies the last animation frame.
+   * @return {!Promise<void>}
+   */
+  applyLastFrame() {
+    if (this.presetTarget_) {
+      return Promise.resolve();
+    }
+    this.runnerPromise_.then((runner) => {
+      runner.init();
+      runner.finish(/* pauseOnError */ true);
+    });
+  }
+
   /** Starts or resumes the animation. */
   start() {
     if (this.hasStarted()) {
@@ -380,12 +395,7 @@ export class AnimationRunner {
     }
 
     if (this.runner_) {
-      try {
-        this.runner_.pause();
-      } catch (e) {
-        // This fails when the animation is finished explicitly
-        // (runner.finish()) since this destroys internal players. This is fine.
-      }
+      this.runner_.pause();
     }
   }
 
@@ -398,7 +408,7 @@ export class AnimationRunner {
     }
 
     if (this.runner_) {
-      devAssert(this.runner_).resume();
+      this.runner_.resume();
     }
   }
 
@@ -539,6 +549,9 @@ export class AnimationManager {
     /** @private @const */
     this.builderPromise_ = this.createAnimationBuilderPromise_();
 
+    /** @private @const {bool} */
+    this.prefersReducedMotion_ = prefersReducedMotion(ampdoc.win);
+
     /** @private {?Array<!AnimationRunner>} */
     this.runners_ = null;
 
@@ -561,14 +574,31 @@ export class AnimationManager {
    * Applies first frame to target element before starting animation.
    * @return {!Promise}
    */
-  applyFirstFrame() {
+  applyFirstFrameOrFinish() {
     return Promise.all(
-      this.getOrCreateRunners_().map((runner) => runner.applyFirstFrame())
+      this.getOrCreateRunners_().map((runner) =>
+        this.prefersReducedMotion_
+          ? runner.applyLastFrame()
+          : runner.applyFirstFrame()
+      )
+    );
+  }
+
+  /**
+   * Applies last frame to target element before starting animation.
+   * @return {!Promise}
+   */
+  applyLastFrame() {
+    return Promise.all(
+      this.getOrCreateRunners_().map((runner) => runner.applyLastFrame())
     );
   }
 
   /** Starts all entrance animations for the page. */
   animateIn() {
+    if (this.prefersReducedMotion_) {
+      return;
+    }
     this.getRunners_().forEach((runner) => runner.start());
   }
 
@@ -588,15 +618,15 @@ export class AnimationManager {
 
   /** Pauses all animations in the page. */
   pauseAll() {
-    if (!this.runners_) {
+    if (!this.runners_ || this.prefersReducedMotion_) {
       return;
     }
     this.getRunners_().forEach((runner) => runner.pause());
   }
 
   /** Resumes all animations in the page. */
   resumeAll() {
-    if (!this.runners_) {
+    if (!this.runners_ || this.prefersReducedMotion_) {
       return;
     }
     this.getRunners_().forEach((runner) => runner.resume());
@@ -615,7 +645,7 @@ export class AnimationManager {
    * @private
    */
   getRunners_() {
-    return devAssert(this.runners_, 'Executed before applyFirstFrame');
+    return devAssert(this.runners_, 'Executed before applyFirstFrameOrFinish');
   }
 
   /**

diff --git a/extensions/amp-story/1.0/test/test-amp-story-page.js b/extensions/amp-story/1.0/test/test-amp-story-page.js
@@ -14,7 +14,6 @@
  * limitations under the License.
  */
 
-import * as MediaQueryProps from '#core/dom/media-query-props';
 import * as VideoUtils from '#core/dom/video';
 import {Action, AmpStoryStoreService} from '../amp-story-store-service';
 import {AmpAudio} from '../../../amp-audio/0.1/amp-audio';
@@ -118,18 +117,6 @@ describes.realWin('amp-story-page', {amp: {extensions}}, (env) => {
     expect(page.animationManager_).to.exist;
   });
 
-  it('should not build the animation manager if `prefers-reduced-motion` is on', async () => {
-    env.sandbox.stub(MediaQueryProps, 'prefersReducedMotion').returns(true);
-
-    const animatedEl = html`<div animate-in="fade-in"></div>`;
-
-    element.appendChild(animatedEl);
-    element.getAmpDoc = () => new AmpDocSingle(win);
-
-    page.buildCallback();
-    expect(page.animationManager_).to.be.null;
-  });
-
   it('should set an active attribute when state becomes active', async () => {
     page.buildCallback();
     await page.layoutCallback();