From ecfccf24480d691addcd6cbda641721c8d5511b1 Mon Sep 17 00:00:00 2001
From: "Lucas Perais (lpe)" <lpe@odoo.com>
Date: Tue, 3 Dec 2019 15:10:23 +0100
Subject: [PATCH] [FIX] portal: manually unmounting works

Before this commit, when manually unmounting the portal
or one of its parents, the teleported elements were not cleaned
and stayed teleported forever

After this commit, the teleported content is brought back and
unmounted much like any component

closes #531
---
 src/misc/portal.ts        | 14 ++++++++-
 tests/misc/portal.test.ts | 62 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/src/misc/portal.ts b/src/misc/portal.ts
index 41178a5f..00cd1051 100644
--- a/src/misc/portal.ts
+++ b/src/misc/portal.ts
@@ -59,6 +59,16 @@ export class Portal extends Component<any, any> {
       }
     });
   }
+  /**
+   * Override to revert back to a classic Component's structure
+   *
+   * @override
+   */
+  __callWillUnmount() {
+    super.__callWillUnmount();
+    this.el!.appendChild(this.portal!.elm!);
+    this.doTargetLookUp = true;
+  }
   /**
    * At each DOM change, we must ensure that the portal contains exactly one
    * child
@@ -126,7 +136,9 @@ export class Portal extends Component<any, any> {
       }
     }
     this.__checkVNodeStructure(vnode);
-    const shouldDeploy = !this.portal && !this.doTargetLookUp;
+    const shouldDeploy =
+      (!this.portal || this.el!.contains(this.portal.elm!)) &&
+      !this.doTargetLookUp;
 
     if (!this.doTargetLookUp && !shouldDeploy) {
       // Only on pure patching, provided the
diff --git a/tests/misc/portal.test.ts b/tests/misc/portal.test.ts
index abc14a4b..62da0031 100644
--- a/tests/misc/portal.test.ts
+++ b/tests/misc/portal.test.ts
@@ -548,6 +548,68 @@ describe("Portal: Basic use and DOM placement", () => {
     expect(error).toBeDefined();
     expect(error.message).toBe("Cannot read property 'crash' of undefined");
   });
+
+  test("portal manual unmount", async () => {
+    class Parent extends Component<any, any> {
+      static components = { Portal };
+      static template = xml`
+        <div>
+          <Portal target="'#outside'">
+            <span>gloria</span>
+          </Portal>
+        </div>`;
+    }
+
+    const parent = new Parent();
+    await parent.mount(fixture);
+
+    expect(outside.innerHTML).toBe('<span>gloria</span>');
+    expect(parent.el!.innerHTML).toBe('<portal></portal>');
+
+    parent.unmount();
+    expect(outside.innerHTML).toBe('');
+    expect(parent.el!.innerHTML).toBe('<portal><span>gloria</span></portal>');
+
+    await parent.mount(fixture);
+    expect(outside.innerHTML).toBe('<span>gloria</span>');
+    expect(parent.el!.innerHTML).toBe('<portal></portal>');
+  });
+
+  test("portal manual unmount with subcomponent", async () => {
+    expect.assertions(9);
+    class Child extends Component<any, any> {
+      static template = xml`<span>gloria</span>`;
+      mounted() {
+        expect(outside.contains(this.el)).toBeTruthy();
+      }
+      willUnmount() {
+        expect(outside.contains(this.el)).toBeTruthy();
+      }
+    }
+    class Parent extends Component<any, any> {
+      static components = { Portal , Child };
+      static template = xml`
+        <div>
+          <Portal target="'#outside'">
+            <Child />
+          </Portal>
+        </div>`;
+    }
+
+    const parent = new Parent();
+    await parent.mount(fixture);
+
+    expect(outside.innerHTML).toBe('<span>gloria</span>');
+    expect(parent.el!.innerHTML).toBe('<portal></portal>');
+
+    parent.unmount();
+    expect(outside.innerHTML).toBe('');
+    expect(parent.el!.innerHTML).toBe('<portal><span>gloria</span></portal>');
+
+    await parent.mount(fixture);
+    expect(outside.innerHTML).toBe('<span>gloria</span>');
+    expect(parent.el!.innerHTML).toBe('<portal></portal>');
+  });
 });
 
 describe("Portal: Events handling", () => {