refactor(cdk/overlay): simplify popover insertion logic

This commit refactors the popover insertion logic in the CDK overlay by removing  and  in favor of a more flexible  API. This change simplifies the API and provides more granular control over where popovers are inserted into the DOM.

Author: tjshiu

goldens/cdk/overlay/index.api.md

@@ -284,7 +284,7 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
     // (undocumented)
     detach(): void;
     dispose(): void;
-    getPopoverInsertionPoint(): Element | null;
+    getPopoverInsertionPoint(): Element | null | {parent: Element};
     _origin: FlexibleConnectedPositionStrategyOrigin;
     positionChanges: Observable<ConnectedOverlayPositionChange>;
     get positions(): ConnectionPositionPair[];
@@ -536,7 +536,7 @@ export interface PositionStrategy {
     attach(overlayRef: OverlayRef): void;
     detach?(): void;
     dispose(): void;
-    getPopoverInsertionPoint?(): Element | null;
+    getPopoverInsertionPoint?(): Element | null | {parent: Element};
 }
 
 // @public

src/cdk/overlay/overlay-config.ts

@@ -67,13 +67,6 @@ export class OverlayConfig {
    */
   usePopover?: boolean;
 
-  /**
-   * Whether to attach the popover as a child of the popover host.
-   * If true, the popover will be attached as a child of the host.
-   * If false, the popover will be attached after the host.
-   */
-  attachPopoverAsChild?: boolean;
-
   constructor(config?: OverlayConfig) {
     if (config) {
       // Use `Iterable` instead of `Array` because TypeScript, as of 3.6.3,

src/cdk/overlay/overlay-directives.ts

@@ -261,22 +261,6 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
   @Input({alias: 'cdkConnectedOverlayMatchWidth', transform: booleanAttribute})
   matchWidth: boolean = false;
 
-  /**
-   * A custom element to use as the host for the popover.
-   * The popover will be inserted after this element in the DOM.
-   * If null, the overlay will be inserted after the origin.
-   */
-  @Input({alias: 'cdkCustomPopoverInsertionElement'})
-  customPopoverHostElement: CdkOverlayOrigin | FlexibleConnectedPositionStrategyOrigin | null;
-
-  /**
-   * Whether to attach the popover as a child of the popover host.
-   * If true, the popover will be attached as a child of the host.
-   * If false, the popover will be attached after the host.
-   */
-  @Input({alias: 'cdkAttachPopoverAsChild', transform: booleanAttribute})
-  attachPopoverAsChild: boolean = false;
-
   /** Shorthand for setting multiple overlay options at once. */
   @Input('cdkConnectedOverlay')
   set _config(value: string | CdkConnectedOverlayConfig) {
@@ -356,8 +340,6 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
     }
 
     if (changes['open']) {
-      console.log('changes[open]');
-      console.log(this.open);
       this.open ? this.attachOverlay() : this.detachOverlay();
     }
   }
@@ -401,7 +383,6 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
       hasBackdrop: this.hasBackdrop,
       disposeOnNavigation: this.disposeOnNavigation,
       usePopover: !!this.usePopover,
-      attachPopoverAsChild: this.attachPopoverAsChild,
     });
 
     if (this.height || this.height === 0) {
@@ -448,9 +429,7 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
       .withViewportMargin(this.viewportMargin)
       .withLockedPosition(this.lockPosition)
       .withTransformOriginOn(this.transformOriginSelector)
-      .withPopoverLocation(this.usePopover === 'global' ? 'global' : 'inline')
-      .withCustomPopoverHostElement(this._getCustomPopoverHostElement())
-      .withAttachPopoverAsChild(this.attachPopoverAsChild);
+      .withPopoverLocation(this.usePopover === null ? 'global' : this.usePopover);
   }
 
   /** Returns the position strategy of the overlay to be set on the overlay config */
@@ -468,18 +447,6 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
     }
   }
 
-  /**
-   * Gets the custom popover host element from the origin input.
-   * @docs-private
-   */
-  private _getCustomPopoverHostElement(): FlexibleConnectedPositionStrategyOrigin | null {
-    if (this.customPopoverHostElement instanceof CdkOverlayOrigin) {
-      return this.customPopoverHostElement.elementRef;
-    } else {
-      return this.customPopoverHostElement;
-    }
-  }
-
   private _getOriginElement(): Element | null {
     if (this.origin instanceof CdkOverlayOrigin) {
       return this.origin.elementRef.nativeElement;
@@ -579,9 +546,6 @@ export class CdkConnectedOverlay implements OnDestroy, OnChanges {
     this.push = config.push ?? this.push;
     this.disposeOnNavigation = config.disposeOnNavigation ?? this.disposeOnNavigation;
     this.usePopover = config.usePopover ?? this.usePopover;
-    this.customPopoverHostElement =
-      config.customPopoverHostElement ?? this.customPopoverHostElement;
-    this.attachPopoverAsChild = config.attachPopoverAsChild ?? this.attachPopoverAsChild;
     this.matchWidth = config.matchWidth ?? this.matchWidth;
   }
 }

src/cdk/overlay/overlay-ref.ts

@@ -410,9 +410,11 @@ export class OverlayRef implements PortalOutlet {
         : null;
 
       if (customInsertionPoint) {
-        this._config.attachPopoverAsChild
-          ? customInsertionPoint.appendChild(this._host)
-          : customInsertionPoint.after(this._host);
+        if (customInsertionPoint instanceof Element) {
+          customInsertionPoint.after(this._host);
+        } else {
+          customInsertionPoint.parent?.appendChild(this._host);
+        }
       } else {
         this._previousHostParent?.appendChild(this._host);
       }

src/cdk/overlay/overlay.ts

@@ -95,10 +95,10 @@ export function createOverlayRef(injector: Injector, config?: OverlayConfig): Ov
   // it's going to end up at the custom insertion point anyways. We need to do it,
   // because some internal clients depend on the host passing through the container first.
   if (customInsertionPoint) {
-    if (overlayConfig.attachPopoverAsChild) {
-      customInsertionPoint.appendChild(host);
-    } else {
+    if (customInsertionPoint instanceof Element) {
       customInsertionPoint.after(host);
+    } else {
+      customInsertionPoint.parent.appendChild(host);
     }
   }
 

src/cdk/overlay/position/flexible-connected-position-strategy.spec.ts

@@ -3019,16 +3019,16 @@ describe('FlexibleConnectedPositionStrategy', () => {
       expect(originElement.nextElementSibling).toBe(overlayRef.hostElement);
     });
 
-    it('should insert the overlay after a custom element', () => {
+    it('should insert the overlay as a child of a custom element', () => {
       if (!('showPopover' in document.body)) {
         return;
       }
 
-      positionStrategy.withCustomPopoverHostElement(customHostElement);
+      positionStrategy.withPopoverLocation({parent: customHostElement});
       attachOverlay({positionStrategy, usePopover: true});
 
       expect(containerElement.contains(overlayRef.hostElement)).toBe(false);
-      expect(customHostElement.nextElementSibling).toBe(overlayRef.hostElement);
+      expect(customHostElement.contains(overlayRef.hostElement)).toBe(true);
       expect(overlayRef.hostElement.getAttribute('popover')).toBe('manual');
     });
 
@@ -3037,8 +3037,8 @@ describe('FlexibleConnectedPositionStrategy', () => {
         return;
       }
 
-      console.log(positionStrategy.getPopoverInsertionPoint());
-      attachOverlay({positionStrategy, usePopover: true, attachPopoverAsChild: true});
+      positionStrategy.withPopoverLocation({parent: originElement});
+      attachOverlay({positionStrategy, usePopover: true});
 
       expect(containerElement.contains(overlayRef.hostElement)).toBe(false);
       expect(originElement.contains(overlayRef.hostElement)).toBe(true);

src/cdk/overlay/position/flexible-connected-position-strategy.ts

@@ -45,6 +45,9 @@ export type FlexibleConnectedPositionStrategyOrigin =
 /** Equivalent of `DOMRect` without some of the properties we don't care about. */
 type Dimensions = Omit<DOMRect, 'x' | 'y' | 'toJSON'>;
 
+/** Possible point to attach a popover to. */
+export type PopoverInsertionPoint = Element | {parent: Element} | null;
+
 /**
  * Creates a flexible position strategy.
  * @param injector Injector used to resolve dependnecies for the position strategy.
@@ -64,7 +67,10 @@ export function createFlexibleConnectedPositionStrategy(
 }
 
 /** Supported locations in the DOM for connected overlays. */
-export type FlexibleOverlayPopoverLocation = 'global' | 'inline';
+export type FlexibleOverlayPopoverLocation =
+  | 'global'
+  | 'inline'
+  | {parent: FlexibleConnectedPositionStrategyOrigin};
 
 /**
  * A strategy for positioning overlays. Using this strategy, an overlay is given an
@@ -164,18 +170,6 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
   /** Configures where in the DOM to insert the overlay when popovers are enabled. */
   private _popoverLocation: FlexibleOverlayPopoverLocation = 'global';
 
-  /**
-   * Defines a specific host element for the popover content. If provided, the popover will attach
-   * to this element.
-   * */
-  private _customPopoverHostElement: FlexibleConnectedPositionStrategyOrigin | null;
-
-  /**
-   * Whether the popover is attached directly as a child of the popover host element instead of
-   * a sibling element.
-   * */
-  private _attachPopoverAsChild = false;
-
   /** Observable sequence of position changes. */
   positionChanges: Observable<ConnectedOverlayPositionChange> = this._positionChanges;
 
@@ -540,44 +534,35 @@ export class FlexibleConnectedPositionStrategy implements PositionStrategy {
     return this;
   }
 
-  /**
-   * Sets a custom element to use as the host for the popover.
-   * The popover will be inserted after this element in the DOM.
-   * If null, the overlay will be inserted after the origin.
-   * @param element The element to use as the host for the popover.
-   */
-  withCustomPopoverHostElement(element: FlexibleConnectedPositionStrategyOrigin | null): this {
-    this._customPopoverHostElement = element;
-    return this;
-  }
-
   /** @docs-private */
-  getPopoverInsertionPoint(): Element | null {
-    // Return null so it falls back to inserting into the overlay container.
+  getPopoverInsertionPoint(): PopoverInsertionPoint {
     if (this._popoverLocation === 'global') {
       return null;
     }
 
     const origin =
-      this._customPopoverHostElement != null ? this._customPopoverHostElement : this._origin;
+      this._popoverLocation === 'inline'
+        ? this._origin
+        : (this._popoverLocation as {parent: FlexibleConnectedPositionStrategyOrigin}).parent;
+    let element: Element | null = null;
 
     if (origin instanceof ElementRef) {
-      return origin.nativeElement;
+      element = origin.nativeElement;
     } else if (origin instanceof Element) {
-      return origin;
+      element = origin;
     }
-    return null;
-  }
 
-  /**
-   * Whether to attach the popover as a child of the popover host.
-   * If true, the popover will be attached as a child of the host.
-   * If false, the popover will be attached after the host.
-   * @param attachPopoverAsChild Whether to attach the popover as a child of the popover host.
-   */
-  withAttachPopoverAsChild(attachPopoverAsChild = false): this {
-    this._attachPopoverAsChild = attachPopoverAsChild;
-    return this;
+    // If the location is 'inline', we're inserting as a sibling.
+    if (this._popoverLocation === 'inline') {
+      return element;
+    }
+
+    // Otherwise we're inserting as a child.
+    if (element) {
+      return {parent: element};
+    }
+
+    return null;
   }
 
   /**

src/cdk/overlay/position/position-strategy.ts

@@ -26,5 +26,5 @@ export interface PositionStrategy {
    * Gets the element in the DOM after which to insert
    * the overlay when it is rendered out as a popover.
    */
-  getPopoverInsertionPoint?(): Element | null;
+  getPopoverInsertionPoint?(): Element | null | {parent: Element};
 }