Merge pull request #543 from muzafferdede/fix-x-show-transition-overlap

Fix x show transition overlap

dist/alpine-ie11.js

@@ -5616,6 +5616,7 @@
     var _this6 = this;
 
     var forceSkip = arguments.length > 3 && arguments[3] !== undefined ? arguments[3] : false;
+    // We don't want to transition on the initial page load.
     if (forceSkip) return hide();
     var attrs = getXAttrs(el, component, 'transition');
     var showAttr = getXAttrs(el, component, 'show')[0];
@@ -5921,8 +5922,9 @@
 
         _newArrowCheck(this, _this14);
 
-        stages.end();
-        setTimeout(function () {
+        stages.end(); // Assign current transition to el in case we need to force it
+
+        el.__x_transition_remaining = once(function () {
           _newArrowCheck(this, _this15);
 
           stages.hide(); // Adding an "isConnected" check, in case the callback
@@ -5930,14 +5932,31 @@
 
           if (el.isConnected) {
             stages.cleanup();
-          }
-        }.bind(this), duration);
+          } // Safe to remove transition from el since it is completed
+
+
+          delete el.__x_transition_remaining;
+        }.bind(this));
+        setTimeout(el.__x_transition_remaining, duration);
       }.bind(this));
     }.bind(this));
   }
   function isNumeric(subject) {
     return !isNaN(subject);
   }
+  /**
+   * Ensure a function is called only once.
+   */
+
+  function once(fn) {
+    var called = false;
+    return function () {
+      if (!called) {
+        called = true;
+        fn.apply(this, arguments);
+      }
+    };
+  }
 
   function handleForDirective(component, templateEl, expression, initialUpdate, extraVars) {
     var _this = this;
@@ -6236,6 +6255,11 @@
 
     var initialUpdate = arguments.length > 4 && arguments[4] !== undefined ? arguments[4] : false;
 
+    // if value is changed resolve any previous pending transitions before starting a new one
+    if (el.__x_transition_remaining && el.__x_transition_last_value !== value) {
+      el.__x_transition_remaining();
+    }
+
     var hide = function hide() {
       _newArrowCheck(this, _this);
 
@@ -6253,6 +6277,9 @@
     }.bind(this);
 
     if (initialUpdate === true) {
+      // Assign current value to el to check later on for preventing transition overlaps
+      el.__x_transition_last_value = value;
+
       if (value) {
         show();
       } else {
@@ -6267,7 +6294,16 @@
 
       _newArrowCheck(this, _this);
 
-      if (!value) {
+      if (value) {
+        transitionIn(el, function () {
+          _newArrowCheck(this, _this2);
+
+          show();
+        }.bind(this), component);
+        resolve(function () {
+          _newArrowCheck(this, _this2);
+        }.bind(this));
+      } else {
         if (el.style.display !== 'none') {
           transitionOut(el, function () {
             var _this3 = this;
@@ -6285,20 +6321,10 @@
             _newArrowCheck(this, _this2);
           }.bind(this));
         }
-      } else {
-        if (el.style.display !== '') {
-          transitionIn(el, function () {
-            _newArrowCheck(this, _this2);
-
-            show();
-          }.bind(this), component);
-        } // Resolve immediately, only hold up parent `x-show`s for hidin.
+      } // Assign current value to el
 
 
-        resolve(function () {
-          _newArrowCheck(this, _this2);
-        }.bind(this));
-      }
+      el.__x_transition_last_value = value;
     }.bind(this); // The working of x-show is a bit complex because we need to
     // wait for any child transitions to finish before hiding
     // some element. Also, this has to be done recursively.
@@ -6319,11 +6345,13 @@
 
     if (component.showDirectiveLastElement && !component.showDirectiveLastElement.contains(el)) {
       component.executeAndClearRemainingShowDirectiveStack();
-    } // We'll push the handler onto a stack to be handled later.
+    } // If x-show value changed from previous transition we'll push the handler onto a stack to be handled later.
 
 
-    component.showDirectiveStack.push(handle);
-    component.showDirectiveLastElement = el;
+    if (el.__x_transition_last_value !== value) {
+      component.showDirectiveStack.push(handle);
+      component.showDirectiveLastElement = el;
+    }
   }
 
   function handleIfDirective(component, el, expressionResult, initialUpdate, extraVars) {

dist/alpine.js

@@ -204,6 +204,7 @@
     }
   }
   function transitionOut(el, hide, component, forceSkip = false) {
+    // We don't want to transition on the initial page load.
     if (forceSkip) return hide();
     const attrs = getXAttrs(el, component, 'transition');
     const showAttr = getXAttrs(el, component, 'show')[0];
@@ -411,21 +412,39 @@
 
       stages.show();
       requestAnimationFrame(() => {
-        stages.end();
-        setTimeout(() => {
+        stages.end(); // Assign current transition to el in case we need to force it
+
+        el.__x_transition_remaining = once(() => {
           stages.hide(); // Adding an "isConnected" check, in case the callback
           // removed the element from the DOM.
 
           if (el.isConnected) {
             stages.cleanup();
-          }
-        }, duration);
+          } // Safe to remove transition from el since it is completed
+
+
+          delete el.__x_transition_remaining;
+        });
+        setTimeout(el.__x_transition_remaining, duration);
       });
     });
   }
   function isNumeric(subject) {
     return !isNaN(subject);
   }
+  /**
+   * Ensure a function is called only once.
+   */
+
+  function once(fn) {
+    let called = false;
+    return function () {
+      if (!called) {
+        called = true;
+        fn.apply(this, arguments);
+      }
+    };
+  }
 
   function handleForDirective(component, templateEl, expression, initialUpdate, extraVars) {
     warnIfMalformedTemplate(templateEl, 'x-for');
@@ -637,6 +656,11 @@
   }
 
   function handleShowDirective(component, el, value, modifiers, initialUpdate = false) {
+    // if value is changed resolve any previous pending transitions before starting a new one
+    if (el.__x_transition_remaining && el.__x_transition_last_value !== value) {
+      el.__x_transition_remaining();
+    }
+
     const hide = () => {
       el.style.display = 'none';
     };
@@ -650,6 +674,9 @@
     };
 
     if (initialUpdate === true) {
+      // Assign current value to el to check later on for preventing transition overlaps
+      el.__x_transition_last_value = value;
+
       if (value) {
         show();
       } else {
@@ -660,7 +687,12 @@
     }
 
     const handle = resolve => {
-      if (!value) {
+      if (value) {
+        transitionIn(el, () => {
+          show();
+        }, component);
+        resolve(() => {});
+      } else {
         if (el.style.display !== 'none') {
           transitionOut(el, () => {
             resolve(() => {
@@ -670,16 +702,10 @@
         } else {
           resolve(() => {});
         }
-      } else {
-        if (el.style.display !== '') {
-          transitionIn(el, () => {
-            show();
-          }, component);
-        } // Resolve immediately, only hold up parent `x-show`s for hidin.
+      } // Assign current value to el
 
 
-        resolve(() => {});
-      }
+      el.__x_transition_last_value = value;
     }; // The working of x-show is a bit complex because we need to
     // wait for any child transitions to finish before hiding
     // some element. Also, this has to be done recursively.
@@ -696,11 +722,13 @@
 
     if (component.showDirectiveLastElement && !component.showDirectiveLastElement.contains(el)) {
       component.executeAndClearRemainingShowDirectiveStack();
-    } // We'll push the handler onto a stack to be handled later.
+    } // If x-show value changed from previous transition we'll push the handler onto a stack to be handled later.
 
 
-    component.showDirectiveStack.push(handle);
-    component.showDirectiveLastElement = el;
+    if (el.__x_transition_last_value !== value) {
+      component.showDirectiveStack.push(handle);
+      component.showDirectiveLastElement = el;
+    }
   }
 
   function handleIfDirective(component, el, expressionResult, initialUpdate, extraVars) {

src/directives/show.js

@@ -1,6 +1,11 @@
 import { transitionIn, transitionOut } from '../utils'
 
 export function handleShowDirective(component, el, value, modifiers, initialUpdate = false) {
+    // if value is changed resolve any previous pending transitions before starting a new one
+    if (el.__x_transition_remaining && el.__x_transition_last_value !== value) {
+        el.__x_transition_remaining()
+    }
+
     const hide = () => {
         el.style.display = 'none'
     }
@@ -14,6 +19,9 @@ export function handleShowDirective(component, el, value, modifiers, initialUpda
     }
 
     if (initialUpdate === true) {
+        // Assign current value to el to check later on for preventing transition overlaps
+        el.__x_transition_last_value = value
+
         if (value) {
             show()
         } else {
@@ -23,28 +31,28 @@ export function handleShowDirective(component, el, value, modifiers, initialUpda
     }
 
     const handle = (resolve) => {
-        if (! value) {
+        if(value) {
+            transitionIn(el,() => {
+                show()
+            }, component)
+            resolve(() => {})
+        } else {
             if ( el.style.display !== 'none' ) {
                 transitionOut(el, () => {
                     resolve(() => {
                         hide()
                     })
-                }, component)
+                },component)
             } else {
                 resolve(() => {})
             }
-        } else {
-            if ( el.style.display !== '' ) {
-                transitionIn(el, () => {
-                    show()
-                }, component)
-            }
-
-            // Resolve immediately, only hold up parent `x-show`s for hidin.
-            resolve(() => {})
         }
+
+        // Assign current value to el
+        el.__x_transition_last_value = value
     }
 
+
     // The working of x-show is a bit complex because we need to
     // wait for any child transitions to finish before hiding
     // some element. Also, this has to be done recursively.
@@ -62,8 +70,9 @@ export function handleShowDirective(component, el, value, modifiers, initialUpda
         component.executeAndClearRemainingShowDirectiveStack()
     }
 
-    // We'll push the handler onto a stack to be handled later.
-    component.showDirectiveStack.push(handle)
-
-    component.showDirectiveLastElement = el
+    // If x-show value changed from previous transition we'll push the handler onto a stack to be handled later.
+    if (el.__x_transition_last_value !== value) {
+        component.showDirectiveStack.push(handle)
+        component.showDirectiveLastElement = el
+    }
 }

src/utils.js

@@ -188,6 +188,7 @@ export function transitionIn(el, show, component, forceSkip = false) {
 }
 
 export function transitionOut(el, hide, component, forceSkip = false) {
+     // We don't want to transition on the initial page load.
     if (forceSkip) return hide()
 
     const attrs = getXAttrs(el, component, 'transition')
@@ -406,7 +407,8 @@ export function transition(el, stages) {
         requestAnimationFrame(() => {
             stages.end()
 
-            setTimeout(() => {
+            // Assign current transition to el in case we need to force it
+            el.__x_transition_remaining = once(() => {
                 stages.hide()
 
                 // Adding an "isConnected" check, in case the callback
@@ -414,7 +416,12 @@ export function transition(el, stages) {
                 if (el.isConnected) {
                     stages.cleanup()
                 }
-            }, duration);
+
+                 // Safe to remove transition from el since it is completed
+                 delete el.__x_transition_remaining
+            })
+
+            setTimeout(el.__x_transition_remaining, duration);
         })
     });
 }
@@ -422,3 +429,16 @@ export function transition(el, stages) {
 export function isNumeric(subject){
     return ! isNaN(subject)
 }
+
+/**
+ * Ensure a function is called only once.
+ */
+export function once(fn) {
+    let called = false
+    return function () {
+        if (!called) {
+            called = true
+            fn.apply(this, arguments)
+        }
+    }
+}

test/transition.spec.js

@@ -620,3 +620,101 @@ test('x-transition supports css animation', async () => {
     )
     expect(document.querySelector('span').classList.contains('animation-leave')).toEqual(false)
 })
+
+
+test('remaining transitions forced to complete if they exists', async () => {
+    jest.spyOn(window, 'requestAnimationFrame').mockImplementation((callback) => {
+        setTimeout(callback, 0)
+    });
+
+    // (hardcoding 10ms animation time for later assertions)
+    jest.spyOn(window, 'getComputedStyle').mockImplementation(el => {
+        return {
+            transitionDuration: '0s',
+            animationDuration: '.1s'
+        }
+    });
+
+    document.body.innerHTML = `
+        <div x-data="{ show: false }">
+            <button x-on:click="show = ! show"></button>
+
+            <span
+                x-show="show"
+                x-transition:enter="animation-enter"
+                x-transition:leave="animation-leave"
+            ></span>
+        </div>
+    `
+
+    Alpine.start()
+
+    await wait(() => { expect(document.querySelector('span').getAttribute('style')).toEqual('display: none;') })
+
+    // trigger animation in
+    document.querySelector('button').click()
+
+    // Wait for the first requestAnimationFrame
+    await new Promise((resolve) =>
+        setTimeout(() => {
+            resolve();
+        }, 0)
+    )
+    expect(document.querySelector('span').classList.contains('animation-enter')).toEqual(true)
+
+    // trigger animation out
+    document.querySelector('button').click()
+
+    // Wait for the next requestAnimationFrame
+    await new Promise((resolve) =>
+        setTimeout(() => {
+            resolve();
+        }, 0)
+    )
+    expect(document.querySelector('span').classList.contains('animation-leave')).toEqual(true)
+
+    // Wait for the next requestAnimationFrame
+        await new Promise((resolve) =>
+        setTimeout(() => {
+            resolve();
+        }, 0)
+    )
+
+    // trigger animation in
+    document.querySelector('button').click()
+
+    // Wait for the next requestAnimationFrame
+    await new Promise((resolve) =>
+        setTimeout(() => {
+            resolve();
+        }, 0)
+    )
+    expect(document.querySelector('span').classList.contains('animation-enter')).toEqual(true)
+
+    // trigger animation out
+    document.querySelector('button').click()
+
+    // Wait for the next requestAnimationFrame
+    await new Promise((resolve) =>
+        setTimeout(() => {
+            resolve();
+        }, 0)
+    )
+    expect(document.querySelector('span').classList.contains('animation-leave')).toEqual(true)
+
+    // The leave class should still be there since the animationDuration property is 100ms
+    await new Promise((resolve) =>
+        setTimeout(() => {
+            resolve();
+        }, 99)
+    )
+    expect(document.querySelector('span').classList.contains('animation-leave')).toEqual(true)
+
+    // The class shouldn't be there anymore
+    await new Promise((resolve) =>
+        setTimeout(() => {
+            resolve();
+        }, 10)
+    )
+    expect(document.querySelector('span').classList.contains('animation-leave')).toEqual(false)
+})