feat: reparentChildren - refactored arguments - breaking change

Author: sorinsarca

packages/menu/src/MenuItem.ts

@@ -387,14 +387,17 @@ export class MenuItem extends LikeAnchor(Focusable) {
         );
         submenu.addEventListener('change', this.handleSubmenuChange);
         const popover = document.createElement('sp-popover');
-        const returnSubmenu = reparentChildren([submenu], popover, (el) => {
-            const slotName = el.slot;
-            el.tabIndex = 0;
-            el.removeAttribute('slot');
-            return (el) => {
-                el.tabIndex = -1;
-                el.slot = slotName;
-            };
+        const returnSubmenu = reparentChildren([submenu], popover, {
+            position: 'beforeend',
+            prepareCallback: (el) => {
+                const slotName = el.slot;
+                el.tabIndex = 0;
+                el.removeAttribute('slot');
+                return (el) => {
+                    el.tabIndex = -1;
+                    el.slot = slotName;
+                };
+            },
         });
         const closeOverlay = openOverlay(this, 'click', popover, {
             placement: this.isLTR ? 'right-start' : 'left-start',

packages/overlay/src/ActiveOverlay.ts

@@ -358,14 +358,17 @@ export class ActiveOverlay extends SpectrumElement {
         element: HTMLElement & { placement: Placement }
     ): void {
         this.originalPlacement = element.getAttribute('placement') as Placement;
-        this.restoreContent = reparentChildren([element], this, (el) => {
-            const slotName = el.slot;
-            const placement = el.placement;
-            el.removeAttribute('slot');
-            return (el) => {
-                el.slot = slotName;
-                el.placement = placement;
-            };
+        this.restoreContent = reparentChildren([element], this, {
+            position: 'beforeend',
+            prepareCallback: (el) => {
+                const slotName = el.slot;
+                const placement = el.placement;
+                el.removeAttribute('slot');
+                return (el) => {
+                    el.slot = slotName;
+                    el.placement = placement;
+                };
+            },
         });
         this.stealOverlayContentResolver();
     }

packages/picker/src/Picker.ts

@@ -314,12 +314,15 @@ export class PickerBase extends SizedMixin(Focusable) {
 
         this.restoreChildren = reparentChildren<
             Element & { focused?: boolean }
-        >(reparentableChildren, this.optionsMenu, () => {
-            return (el) => {
-                if (typeof el.focused !== 'undefined') {
-                    el.focused = false;
-                }
-            };
+        >(reparentableChildren, this.optionsMenu, {
+            position: 'beforeend',
+            prepareCallback: () => {
+                return (el) => {
+                    if (typeof el.focused !== 'undefined') {
+                        el.focused = false;
+                    }
+                };
+            },
         });
 
         this.sizePopover(this.popover);

packages/shared/src/reparent-children.ts

@@ -32,40 +32,54 @@ function restoreChildren<T extends Element>(
 
 export const reparentChildren = <T extends Element>(
     srcElements: T[],
-    newParent: Element,
-    prepareCallback?: ((el: T) => ((el: T) => void) | void) | null,
-    position?: InsertPosition
-): (() => Element[]) => {
-    const placeholderItems: Comment[] = [];
-    const cleanupCallbacks: ((el: T) => void)[] = [];
+    destination: Element,
+    {
+        position,
+        prepareCallback,
+    }: {
+        position: InsertPosition;
+        prepareCallback?: (el: T) => ((el: T) => void) | void;
+    } = { position: 'beforeend' }
+): (() => T[]) => {
+    let length = srcElements.length;
+    if (length === 0) {
+        return () => srcElements;
+    }
 
-    // default is to append
-    position = position || 'beforeend';
+    let step = 1;
+    let index = 0;
 
     if (position === 'afterbegin' || position === 'afterend') {
-        srcElements.reverse();
+        step = -1;
+        index = length - 1;
     }
 
-    for (let index = 0; index < srcElements.length; ++index) {
+    const placeholderItems = new Array<Comment>(length);
+    const cleanupCallbacks = new Array<(el: T) => void>(length);
+    const placeholderTemplate: Comment = document.createComment(
+        'placeholder for reparented element'
+    );
+
+    do {
         const srcElement = srcElements[index];
         if (prepareCallback) {
-            cleanupCallbacks.push(
-                prepareCallback(srcElement) as (el: T) => void
-            );
+            cleanupCallbacks[index] = prepareCallback(srcElement) as (
+                el: T
+            ) => void;
         }
-        const placeholderItem: Comment = document.createComment(
-            'placeholder for reparented element'
-        );
-        placeholderItems.push(placeholderItem);
+        placeholderItems[index] = placeholderTemplate.cloneNode() as Comment;
+
         const parentElement =
             srcElement.parentElement || srcElement.getRootNode();
         if (parentElement && parentElement !== srcElement) {
-            parentElement.replaceChild(placeholderItem, srcElement);
+            parentElement.replaceChild(placeholderItems[index], srcElement);
         }
-        newParent.insertAdjacentElement(position, srcElement);
-    }
+        destination.insertAdjacentElement(position, srcElement);
+
+        index += step;
+    } while (--length > 0);
 
-    return function (): Element[] {
+    return function (): T[] {
         return restoreChildren<T>(
             placeholderItems,
             srcElements,

packages/shared/test/reparent-children.test.ts

@@ -90,17 +90,16 @@ describe('Reparent Children', () => {
         ) as HTMLDivElement;
 
         expect(child.getAttribute('slot')).to.equal('slot');
-        const restore = reparentChildren(
-            [child],
-            destination,
-            (el: Element) => {
+        const restore = reparentChildren([child], destination, {
+            position: 'beforeend',
+            prepareCallback: (el: Element) => {
                 const slotName = el.slot;
                 el.removeAttribute('slot');
                 return (el: Element) => {
                     el.slot = slotName;
                 };
-            }
-        );
+            },
+        });
 
         expect(child.hasAttribute('slot')).to.be.false;
 
@@ -132,12 +131,9 @@ describe('Reparent Children', () => {
 
         expect(source.children.length).to.equal(5);
         expect(destination.children.length).to.equal(1);
-        const restore = reparentChildren(
-            [...children],
-            destination,
-            null,
-            'beforeend'
-        );
+        const restore = reparentChildren([...children], destination, {
+            position: 'beforeend',
+        });
 
         expect(source.children.length).to.equal(0);
         expect(destination.children.length).to.equal(5 + 1);
@@ -179,12 +175,9 @@ describe('Reparent Children', () => {
 
         expect(source.children.length).to.equal(5);
         expect(destination.children.length).to.equal(1);
-        const restore = reparentChildren(
-            [...children],
-            destination,
-            null,
-            'afterbegin'
-        );
+        const restore = reparentChildren([...children], destination, {
+            position: 'afterbegin',
+        });
 
         expect(source.children.length).to.equal(0);
         expect(destination.children.length).to.equal(5 + 1);
@@ -224,12 +217,9 @@ describe('Reparent Children', () => {
         ) as HTMLDivElement;
 
         expect(source.children.length).to.equal(5);
-        const restore = reparentChildren(
-            [...children],
-            destination,
-            null,
-            'beforebegin'
-        );
+        const restore = reparentChildren([...children], destination, {
+            position: 'beforebegin',
+        });
 
         expect(source.children.length).to.equal(0);
         expect(destination.children.length).to.equal(0);
@@ -274,12 +264,9 @@ describe('Reparent Children', () => {
         expect(marker.previousElementSibling).to.equal(destination);
         expect(marker.nextElementSibling).to.be.null;
 
-        const restore = reparentChildren(
-            [...children],
-            destination,
-            null,
-            'afterend'
-        );
+        const restore = reparentChildren([...children], destination, {
+            position: 'afterend',
+        });
 
         expect(source.children.length).to.equal(0);
         expect(destination.children.length).to.equal(0);