Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

ng-leave removes the item when transition of a child element is finished #16210

Closed
1 of 3 tasks
bzzz-dmg opened this issue Aug 30, 2017 · 4 comments
Closed
1 of 3 tasks

Comments

@bzzz-dmg
Copy link

bzzz-dmg commented Aug 30, 2017

I'm submitting a ...

  • bug report
  • feature request
  • other (Please do not submit support requests here (see above))

Current behavior:

ng-leave removes item too early, when some of the child elements finishes its transition

Expected / new behavior:

Items are not removed before ng-leave transition is finished.

Minimal reproduction of the problem with instructions:

Demo: https://plnkr.co/edit/pwDJA6s6ZJvfeJKiRjWk?p=preview
In the first list items are removed only after child transition is finished, so it works as expected.
In the second list items are removed a bit earlier so that child transition ends before ng-leave transition which leads to termination of ng-leave transition and removes the item.

Here is a workaround for this particular problem: https://plnkr.co/edit/0Xwe0JWfx5wmVRfDJi2i?p=preview
It cancels transition of child element, but it have to work without such hacks.

AngularJS version: 1.6.6

Browser: [Chromium 60 | Chrome 60 | Firefox 55]

Anything else:

I'm not sure how it works exactly, but transition time should probably be determined only on the item ng-leave is applied on, not anything else.

@Narretz
Copy link
Contributor

Narretz commented Sep 4, 2017

Hi,
thanks for the report.
I think the problem is that our transitionend event listener does not work correctly when the event is coming from a child element. (

function onAnimationProgress(event) {
event.stopPropagation();
var ev = event.originalEvent || event;
// we now always use `Date.now()` due to the recent changes with
// event.timeStamp in Firefox, Webkit and Chrome (see #13494 for more info)
var timeStamp = ev.$manualTimeStamp || Date.now();
/* Firefox (or possibly just Gecko) likes to not round values up
* when a ms measurement is used for the animation */
var elapsedTime = parseFloat(ev.elapsedTime.toFixed(ELAPSED_TIME_MAX_DECIMAL_PLACES));
/* $manualTimeStamp is a mocked timeStamp value which is set
* within browserTrigger(). This is only here so that tests can
* mock animations properly. Real events fallback to event.timeStamp,
* or, if they don't, then a timeStamp is automatically created for them.
* We're checking to see if the timeStamp surpasses the expected delay,
* but we're using elapsedTime instead of the timeStamp on the 2nd
* pre-condition since animationPauseds sometimes close off early */
if (Math.max(timeStamp - startTime, 0) >= maxDelayTime && elapsedTime >= maxDuration) {
// we set this flag to ensure that if the transition is paused then, when resumed,
// the animation will automatically close itself since transitions cannot be paused.
animationCompleted = true;
close();
}
}
) I think we should simply ignore all of the event in this case. I'll work on a test case and PR.

@Narretz
Copy link
Contributor

Narretz commented Sep 5, 2017

Here's a plunker with a fix: https://plnkr.co/edit/nKU8lkiwVI6kphL99G6J?p=preview
And here's the PR #16214

@bzzz-dmg
Copy link
Author

bzzz-dmg commented Sep 5, 2017

Looks good, thank you.

Narretz added a commit that referenced this issue Sep 6, 2017
Previously, non-angular CSS transitions / animations on child elements
would trigger the ngAnimate [transition|animation]end listener and
could close a running animation prematurely.

Closes #16210
@Narretz
Copy link
Contributor

Narretz commented Sep 6, 2017

Fix will be in the 1.6.7 release (commit 1391e99)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants