fix(core): 🐛 keep container position

when container is not the only child, keep its position rather than append it at the and of the wrapper
barbajs · Aug 22, 2019 · 5482154 · 5482154
1 parent e92240a
commit 5482154
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 11 deletions.
diff --git a/packages/core/__tests__/utils/dom.test.ts b/packages/core/__tests__/utils/dom.test.ts
@@ -18,9 +18,7 @@ wrapper.appendChild(container);
 
 // Expected
 const checkDoc = new RegExp(
-  `^<html>[\\s\\S]+body[\\s\\S]+${dom['_attr'].wrapper}[\\s\\S]+${
-    dom['_attr'].container
-  }[\\s\\S]+${namespace}[\\s\\S]+</html>$`
+  `^<html>[\\s\\S]+body[\\s\\S]+${dom['_attr'].wrapper}[\\s\\S]+${dom['_attr'].container}[\\s\\S]+${namespace}[\\s\\S]+</html>$`
 );
 
 afterEach(() => {
@@ -77,3 +75,33 @@ it('get namespace', () => {
   expect(dom.getNamespace()).toBe(namespace);
   expect(dom.getNamespace(wrapper)).toBe(namespace);
 });
+
+it('remove container', () => {
+  document.body.appendChild(wrapper);
+
+  dom.removeContainer(container);
+
+  expect(wrapper.children.length).toBe(0);
+});
+
+it('add container', () => {
+  document.body.appendChild(wrapper);
+
+  dom.removeContainer(container);
+  dom.addContainer(container, wrapper);
+
+  expect(wrapper.children.length).toBe(1);
+});
+
+it('add container with sibling', () => {
+  const sibling = document.createElement('div');
+
+  document.body.appendChild(wrapper);
+  wrapper.appendChild(sibling);
+
+  dom.removeContainer(container);
+  dom.addContainer(container, wrapper);
+
+  expect(wrapper.children.length).toBe(2);
+  expect(container.nextElementSibling).toBe(sibling);
+});
diff --git a/packages/core/src/modules/Transitions.ts b/packages/core/src/modules/Transitions.ts
@@ -26,7 +26,7 @@ import {
 // Hooks
 import { hooks } from '../hooks';
 // Utils
-import { helpers } from '../utils';
+import { dom, helpers } from '../utils';
 // Modules
 import { Logger } from './Logger';
 import { Store } from './Store';
@@ -281,20 +281,16 @@ export class Transitions {
    * Add next container.
    */
   public async add(data: ITransitionData, wrapper: Wrapper): Promise<void> {
-    wrapper.appendChild(data.next.container);
+    dom.addContainer(data.next.container, wrapper);
     hooks.do('nextAdded', data);
   }
 
   /**
    * Remove current container.
    */
   public async remove(data: ITransitionData): Promise<void> {
-    const { container } = data.current;
-
-    if (document.body.contains(container)) {
-      container.parentNode.removeChild(container);
-      hooks.do('currentRemoved', data);
-    }
+    dom.removeContainer(data.current.container);
+    hooks.do('currentRemoved', data);
   }
 
   /**

diff --git a/packages/core/src/utils/dom.ts b/packages/core/src/utils/dom.ts
@@ -20,6 +20,7 @@ import { schemaAttribute } from '../schemas/attribute';
 export class Dom {
   private _attr: ISchemaAttribute = schemaAttribute;
   private _parser: DOMParser = new DOMParser();
+  private _sibling: HTMLElement = null;
 
   /**
    * Convert HTMLDocument to string.
@@ -82,6 +83,27 @@ export class Dom {
     );
   }
 
+  /**
+   * Remove container and store next sibling (if applicable).
+   */
+  public removeContainer(container: HTMLElement) {
+    if (document.body.contains(container)) {
+      this._sibling = container.nextElementSibling as HTMLElement;
+      container.parentNode.removeChild(container);
+    }
+  }
+
+  /**
+   * Add container before next sibling or at the end of the wrapper.
+   */
+  public addContainer(container: HTMLElement, wrapper: HTMLElement) {
+    if (this._sibling) {
+      wrapper.insertBefore(container, this._sibling);
+    } else {
+      wrapper.appendChild(container);
+    }
+  }
+
   /**
    * Get `[data-barba-namespace]`.
    */