fix(dialog): Cancel open's rAF when close is called (#4087)

material-components · Nov 15, 2018 · 2516c25 · 2516c25
1 parent 2ae6335
commit 2516c25
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 8 deletions.
diff --git a/packages/mdc-dialog/foundation.js b/packages/mdc-dialog/foundation.js
@@ -102,10 +102,6 @@ class MDCDialogFoundation extends MDCFoundation {
       this.close(strings.DESTROY_ACTION);
     }
 
-    if (this.animationFrame_) {
-      cancelAnimationFrame(this.animationFrame_);
-    }
-
     if (this.animationTimer_) {
       clearTimeout(this.animationTimer_);
       this.handleAnimationTimerEnd_();
@@ -153,6 +149,9 @@ class MDCDialogFoundation extends MDCFoundation {
     this.adapter_.removeClass(cssClasses.OPEN);
     this.adapter_.removeBodyClass(cssClasses.SCROLL_LOCK);
 
+    cancelAnimationFrame(this.animationFrame_);
+    this.animationFrame_ = 0;
+
     clearTimeout(this.animationTimer_);
     this.animationTimer_ = setTimeout(() => {
       this.handleAnimationTimerEnd_();

diff --git a/test/screenshot/spec/mdc-dialog/fixture.js b/test/screenshot/spec/mdc-dialog/fixture.js
@@ -25,6 +25,10 @@ import {strings} from '../../../../packages/mdc-dialog/constants';
 
 window.mdc.testFixture.fontsLoaded.then(() => {
   class DialogFixture {
+    get dialog() {
+      return this.dialogInstance_;
+    }
+
     /** @param {!HTMLElement} dialogEl */
     initialize(dialogEl) {
       /**
@@ -390,8 +394,9 @@ window.mdc.testFixture.fontsLoaded.then(() => {
     }
   }
 
-  [].forEach.call(document.querySelectorAll('.mdc-dialog'), (dialogEl) => {
+  mdc.testFixture.dialogs = [].map.call(document.querySelectorAll('.mdc-dialog'), (dialogEl) => {
     const dialogFixture = new DialogFixture();
     dialogFixture.initialize(dialogEl);
+    return dialogFixture.dialog;
   });
 });
diff --git a/test/unit/mdc-dialog/foundation.test.js b/test/unit/mdc-dialog/foundation.test.js
@@ -117,14 +117,17 @@ test('#destroy cancels layout handling if called on same frame as layout', () =>
   td.verify(mockAdapter.isContentScrollable(), {times: 0});
 });
 
-test('#open adds CSS classes', () => {
+test('#open adds CSS classes after rAF', () => {
   const {foundation, mockAdapter} = setupTest();
   const clock = installClock();
 
   foundation.open();
-  clock.runToFrame();
-  clock.tick(100);
+  td.verify(mockAdapter.addClass(cssClasses.OPEN), {times: 0});
+  td.verify(mockAdapter.addBodyClass(cssClasses.SCROLL_LOCK), {times: 0});
 
+  // Note: #open uses a combination of rAF and setTimeout due to Firefox behavior, so we need to wait 2 ticks
+  clock.runToFrame();
+  clock.runToFrame();
   td.verify(mockAdapter.addClass(cssClasses.OPEN));
   td.verify(mockAdapter.addBodyClass(cssClasses.SCROLL_LOCK));
 });
@@ -140,6 +143,20 @@ test('#close removes CSS classes', () => {
   td.verify(mockAdapter.removeBodyClass(cssClasses.SCROLL_LOCK));
 });
 
+test('#close cancels rAF scheduled by open if still pending', () => {
+  const {foundation, mockAdapter} = setupTest();
+  const clock = installClock();
+
+  foundation.open();
+  td.reset();
+  foundation.close();
+
+  // Note: #open uses a combination of rAF and setTimeout due to Firefox behavior, so we need to wait 2 ticks
+  clock.runToFrame();
+  clock.runToFrame();
+  td.verify(mockAdapter.addClass(cssClasses.OPEN), {times: 0});
+});
+
 test('#open adds the opening class to start an animation, and removes it after the animation is done', () => {
   const {foundation, mockAdapter} = setupTest();
   const clock = installClock();