Fix popover scroll parent detection in nested shadow DOM containers

packages/uui-popover-container/lib/uui-popover-container.element.ts

@@ -72,14 +72,20 @@ export class UUIPopoverContainerElement extends LitElement {
   #sizeObserver: ResizeObserver | null = null;
   #size: { width: number; height: number } = { width: 0, height: 0 };
 
+  constructor() {
+    super();
+
+    this.addEventListener('beforetoggle', this.#onBeforeToggle, {
+      passive: true,
+    });
+  }
+
   connectedCallback(): void {
+    super.connectedCallback();
     if (!this.hasAttribute('popover')) {
       this.setAttribute('popover', '');
     }
 
-    super.connectedCallback();
-    this.addEventListener('beforetoggle', this.#onBeforeToggle);
-
     if (!this.#sizeObserver) {
       this.#sizeObserver = new ResizeObserver(entries => {
         const element = entries[0]; // should be only one
@@ -101,7 +107,6 @@ export class UUIPopoverContainerElement extends LitElement {
 
   disconnectedCallback(): void {
     super.disconnectedCallback();
-    this.removeEventListener('beforetoggle', this.#onBeforeToggle);
     this.#stopScrollListener();
     this.#sizeObserver?.disconnect();
     this.#sizeObserver = null;
@@ -116,8 +121,6 @@ export class UUIPopoverContainerElement extends LitElement {
       this.id,
     );
 
-    this.#getScrollParents();
-
     // Dispatch a custom event that can be listened to by the popover target.
     // Mostly used for UUIButton.
     this.#targetElement?.dispatchEvent(
@@ -131,16 +134,17 @@ export class UUIPopoverContainerElement extends LitElement {
       }),
     );
 
-    if (!this._open) {
-      this.#stopScrollListener();
-      return;
-    }
+    if (this._open) {
+      this.#calculateScrollParents();
 
-    this.#startScrollListener();
+      this.#startScrollListener();
 
-    requestAnimationFrame(() => {
-      this.#initUpdate();
-    });
+      requestAnimationFrame(() => {
+        this.#initUpdate();
+      });
+    } else {
+      this.#stopScrollListener();
+    }
   };
 
   #initUpdate = () => {
@@ -340,7 +344,17 @@ export class UUIPopoverContainerElement extends LitElement {
     document.removeEventListener('scroll', this.#initUpdate);
   }
 
-  #getScrollParents(): any {
+  /**
+   * @internal
+   */
+  _getScrollParents() {
+    return this.#scrollParents;
+  }
+
+  #calculateScrollParents(): void {
+    // Clear previous scroll parents to avoid duplicates
+    this.#scrollParents = [];
+
     if (!this.#targetElement) return;
 
     let style = getComputedStyle(this.#targetElement);
@@ -349,7 +363,7 @@ export class UUIPopoverContainerElement extends LitElement {
     }
 
     const includeHidden = false;
-    const excludeStaticParent = style.position === 'absolute';
+    let excludeStaticParent = style.position === 'absolute';
     const overflowRegex = includeHidden
       ? /(auto|scroll|hidden)/
       : /(auto|scroll)/;
@@ -362,6 +376,11 @@ export class UUIPopoverContainerElement extends LitElement {
         el = this.#getAncestorElement(el);
         continue;
       }
+
+      if (style.position !== 'static') {
+        excludeStaticParent = style.position === 'absolute';
+      }
+
       if (
         overflowRegex.test(style.overflow + style.overflowY + style.overflowX)
       ) {
@@ -379,10 +398,10 @@ export class UUIPopoverContainerElement extends LitElement {
   #getAncestorElement(el: HTMLElement | null): HTMLElement | null {
     if (el?.parentElement) {
       return el.parentElement;
-    } else {
-      // If we had no parentElement, then check for shadow roots:
-      return (el?.getRootNode() as any)?.host;
     }
+
+    // If we had no parentElement, then check for shadow roots:
+    return (el?.getRootNode() as any)?.host;
   }
 
   render() {

packages/uui-popover-container/lib/uui-popover-container.story.ts

@@ -9,6 +9,28 @@ const meta: Meta<UUIPopoverContainerElement> = {
   id: 'uui-popover-container',
   component: 'uui-popover-container',
   title: 'Displays/Popover Container',
+  argTypes: {
+    open: {
+      control: false,
+    },
+    placement: {
+      options: [
+        'auto',
+        'top',
+        'top-start',
+        'top-end',
+        'bottom',
+        'bottom-start',
+        'bottom-end',
+        'right',
+        'right-start',
+        'right-end',
+        'left',
+        'left-start',
+        'left-end',
+      ],
+    },
+  },
   parameters: {
     readme: {
       markdown: readme,
@@ -32,28 +54,6 @@ export const Overview: Story = {
     placement: 'bottom-start',
     margin: 0,
   },
-  argTypes: {
-    open: {
-      control: false,
-    },
-    placement: {
-      options: [
-        'auto',
-        'top',
-        'top-start',
-        'top-end',
-        'bottom',
-        'bottom-start',
-        'bottom-end',
-        'right',
-        'right-start',
-        'right-end',
-        'left',
-        'left-start',
-        'left-end',
-      ],
-    },
-  },
   render: args => html`
     <uui-button
       id="popover-button"
@@ -119,28 +119,6 @@ export const Tooltip: Story = {
     placement: 'bottom-start',
     margin: 0,
   },
-  argTypes: {
-    open: {
-      control: false,
-    },
-    placement: {
-      options: [
-        'auto',
-        'top',
-        'top-start',
-        'top-end',
-        'bottom',
-        'bottom-start',
-        'bottom-end',
-        'right',
-        'right-start',
-        'right-end',
-        'left',
-        'left-start',
-        'left-end',
-      ],
-    },
-  },
   render: args => html`
     Sometimes words such as
     <b id="tooltip-toggle" popovertarget="tooltip-popover">petrichor</b> needs
@@ -164,28 +142,6 @@ export const InsideScrollContainer: Story = {
     placement: 'bottom-start',
     margin: 0,
   },
-  argTypes: {
-    open: {
-      control: false,
-    },
-    placement: {
-      options: [
-        'auto',
-        'top',
-        'top-start',
-        'top-end',
-        'bottom',
-        'bottom-start',
-        'bottom-end',
-        'right',
-        'right-start',
-        'right-end',
-        'left',
-        'left-start',
-        'left-end',
-      ],
-    },
-  },
   render: args => html`
     <div style="height: 500px; overflow: auto; outline: 1px solid black">
       <div
@@ -222,15 +178,10 @@ export const InsideShadowDOMScrollContainer: Story = {
     placement: 'bottom-start',
     margin: 0,
   },
-  argTypes: {
-    open: {
-      control: false,
-    },
-  },
   render: () => html`
     <div style="height: 500px; overflow: auto; outline: 1px solid black">
       <div
-        style="position:static; width: 300px; height: 300px; outline: 1px solid black; overflow: auto;">
+        style="position:static; width: 300px; height: 300px; outline: 1px solid black; overflow: auto; margin:auto">
         <div style="height: 150px"></div>
         <uui-popover-container-shadowdomtester></uui-popover-container-shadowdomtester>
         <div style="height: 150px"></div>

packages/uui-popover-container/lib/uui-popover-container.test.ts

@@ -1,5 +1,17 @@
-import { html, fixture, expect } from '@open-wc/testing';
+import { html, fixture, expect, aTimeout } from '@open-wc/testing';
 import { UUIPopoverContainerElement } from './uui-popover-container.element';
+import '@umbraco-ui/uui-button/lib';
+
+class DummyElementWithShadowDom extends HTMLElement {
+  constructor() {
+    super();
+    this.attachShadow({ mode: 'open' });
+    this.shadowRoot!.innerHTML = `<div style="height: 200px; overflow: auto;" id="scrollable-div">
+        <slot></slot>
+      </div>`;
+  }
+}
+customElements.define('dummy-shadow-dom', DummyElementWithShadowDom);
 
 describe('UUIPopoverContainerElement', () => {
   let element: UUIPopoverContainerElement;
@@ -24,4 +36,125 @@ describe('UUIPopoverContainerElement', () => {
     await element.updateComplete;
     expect(element).to.have.attribute('popover');
   });
+
+  describe('scroll parent detection', () => {
+    it('should properly detect scroll parents in nested shadow DOM containers', async () => {
+      // Create a test structure with nested shadow DOM and scroll containers
+      const testContainer = await fixture(html`
+        <main>
+          <div style="height: 300px; overflow: auto;" id="outer-scroll">
+            <dummy-shadow-dom>
+              <div style="height: 200px; overflow: auto;" id="inner-scroll">
+                <div style="height: 100px;"></div>
+                <uui-button
+                  id="trigger-button"
+                  popovertarget="test-popover"
+                  label="Open"></uui-button>
+                <div style="height: 400px;"></div>
+              </div>
+            </dummy-shadow-dom>
+            <div style="height: 500px;"></div>
+          </div>
+          <uui-popover-container id="test-popover" popover>
+            Test content
+          </uui-popover-container>
+        </main>
+      `);
+
+      const popover = testContainer.querySelector(
+        '#test-popover',
+      ) as UUIPopoverContainerElement;
+      const button = testContainer.querySelector('#trigger-button');
+
+      // Trigger the popover open
+      button?.click();
+      await aTimeout(100);
+
+      // Access the private scroll parents array for testing
+      const scrollParents = popover._getScrollParents();
+
+      // Should find all scroll containers
+      expect(scrollParents.length).to.be.equal(3); // outer-scroll, inner-scroll, document.body
+
+      // Should include the document.body as the last element
+      expect(scrollParents[scrollParents.length - 1]).to.equal(document.body);
+    });
+
+    it('should ignore scroll parents with position: absolute', async () => {
+      // Create a test structure with nested shadow DOM and scroll containers
+      const testContainer = await fixture(html`
+        <main>
+          <div style="height: 300px; overflow: auto;" id="outer-scroll">
+            <div style="position: absolute; top:0; left:0;" id="inner-scroll">
+              <div style="height: 100px;"></div>
+              <uui-button
+                id="trigger-button"
+                popovertarget="test-popover"
+                label="Open"></uui-button>
+              <div style="height: 400px;"></div>
+            </div>
+            <div style="height: 500px;"></div>
+          </div>
+          <uui-popover-container id="test-popover" popover>
+            Test content
+          </uui-popover-container>
+        </main>
+      `);
+
+      const popover = testContainer.querySelector(
+        '#test-popover',
+      ) as UUIPopoverContainerElement;
+      const innerScroll = testContainer.querySelector(
+        '#inner-scroll',
+      ) as HTMLElement;
+      const button = testContainer.querySelector('#trigger-button');
+
+      // Trigger the popover open
+      button?.click();
+      await aTimeout(100);
+
+      // Access the private scroll parents array for testing
+      const scrollParents = popover._getScrollParents();
+
+      // Should not contain the inner scroll since it's position: absolute
+      expect(scrollParents).to.not.include(innerScroll);
+    });
+
+    it('should reset scroll parents when called multiple times', async () => {
+      const testContainer = await fixture(html`
+        <main>
+          <div style="height: 300px; overflow: auto;" id="scroll-container">
+            <uui-button
+              id="trigger-button"
+              popovertarget="test-popover"
+              label="Open"></uui-button>
+            <div style="height: 400px;"></div>
+          </div>
+          <uui-popover-container id="test-popover" popover>
+            Test content
+          </uui-popover-container>
+        </main>
+      `);
+
+      const popover = testContainer.querySelector(
+        '#test-popover',
+      ) as UUIPopoverContainerElement;
+      const button = testContainer.querySelector('#trigger-button');
+
+      // Open and close the popover multiple times
+      button?.click();
+      await aTimeout(50);
+      popover.hidePopover();
+      await aTimeout(50);
+
+      button?.click();
+      await aTimeout(50);
+
+      const scrollParents = popover._getScrollParents();
+
+      // Should not have duplicate entries
+      const uniqueParents = [...new Set(scrollParents)];
+      expect(scrollParents.length).to.equal(uniqueParents.length);
+    });
+  });
 });