Feature - Transition improvements #623
Conversation
| export function once(callback) { | ||
| let called = false | ||
|
|
||
| return function () { |
There was a problem hiding this comment.
missing arguments parameter?
There was a problem hiding this comment.
There was a problem hiding this comment.
that function is always without arguments anyway, I can pass an empty array if clearer but I just kept the same function as VueJS (https://github.com/vuejs/vue/blob/4de4649d9637262a9b007720b59f80ac72a5620c/src/shared/util.js#L335)
There was a problem hiding this comment.
omg, I never knew about this weird arguments variable - cheers!
There was a problem hiding this comment.
could use function(...args) instead but I think the transpiled output would be larger & it probably makes sense to do it the same way Vue does
|
Great work @SimoTod! <div x-data="{show: true}">
<p>shown: <span x-text="show"></span></p>
<button @click="show = !show">Toggle</button>
<div x-show.transition.duration.1000ms="show">
PARENT
<div x-show.transition.duration.5000ms="show">
CHILD
</div>
</div>
</div>What do you expect in this case to happen? child needs 5000ms to complete the transition but parent needs 1000ms. I am expecting child element finish its transition and parent start once all child element finished. Or am i wrong? |
|
I need to test it but I believe that, with the current code, they start at the same time when showing but they are sequential when hiding, since the resolve callback is separate for the show part in x-show. Yeah, so hide wait for the child component to disappear show doesn't. This only affects the x-show directive, transition effects start together. It's not linked to this PR (it happens in previous versions since it's been coded in that way) so I would suggest we start a new thread to discuss if it needs fixing. |
|
Yeah, I was not trying to link it to this PR but want to know if i make sense to expect this behavior. Maybe we should consider this behavior in transitions. Will be great for the component patent - child reactivity |
SimoTod
force-pushed
the
feature/transition-improvements
branch
from
e96b31b to
af7b662
Compare
SimoTod
force-pushed
the
feature/transition-improvements
branch
from
8461d2c to
5c7eca5
Compare
| transitionIn(el,() => { | ||
| show() | ||
| }, component) | ||
| if(el.style.display === 'none' || el.__x_transition) { |
There was a problem hiding this comment.
This line removed in v2.4.1 caused a regression experienced by several users.
We need to queue the animation only if the element is hidden or there is a transition running.
We don't check the type of transition and we don't add additional logic here, the low level functions will take care of ensuring that duplicated transition are discarded
SimoTod
force-pushed
the
feature/transition-improvements
branch
from
42285a2 to
1583d14
Compare
|
@calebporzio a couple of guys on discord confirmed that it works correctly for them. |
|
Thank you so much @SimoTod for all your hard work on this ❤️ ❤️ ❤️ |
Closes #612
Closes #376
Closes #643
v2.4.1 fixed an issue with x-show directives inside x-for but introduced a regression experienced by a few people in the discord channel and affecting couple of demos:
show animations are re triggered every time a component is repainted so, depending on the effect applied, it could result in element flickering or similar.
2.4.1 also still represented the issue when show and hide overlap (See https://codepen.io/SimoTod/full/XWXzKaQ and try to double click on toggle: show variable will be set to true but the x-show div will be hidden and, from that point, the whole logic will be broken unless you refresh the page).
This PR aims to reinstantiate the fix in 2.4.0 but with support for x-show inside x-for and a few more changes:
If this PR is accepted, there will be a follow up PR fixing some additional edge cases with request animation frame.I kept them separated so it's easier to review and the second bunch of changes doesn't block this first PR which is probably more important.
Memory card game using fixed version: https://codepen.io/SimoTod/full/yLePaLW
Overlapping transition using fixed version: https://codepen.io/SimoTod/pen/gOPXwbv
-- NOTE --
test('transition out not called when item is already hidden')andtest('transition in not called when item is already visible)have been updated to catch the regression in 2.4.1. Those tests were written with the original reactivity layer, the salesforce membrane by default does not trigger a repainting if value hasn't changed so those 2 tests were not valid anymore.I've also added a test to cover the regression in 2.4.0 (it was a bit tricky to strip it down to a minimal state but it happens when x-show directives inside x-for have an x-bind:style directive).