4

I am using the foreach binding along with the beforeRemove callback in order to animate the removal of my list items. It is working fine when I remove an item above the item I just previously removed, but when I attempt to remove an item below the item previously removed, the transition doesn't work correctly.

Here is a jsfiddle illustrating my issue: http://jsfiddle.net/tylerweeres/2szjxzs0/2/

HTML

<h3>Animations</h3>
<div data-bind="foreach: { data: people, beforeRemove: animateOut }">
    <div style="padding: 5px;" class="person">
        <span data-bind="text: name" style="display: inline-block; width: 200px;"></span>
        <button style="padding: 5px 15px;" data-bind="click: $parent.removePerson">Remove</button>
    </div>
</div>

CSS

.person {
    font-family: georgia;
    padding: 5px;
    transition: all 600ms ease-in-out;
    transform-origin: 75px;
}

.person.remove {
    transform: translateX(2000px) rotateZ(540deg);
}

JS

function vm () {
    "use strict";
    var me = this;

    me.people = ko.observableArray([
        { "name": "Irma Olsen" },
        { "name": "Winifred Cabrera" },
        // ...
        { "name": "Lucius Howard" },
        { "name": "Hedda Santana" }
    ]);

    me.removePerson = function (person) {
        me.people.remove(person);
    };

    me.animateOut = function animateOut(node, i, person) {
        // Make sure it's not a text node
        if (node.nodeType == Node.ELEMENT_NODE) {
            node.classList.add("remove");
        }
    }
}

ko.applyBindings(new vm());

3 Answers3

2

This is a weird one, but I think it has to do with crazy browser rendering logic stuff. You need to force a redraw that isn't happening. At one point we have:

<div class="person remove">
<div class="person">

Then we go to

<div class=person>
<div class=person>

The same div doesn't have the remove class anymore -- when we add the remove class, Knockout removes the data, but the browser doesn't know to update this div and so it doesn't perform the transition which means transitionEnd doesn't fire and the div doesn't get removed either.

So how do we force a redraw? Obviously by calling node.offsetHeight (a valid expression).

http://jsfiddle.net/ExplosionPIlls/2szjxzs0/6/

I'm sure someone can explain much more eloquently how this works and why this needs to be done, but at the very least it should solve your immediate problem.

Community
  • 1
  • 1
Explosion Pills
  • 188,624
  • 52
  • 326
  • 405
2

From the documentation of the beforeRemove event:

beforeRemove — is invoked when an array item has been removed, but before the corresponding DOM nodes have been removed. If you specify a beforeRemove callback, then it becomes your responsibility to remove the DOM nodes.

In your beforeRemove you only remove the DOM nodes if they are not text node but you need to remove all the nodes.

So you need to add an else case with node.parentElement.removeChild(node);:

me.animateOut = function animateOut(node, i, person) {
    // Make sure it's not a text node
    if (node.nodeType == Node.ELEMENT_NODE) {
        node.addEventListener("transitionend", function () {
            this.parentElement.removeChild(this);
        });
        node.classList.add("remove");
    }
    else{
        node.parentElement.removeChild(node);
    }
}

Demo JSFiddle.

nemesv
  • 138,284
  • 16
  • 416
  • 359
  • Thanks. It's probably best to remove the text nodes as well, but Explosion Pills nailed it with the redraw. – tyler.weeres Aug 08 '14 at 21:11
  • 1
    I think that the proper solution is remove the text nodes because in this case knockout properly handles the rendering and you don't hacks to force a redraw... – nemesv Aug 08 '14 at 21:12
  • It does help, but the issue would still occur when removing an item while the last transition was still in progress. Forcing the redraw works. – tyler.weeres Aug 08 '14 at 21:17
1

Your transition ends after the remove happens so it probably messes up the rendering.

You can fix this by notifying the people collection that the transition has ended.

me.animateOut = function animateOut(node, i, person) {
    // Make sure it's not a text node
    if (node.nodeType == Node.ELEMENT_NODE) {
        node.addEventListener("transitionend", function () {
            this.parentElement.removeChild(this);
            me.people.notifySubscribers(); // Add this line here
        });
        node.classList.add("remove");
    }
}

Edited Fiddle

Note that if you have manual subscriptions they will be notified with an undefined argument to the notification callback so you should be ready for that.

dee-see
  • 23,668
  • 5
  • 58
  • 91