Merge pull request #623 from SimoTod/feature/transition-improvements

Feature - Transition improvements

src/directives/if.js

@@ -5,7 +5,7 @@ export function handleIfDirective(component, el, expressionResult, initialUpdate
 
     const elementHasAlreadyBeenAdded = el.nextElementSibling && el.nextElementSibling.__x_inserted_me === true
 
-    if (expressionResult && ! elementHasAlreadyBeenAdded) {
+    if (expressionResult && (! elementHasAlreadyBeenAdded || el.__x_transition)) {
         const clone = document.importNode(el.content, true);
 
         el.parentElement.insertBefore(clone, el.nextElementSibling)

src/directives/show.js

@@ -24,12 +24,14 @@ export function handleShowDirective(component, el, value, modifiers, initialUpda
 
     const handle = (resolve) => {
         if (value) {
-            transitionIn(el,() => {
-                show()
-            }, component)
+            if(el.style.display === 'none' || el.__x_transition) {
+                transitionIn(el, () => {
+                    show()
+                }, component)
+            }
             resolve(() => {})
         } else {
-            if (el.style.display !== 'none' ) {
+            if (el.style.display !== 'none') {
                 transitionOut(el, () => {
                     resolve(() => {
                         hide()

src/utils.js

@@ -165,9 +165,19 @@ export function convertClassStringToArray(classList, filterFn = Boolean) {
     return classList.split(' ').filter(filterFn)
 }
 
+const TRANSITION_TYPE_IN = 'in'
+const TRANSITION_TYPE_OUT = 'out'
+
 export function transitionIn(el, show, component, forceSkip = false) {
+    // We don't want to transition on the initial page load.
     if (forceSkip) return show()
 
+    if (el.__x_transition && el.__x_transition.type === TRANSITION_TYPE_IN) {
+        // there is already a similar transition going on, this was probably triggered by
+        // a change in a different property, let's just leave the previous one doing its job
+        return
+    }
+
     const attrs = getXAttrs(el, component, 'transition')
     const showAttr = getXAttrs(el, component, 'show')[0]
 
@@ -195,9 +205,15 @@ 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.
+    // We don't want to transition on the initial page load.
     if (forceSkip) return hide()
 
+    if (el.__x_transition && el.__x_transition.type === TRANSITION_TYPE_OUT) {
+        // there is already a similar transition going on, this was probably triggered by
+        // a change in a different property, let's just leave the previous one doing its job
+        return
+    }
+
     const attrs = getXAttrs(el, component, 'transition')
     const showAttr = getXAttrs(el, component, 'show')[0]
 
@@ -234,7 +250,7 @@ export function transitionHelperIn(el, modifiers, showCallback) {
         },
     }
 
-    transitionHelper(el, modifiers, showCallback, () => {}, styleValues)
+    transitionHelper(el, modifiers, showCallback, () => {}, styleValues, TRANSITION_TYPE_IN)
 }
 
 export function transitionHelperOut(el, modifiers, settingBothSidesOfTransition, hideCallback) {
@@ -258,7 +274,7 @@ export function transitionHelperOut(el, modifiers, settingBothSidesOfTransition,
         },
     }
 
-    transitionHelper(el, modifiers, () => {}, hideCallback, styleValues)
+    transitionHelper(el, modifiers, () => {}, hideCallback, styleValues, TRANSITION_TYPE_OUT)
 }
 
 function modifierValue(modifiers, key, fallback) {
@@ -293,7 +309,13 @@ function modifierValue(modifiers, key, fallback) {
     return rawValue
 }
 
-export function transitionHelper(el, modifiers, hook1, hook2, styleValues) {
+export function transitionHelper(el, modifiers, hook1, hook2, styleValues, type) {
+    // clear the previous transition if exists to avoid caching the wrong styles
+    if (el.__x_transition) {
+        cancelAnimationFrame(el.__x_transition.nextFrame)
+        el.__x_transition.callback && el.__x_transition.callback()
+    }
+
     // If the user set these style values, we'll put them back when we're done with them.
     const opacityCache = el.style.opacity
     const transformCache = el.style.transform
@@ -338,7 +360,7 @@ export function transitionHelper(el, modifiers, hook1, hook2, styleValues) {
         },
     }
 
-    transition(el, stages)
+    transition(el, stages, type)
 }
 
 export function transitionClassesIn(el, component, directives, showCallback) {
@@ -352,7 +374,7 @@ export function transitionClassesIn(el, component, directives, showCallback) {
     const enterStart = convertClassStringToArray(ensureStringExpression((directives.find(i => i.value === 'enter-start') || { expression: '' }).expression))
     const enterEnd = convertClassStringToArray(ensureStringExpression((directives.find(i => i.value === 'enter-end') || { expression: '' }).expression))
 
-    transitionClasses(el, enter, enterStart, enterEnd, showCallback, () => {})
+    transitionClasses(el, enter, enterStart, enterEnd, showCallback, () => {}, TRANSITION_TYPE_IN)
 }
 
 export function transitionClassesOut(el, component, directives, hideCallback) {
@@ -360,10 +382,16 @@ export function transitionClassesOut(el, component, directives, hideCallback) {
     const leaveStart = convertClassStringToArray((directives.find(i => i.value === 'leave-start') || { expression: '' }).expression)
     const leaveEnd = convertClassStringToArray((directives.find(i => i.value === 'leave-end') || { expression: '' }).expression)
 
-    transitionClasses(el, leave, leaveStart, leaveEnd, () => {}, hideCallback)
+    transitionClasses(el, leave, leaveStart, leaveEnd, () => {}, hideCallback, TRANSITION_TYPE_OUT)
 }
 
-export function transitionClasses(el, classesDuring, classesStart, classesEnd, hook1, hook2) {
+export function transitionClasses(el, classesDuring, classesStart, classesEnd, hook1, hook2, type) {
+    // clear the previous transition if exists to avoid caching the wrong classes
+    if (el.__x_transition) {
+        cancelAnimationFrame(el.__x_transition.nextFrame)
+        el.__x_transition.callback && el.__x_transition.callback()
+    }
+
     const originalClasses = el.__x_original_classes || []
 
     const stages = {
@@ -390,14 +418,35 @@ export function transitionClasses(el, classesDuring, classesStart, classesEnd, h
         },
     }
 
-    transition(el, stages)
+    transition(el, stages, type)
 }
 
-export function transition(el, stages) {
+export function transition(el, stages, type) {
+    el.__x_transition = {
+        // Set transition type so we can avoid clearing transition if the direction is the same
+       type: type,
+        // create a callback for the last stages of the transition so we can call it
+        // from different point and early terminate it. Once will ensure that function
+        // is only called one time.
+        callback: once(() => {
+            stages.hide()
+
+            // Adding an "isConnected" check, in case the callback
+            // removed the element from the DOM.
+            if (el.isConnected) {
+                stages.cleanup()
+            }
+
+            delete el.__x_transition
+        }),
+        // This store the next animation frame so we can cancel it
+        nextFrame: null
+    }
+
     stages.start()
     stages.during()
 
-    requestAnimationFrame(() => {
+    el.__x_transition.nextFrame =requestAnimationFrame(() => {
         // Note: Safari's transitionDuration property will list out comma separated transition durations
         // for every single transition property. Let's grab the first one and call it a day.
         let duration = Number(getComputedStyle(el).transitionDuration.replace(/,.*/, '').replace('s', '')) * 1000
@@ -408,19 +457,10 @@ export function transition(el, stages) {
 
         stages.show()
 
-        requestAnimationFrame(() => {
+        el.__x_transition.nextFrame =requestAnimationFrame(() => {
             stages.end()
 
-            // Assign current transition to el in case we need to force it.
-            setTimeout(() => {
-                stages.hide()
-
-                // Adding an "isConnected" check, in case the callback
-                // removed the element from the DOM.
-                if (el.isConnected) {
-                    stages.cleanup()
-                }
-            }, duration)
+            setTimeout(el.__x_transition.callback, duration)
         })
     });
 }
@@ -428,3 +468,16 @@ export function transition(el, stages) {
 export function isNumeric(subject){
     return ! isNaN(subject)
 }
+
+// Thanks @vuejs
+// https://github.com/vuejs/vue/blob/4de4649d9637262a9b007720b59f80ac72a5620c/src/shared/util.js
+export function once(callback) {
+    let called = false
+
+    return function () {
+        if (! called) {
+            called = true
+            callback.apply(this, arguments)
+        }
+    }
+}

test/show.spec.js

@@ -126,3 +126,36 @@ test('x-show works with nested x-shows of different functions (hiding vs showing
         expect(document.querySelector('h1').getAttribute('style')).toEqual(null)
     })
 })
+
+// Regression in 2.4.0
+test('x-show with x-bind:style inside x-for works correctly', async () => {
+    document.body.innerHTML = `
+        <div x-data="{items: [{ cleared: false }, { cleared: false }]}">
+            <template x-for="(item, index) in items" :key="index">
+                <button x-show="! item.cleared"
+                    x-bind:style="'background: #999'"
+                    @click="item.cleared = true"
+                >
+                </button>
+            </template>
+        </div>
+    `
+    Alpine.start()
+
+    expect(document.querySelectorAll('button')[0].style.display).toEqual('')
+    expect(document.querySelectorAll('button')[1].style.display).toEqual('')
+
+    document.querySelectorAll('button')[0].click()
+
+    await wait(() => {
+        expect(document.querySelectorAll('button')[0].style.display).toEqual('none')
+        expect(document.querySelectorAll('button')[1].style.display).toEqual('')
+    })
+
+    document.querySelectorAll('button')[1].click()
+
+    await wait(() => {
+        expect(document.querySelectorAll('button')[0].style.display).toEqual('none')
+        expect(document.querySelectorAll('button')[1].style.display).toEqual('none')
+    })
+})

test/transition.spec.js

@@ -135,15 +135,12 @@ test('transition out', async () => {
     expect(document.querySelector('span').classList.contains('leave-end')).toEqual(true)
     expect(document.querySelector('span').getAttribute('style')).toEqual(null)
 
-    await new Promise((resolve) =>
-        setTimeout(() => {
-            expect(document.querySelector('span').classList.contains('leave')).toEqual(false)
-            expect(document.querySelector('span').classList.contains('leave-start')).toEqual(false)
-            expect(document.querySelector('span').classList.contains('leave-end')).toEqual(false)
-            expect(document.querySelector('span').getAttribute('style')).toEqual('display: none;')
-            resolve();
-        }, 10)
-    )
+    await timeout(10)
+
+    expect(document.querySelector('span').classList.contains('leave')).toEqual(false)
+    expect(document.querySelector('span').classList.contains('leave-start')).toEqual(false)
+    expect(document.querySelector('span').classList.contains('leave-end')).toEqual(false)
+    expect(document.querySelector('span').getAttribute('style')).toEqual('display: none;')
 })
 
 test('if only transition leave directives are present, don\'t transition in at all', async () => {
@@ -281,8 +278,8 @@ test('transition in not called when item is already visible', async () => {
     });
 
     document.body.innerHTML = `
-        <div x-data="{ show: true }">
-            <button x-on:click="show = true"></button>
+        <div x-data="{ show: true, foo: 'bar' }">
+            <button x-on:click="foo = 'bob'"></button>
 
             <span
                 x-show="show"
@@ -330,8 +327,8 @@ test('transition out not called when item is already hidden', async () => {
     });
 
     document.body.innerHTML = `
-        <div x-data="{ show: false }">
-            <button x-on:click="show = false"></button>
+        <div x-data="{ show: false, foo: 'bar' }">
+            <button x-on:click="foo = 'bob'"></button>
 
             <span
                 x-show="show"
@@ -620,3 +617,118 @@ test('x-transition supports css animation', async () => {
     )
     expect(document.querySelector('span').classList.contains('animation-leave')).toEqual(false)
 })
+
+test('x-transition do not overlap', async () => {
+    jest.spyOn(window, 'requestAnimationFrame').mockImplementation((callback) => {
+        setTimeout(callback, 0)
+    });
+
+    document.body.innerHTML = `
+        <div x-data="{ show: true }">
+            <button x-on:click="show = ! show"></button>
+
+            <span x-show.transition="show"></span>
+        </div>
+    `
+
+    Alpine.start()
+
+    // Initial state
+    expect(document.querySelector('span').style.display).toEqual("")
+    expect(document.querySelector('span').style.opacity).toEqual("")
+    expect(document.querySelector('span').style.transform).toEqual("")
+    expect(document.querySelector('span').style.transformOrigin).toEqual("")
+
+    // Trigger transition out
+    document.querySelector('button').click()
+
+    // Trigger transition in before the previous one has finished
+    await timeout(10)
+    document.querySelector('button').click()
+
+    // Check the element is still visible and style properties are correct
+    await timeout(200)
+    expect(document.querySelector('span').style.display).toEqual("")
+    expect(document.querySelector('span').style.opacity).toEqual("")
+    expect(document.querySelector('span').style.transform).toEqual("")
+    expect(document.querySelector('span').style.transformOrigin).toEqual("")
+
+    // Hide the element
+    document.querySelector('button').click()
+    await timeout(200)
+    expect(document.querySelector('span').style.display).toEqual("none")
+    expect(document.querySelector('span').style.opacity).toEqual("")
+    expect(document.querySelector('span').style.transform).toEqual("")
+    expect(document.querySelector('span').style.transformOrigin).toEqual("")
+
+    // Trigger transition in
+    document.querySelector('button').click()
+
+    // Trigger transition out before the previous one has finished
+    await timeout(10)
+    document.querySelector('button').click()
+
+    // Check the element is hidden and style properties are correct
+    await timeout(200)
+    expect(document.querySelector('span').style.display).toEqual("none")
+    expect(document.querySelector('span').style.opacity).toEqual("")
+    expect(document.querySelector('span').style.transform).toEqual("")
+    expect(document.querySelector('span').style.transformOrigin).toEqual("")
+})
+
+test('x-transition using classes do not overlap', async () => {
+    jest.spyOn(window, 'requestAnimationFrame').mockImplementation((callback) => {
+        setTimeout(callback, 0)
+    });
+    jest.spyOn(window, 'getComputedStyle').mockImplementation(el => {
+        return { transitionDuration: '.1s' }
+    });
+
+    document.body.innerHTML = `
+        <div x-data="{ show: true }">
+            <button x-on:click="show = ! show"></button>
+
+            <span x-show="show"
+                x-transition:enter="enter"
+                x-transition:leave="leave">
+            </span>
+        </div>
+    `
+
+    Alpine.start()
+
+    // Initial state
+    expect(document.querySelector('span').style.display).toEqual("")
+
+    const emptyClassList = document.querySelector('span').classList
+
+    // Trigger transition out
+    document.querySelector('button').click()
+
+    // Trigger transition in before the previous one has finished
+    await timeout(10)
+    document.querySelector('button').click()
+
+    // Check the element is still visible and class property is correct
+    await timeout(200)
+    expect(document.querySelector('span').style.display).toEqual("")
+    expect(document.querySelector('span').classList).toEqual(emptyClassList)
+
+    // Hide the element
+    document.querySelector('button').click()
+    await timeout(200)
+    expect(document.querySelector('span').style.display).toEqual("none")
+    expect(document.querySelector('span').classList).toEqual(emptyClassList)
+
+    // Trigger transition in
+    document.querySelector('button').click()
+
+    // Trigger transition out before the previous one has finished
+    await timeout(10)
+    document.querySelector('button').click()
+
+    // Check the element is hidden and class property is correct
+    await timeout(200)
+    expect(document.querySelector('span').style.display).toEqual("none")
+    expect(document.querySelector('span').classList).toEqual(emptyClassList)
+})