diff --git a/packages/uui-popover-container/lib/uui-popover-container.element.ts b/packages/uui-popover-container/lib/uui-popover-container.element.ts index 4c8590261..16de69597 100644 --- a/packages/uui-popover-container/lib/uui-popover-container.element.ts +++ b/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() { diff --git a/packages/uui-popover-container/lib/uui-popover-container.story.ts b/packages/uui-popover-container/lib/uui-popover-container.story.ts index dc94ff36f..6f85210bc 100644 --- a/packages/uui-popover-container/lib/uui-popover-container.story.ts +++ b/packages/uui-popover-container/lib/uui-popover-container.story.ts @@ -9,6 +9,28 @@ const meta: Meta = { 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` html` Sometimes words such as petrichor 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`
html`
+ style="position:static; width: 300px; height: 300px; outline: 1px solid black; overflow: auto; margin:auto">
diff --git a/packages/uui-popover-container/lib/uui-popover-container.test.ts b/packages/uui-popover-container/lib/uui-popover-container.test.ts index a2ad641c8..b6e960e62 100644 --- a/packages/uui-popover-container/lib/uui-popover-container.test.ts +++ b/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 = `
+ +
`; + } +} +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` +
+
+ +
+
+ +
+
+ +
+
+ + Test content + +
+ `); + + 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` +
+
+
+
+ +
+
+
+
+ + Test content + +
+ `); + + 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` +
+
+ +
+
+ + Test content + +
+ `); + + 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); + }); + }); }); diff --git a/packages/uui-popover/lib/uui-popover.element.ts b/packages/uui-popover/lib/uui-popover.element.ts index 04791e922..83837772f 100644 --- a/packages/uui-popover/lib/uui-popover.element.ts +++ b/packages/uui-popover/lib/uui-popover.element.ts @@ -165,6 +165,9 @@ export class UUIPopoverElement extends LitElement { } private _getScrollParents(): any { + // Clear previous scroll parents to avoid duplicates + this._scrollParents = []; + const hostElement = this.shadowRoot!.host; let style = getComputedStyle(hostElement); if (style.position === 'fixed') { @@ -177,11 +180,12 @@ export class UUIPopoverElement extends LitElement { ? /(auto|scroll|hidden)/ : /(auto|scroll)/; - let el = hostElement; - while ((el = el.parentElement as Element)) { + let el: Element | null = hostElement; + while (el) { style = getComputedStyle(el); if (excludeStaticParent && style.position === 'static') { + el = this._getAncestorElement(el); continue; } if ( @@ -192,10 +196,21 @@ export class UUIPopoverElement extends LitElement { if (style.position === 'fixed') { return; } + + el = this._getAncestorElement(el); } this._scrollParents.push(document.body); } + private _getAncestorElement(el: Element | null): Element | null { + if (el?.parentElement) { + return el.parentElement; + } else { + // If we had no parentElement, then check for shadow roots: + return (el?.getRootNode() as any)?.host; + } + } + private _createIntersectionObserver() { if (this.intersectionObserver) { // break out, as we already have it