[FIX] qweb: no more duplicated nodes during transitions

If a node was re-added while being removed (i.e. during the remove
transition), the node was duplicated in the DOM for the delay of
the remove transition. This rev. removes the old occurence directly
in that case.

Fixes #121

src/qweb_extensions.ts

@@ -1,4 +1,5 @@
 import { QWeb, UTILS } from "./qweb_core";
+import { VNode } from "./vdom";
 
 /**
  * Owl QWeb Extensions
@@ -95,7 +96,17 @@ UTILS.nextFrame = function(cb: () => void) {
   requestAnimationFrame(() => requestAnimationFrame(cb));
 };
 
-UTILS.transitionInsert = function(elm: HTMLElement, name: string) {
+UTILS.transitionInsert = function(vn: VNode, name: string) {
+  const elm = <HTMLElement>vn.elm;
+  // remove potential duplicated vnode that is currently being removed, to
+  // prevent from having twice the same node in the DOM during an animation
+  const dup =
+    elm.parentElement &&
+    elm.parentElement!.querySelector(`*[data-owl-key='${vn.key}']`);
+  if (dup) {
+    dup.remove();
+  }
+
   elm.classList.add(name + "-enter");
   elm.classList.add(name + "-enter-active");
   const finalize = () => {
@@ -109,11 +120,10 @@ UTILS.transitionInsert = function(elm: HTMLElement, name: string) {
   });
 };
 
-UTILS.transitionRemove = function(
-  elm: HTMLElement,
-  name: string,
-  rm: () => void
-) {
+UTILS.transitionRemove = function(vn: VNode, name: string, rm: () => void) {
+  const elm = <HTMLElement>vn.elm;
+  elm.setAttribute("data-owl-key", vn.key!);
+
   elm.classList.add(name + "-leave");
   elm.classList.add(name + "-leave-active");
   const finalize = () => {
@@ -170,8 +180,8 @@ QWeb.addDirective({
   atNodeCreation({ value, addNodeHook }) {
     let name = value;
     const hooks = {
-      insert: `this.utils.transitionInsert(vn.elm, '${name}');`,
-      remove: `this.utils.transitionRemove(vn.elm, '${name}', rm);`
+      insert: `this.utils.transitionInsert(vn, '${name}');`,
+      remove: `this.utils.transitionRemove(vn, '${name}', rm);`
     };
     for (let hookName in hooks) {
       addNodeHook(hookName, hooks[hookName]);
@@ -297,7 +307,7 @@ const T_WIDGET_MODS_CODE = Object.assign({}, MODS_CODE, {
  *         let nvn = w4._mount(vnode, vn.elm);
  *         pvnode.elm = nvn.elm;
  *         // what follows is only present if there are animations on the widget
- *         utils.transitionInsert(vn.elm, "fade");
+ *         utils.transitionInsert(vn, "fade");
  *       },
  *       remove() {
  *         // override with empty function to prevent from removing the node
@@ -310,7 +320,7 @@ const T_WIDGET_MODS_CODE = Object.assign({}, MODS_CODE, {
  *         let finalize = () => {
  *           w4.destroy();
  *         };
- *         utils.transitionRemove(vn.elm, "fade", finalize);
+ *         utils.transitionRemove(vn, "fade", finalize);
  *       }
  *     };
  *     // the pvnode is inserted at the correct position in the div's children
@@ -422,7 +432,7 @@ QWeb.addDirective({
     }
     let transitionsInsertCode = "";
     if (transition) {
-      transitionsInsertCode = `utils.transitionInsert(vn.elm, '${transition}');`;
+      transitionsInsertCode = `utils.transitionInsert(vn, '${transition}');`;
     }
     let finalizeWidgetCode = `w${widgetID}.${
       keepAlive ? "unmount" : "destroy"
@@ -434,7 +444,7 @@ QWeb.addDirective({
       finalizeWidgetCode = `let finalize = () => {
           ${finalizeWidgetCode}
         };
-        utils.transitionRemove(vn.elm, '${transition}', finalize);`;
+        utils.transitionRemove(vn, '${transition}', finalize);`;
     }
 
     let createHook = "";

tests/__snapshots__/animations.test.ts.snap

@@ -30,10 +30,10 @@ exports[`animations t-transition combined with t-widget 1`] = `
         w4 = new W4(owner, props4);
         context.__owl__.cmap[4] = w4.__owl__.id;
         def3 = w4._prepare();
-        def3 = def3.then(vnode=>{let pvnode=h(vnode.sel, {key: 4, hook: {insert(vn) {let nvn=w4._mount(vnode, pvnode.elm);pvnode.elm=nvn.elm;utils.transitionInsert(vn.elm, 'chimay');},remove() {},destroy(vn) {let finalize = () => {
+        def3 = def3.then(vnode=>{let pvnode=h(vnode.sel, {key: 4, hook: {insert(vn) {let nvn=w4._mount(vnode, pvnode.elm);pvnode.elm=nvn.elm;utils.transitionInsert(vn, 'chimay');},remove() {},destroy(vn) {let finalize = () => {
           w4.destroy();
         };
-        utils.transitionRemove(vn.elm, 'chimay', finalize);}}});c1[_2_index]=pvnode;w4.__owl__.pvnode = pvnode;});
+        utils.transitionRemove(vn, 'chimay', finalize);}}});c1[_2_index]=pvnode;w4.__owl__.pvnode = pvnode;});
     } else {
         def3 = def3 || w4._updateProps(props4, extra.forceUpdate, extra.patchQueue);
         def3 = def3.then(()=>{if (w4.__owl__.isDestroyed) {return};let pvnode=w4.__owl__.pvnode;c1[_2_index]=pvnode;});
@@ -74,10 +74,10 @@ exports[`animations t-transition combined with t-widget and t-if 1`] = `
             w4 = new W4(owner, props4);
             context.__owl__.cmap[4] = w4.__owl__.id;
             def3 = w4._prepare();
-            def3 = def3.then(vnode=>{let pvnode=h(vnode.sel, {key: 4, hook: {insert(vn) {let nvn=w4._mount(vnode, pvnode.elm);pvnode.elm=nvn.elm;utils.transitionInsert(vn.elm, 'chimay');},remove() {},destroy(vn) {let finalize = () => {
+            def3 = def3.then(vnode=>{let pvnode=h(vnode.sel, {key: 4, hook: {insert(vn) {let nvn=w4._mount(vnode, pvnode.elm);pvnode.elm=nvn.elm;utils.transitionInsert(vn, 'chimay');},remove() {},destroy(vn) {let finalize = () => {
           w4.destroy();
         };
-        utils.transitionRemove(vn.elm, 'chimay', finalize);}}});c1[_2_index]=pvnode;w4.__owl__.pvnode = pvnode;});
+        utils.transitionRemove(vn, 'chimay', finalize);}}});c1[_2_index]=pvnode;w4.__owl__.pvnode = pvnode;});
         } else {
             def3 = def3 || w4._updateProps(props4, extra.forceUpdate, extra.patchQueue);
             def3 = def3.then(()=>{if (w4.__owl__.isDestroyed) {return};let pvnode=w4.__owl__.pvnode;c1[_2_index]=pvnode;});
@@ -96,10 +96,10 @@ exports[`animations t-transition with no delay/duration 1`] = `
     var vn1 = h('span', p1, c1);
     p1.hook = {
       insert: vn => {
-        this.utils.transitionInsert(vn.elm, 'jupiler');
+        this.utils.transitionInsert(vn, 'jupiler');
       },
       remove: (vn, rm) => {
-        this.utils.transitionRemove(vn.elm, 'jupiler', rm);
+        this.utils.transitionRemove(vn, 'jupiler', rm);
       },
     };
     c1.push({text: \`blue\`});
@@ -115,10 +115,10 @@ exports[`animations t-transition, on a simple node (insert) 1`] = `
     var vn1 = h('span', p1, c1);
     p1.hook = {
       insert: vn => {
-        this.utils.transitionInsert(vn.elm, 'chimay');
+        this.utils.transitionInsert(vn, 'chimay');
       },
       remove: (vn, rm) => {
-        this.utils.transitionRemove(vn.elm, 'chimay', rm);
+        this.utils.transitionRemove(vn, 'chimay', rm);
       },
     };
     c1.push({text: \`blue\`});

tests/animations.test.ts

@@ -253,11 +253,11 @@ describe("animations", () => {
     widget.state.display = false;
     patchNextFrame(cb => {
       expect(fixture.innerHTML).toBe(
-        '<div><span class="chimay-leave chimay-leave-active">blue</span></div>'
+        '<div><span class="chimay-leave chimay-leave-active" data-owl-key="4">blue</span></div>'
       );
       cb();
       expect(fixture.innerHTML).toBe(
-        '<div><span class="chimay-leave-active chimay-leave-to">blue</span></div>'
+        '<div><span class="chimay-leave-active chimay-leave-to" data-owl-key="4">blue</span></div>'
       );
       def.resolve();
     });
@@ -287,4 +287,133 @@ describe("animations", () => {
     expect(widget.f).toHaveBeenCalledTimes(1);
     unpatchNextFrame();
   });
+
+  test("t-transition, remove and re-add before transitionend", async () => {
+    expect.assertions(11);
+
+    env.qweb.addTemplates(
+      `<templates>
+        <div t-name="Parent">
+          <button t-on-click="toggle">Toggle</button>
+          <span t-if="state.flag" t-transition="chimay">blue</span>
+        </div>
+      </templates>`
+    );
+    class Parent extends Widget {
+      constructor(parent) {
+        super(parent);
+        this.state = { flag: false };
+      }
+      toggle() {
+        this.state.flag = !this.state.flag;
+      }
+    }
+
+    const widget = new Parent(env);
+    await widget.mount(fixture);
+    let button = widget.el!.querySelector("button");
+
+    let def = makeDeferred();
+    let phase = "enter";
+    patchNextFrame(cb => {
+      let spans = fixture.querySelectorAll("span");
+      expect(spans.length).toBe(1);
+      expect(spans[0].className).toBe(`chimay-${phase} chimay-${phase}-active`);
+      cb();
+      expect(spans[0].className).toBe(
+        `chimay-${phase}-active chimay-${phase}-to`
+      );
+      def.resolve();
+    });
+
+    // click display the span
+    button!.click();
+    await def; // wait for the mocked repaint to be done
+    widget.el!.querySelector("span")!.dispatchEvent(new Event("transitionend")); // mock end of css transition
+    expect(fixture.innerHTML).toBe(
+      '<div><button>Toggle</button><span class="">blue</span></div>'
+    );
+
+    // click to remove the span, and click again to re-add it before transitionend
+    def = makeDeferred();
+    phase = "leave";
+    button!.click();
+
+    await def; // wait for the mocked repaint to be done
+    def = makeDeferred();
+    phase = "enter";
+    button!.click();
+
+    await def; // wait for the mocked repaint to be done
+    widget.el!.querySelector("span")!.dispatchEvent(new Event("transitionend")); // mock end of css transition
+    expect(fixture.innerHTML).toBe(
+      '<div><button>Toggle</button><span class="">blue</span></div>'
+    );
+  });
+
+  test("t-transition combined with t-widget, remove and re-add before transitionend", async () => {
+    expect.assertions(11);
+
+    env.qweb.addTemplates(
+      `<templates>
+        <div t-name="Parent">
+          <button t-on-click="toggle">Toggle</button>
+          <t t-if="state.flag" t-widget="Child" t-transition="chimay"/>
+        </div>
+        <span t-name="Child">blue</span>
+      </templates>`
+    );
+    class Child extends Widget {}
+    class Parent extends Widget {
+      widgets = { Child };
+      constructor(parent) {
+        super(parent);
+        this.state = { flag: false };
+      }
+      toggle() {
+        this.state.flag = !this.state.flag;
+      }
+    }
+
+    const widget = new Parent(env);
+    await widget.mount(fixture);
+    let button = widget.el!.querySelector("button");
+
+    let def = makeDeferred();
+    let phase = "enter";
+    patchNextFrame(cb => {
+      let spans = fixture.querySelectorAll("span");
+      expect(spans.length).toBe(1);
+      expect(spans[0].className).toBe(`chimay-${phase} chimay-${phase}-active`);
+      cb();
+      expect(spans[0].className).toBe(
+        `chimay-${phase}-active chimay-${phase}-to`
+      );
+      def.resolve();
+    });
+
+    // click display the span
+    button!.click();
+    await def; // wait for the mocked repaint to be done
+    widget.el!.querySelector("span")!.dispatchEvent(new Event("transitionend")); // mock end of css transition
+    expect(fixture.innerHTML).toBe(
+      '<div><button>Toggle</button><span class="">blue</span></div>'
+    );
+
+    // click to remove the span, and click again to re-add it before transitionend
+    def = makeDeferred();
+    phase = "leave";
+    button!.click();
+
+    await def; // wait for the mocked repaint to be done
+    def = makeDeferred();
+    phase = "enter";
+    button!.click();
+
+    await def; // wait for the mocked repaint to be done
+    widget.el!.querySelector("span")!.dispatchEvent(new Event("transitionend")); // mock end of css transition
+    expect(fixture.innerHTML).toBe(
+      '<div><button>Toggle</button><span class="">blue</span></div>'
+    );
+  });
 });

tools/playground/samples.js

@@ -187,7 +187,7 @@ const ANIMATION_CSS = `button {
 
 .flash {
     background-position: center;
-    transition: background 0.5s;
+    transition: background .6s;
 }
 
 .flash:active {
@@ -208,7 +208,7 @@ const ANIMATION_CSS = `button {
 }
 
 .fade-enter-active, .fade-leave-active {
-    transition: opacity .5s;
+    transition: opacity .6s;
 }
 .fade-enter, .fade-leave-to {
     opacity: 0;