From 4251d13c1a035bdd4046dc39ef0ea1ec15203e9b Mon Sep 17 00:00:00 2001 From: John Yanarella Date: Mon, 26 Oct 2026 15:44:45 -0700 Subject: [PATCH] fix(core): observe only host hidden changes for popover visibility Signed-off-by: John Yanarella --- .../type-native-popover.controller.test.ts | 93 +++++++++++++++++++ .../type-native-popover.controller.ts | 24 ++--- projects/core/src/tooltip/tooltip.test.ts | 37 ++++++++ 3 files changed, 142 insertions(+), 12 deletions(-) diff --git a/projects/core/src/internal/controllers/type-native-popover.controller.test.ts b/projects/core/src/internal/controllers/type-native-popover.controller.test.ts index 6cb142d8c6..34da5c5ddb 100644 --- a/projects/core/src/internal/controllers/type-native-popover.controller.test.ts +++ b/projects/core/src/internal/controllers/type-native-popover.controller.test.ts @@ -1308,6 +1308,99 @@ describe('type-popover.controller - dynamic DOM creation', () => { }); }); +describe('type-popover.controller - host visibility observation', () => { + let fixture: HTMLElement; + let element: TypeNativePopoverControllerTestElement; + let child: HTMLElement; + + beforeEach(async () => { + fixture = await createFixture(html` + anchor + + `); + const popover = fixture.querySelector( + 'type-native-popover-controller-test-element' + ); + const content = popover?.querySelector('span'); + if (!popover || !content) throw new Error('Missing popover fixture'); + element = popover; + child = content; + await elementIsStable(element); + await new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(resolve))); + }); + + afterEach(() => { + removeFixture(fixture); + vi.restoreAllMocks(); + }); + + it.each([false, true])('should ignore descendant visibility changes when open is %s', async open => { + if (open) element.showPopover(); + await elementIsStable(element); + const show = vi.spyOn(element, 'showPopover'); + const hide = vi.spyOn(element, 'hidePopover'); + const matches = vi.spyOn(element, 'matches'); + + child.hidden = true; + await elementIsStable(element); + child.hidden = false; + await elementIsStable(element); + + expect(show).not.toHaveBeenCalled(); + expect(hide).not.toHaveBeenCalled(); + expect(matches).not.toHaveBeenCalledWith(':popover-open'); + expect(element.matches(':popover-open')).toBe(open); + }); + + it('should stay closed after a native hide when descendants change visibility', async () => { + element.showPopover(); + await elementIsStable(element); + element.hidePopover(); + await elementIsStable(element); + expect(element.hidden).toBe(false); + + child.hidden = true; + await elementIsStable(element); + + expect(element.matches(':popover-open')).toBe(false); + }); + + it('should preserve host hidden visibility commands after reconnection', async () => { + element.remove(); + fixture.append(element); + await elementIsStable(element); + await new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(resolve))); + const show = vi.spyOn(element, 'showPopover'); + const hide = vi.spyOn(element, 'hidePopover'); + + element.hidden = true; + await elementIsStable(element); + element.hidden = false; + await elementIsStable(element); + expect(element.matches(':popover-open')).toBe(true); + expect(show).toHaveBeenCalledTimes(1); + + element.hidden = true; + await elementIsStable(element); + expect(element.matches(':popover-open')).toBe(false); + expect(hide).toHaveBeenCalledTimes(1); + }); + + it('should disconnect visibility observation', async () => { + element.remove(); + const show = vi.spyOn(element, 'showPopover'); + const hide = vi.spyOn(element, 'hidePopover'); + + element.hidden = true; + await elementIsStable(element); + element.hidden = false; + await elementIsStable(element); + + expect(show).not.toHaveBeenCalled(); + expect(hide).not.toHaveBeenCalled(); + }); +}); + describe('type-popover.controller - interest timeout cleanup on disconnect', () => { it('should cancel pending interest timeout when element is disconnected', async () => { const fixture = await createFixture(html` diff --git a/projects/core/src/internal/controllers/type-native-popover.controller.ts b/projects/core/src/internal/controllers/type-native-popover.controller.ts index adc77b7bf8..f7217d1285 100644 --- a/projects/core/src/internal/controllers/type-native-popover.controller.ts +++ b/projects/core/src/internal/controllers/type-native-popover.controller.ts @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import type { ReactiveController, ReactiveElement } from 'lit'; -import { clickOutsideElementBounds, generateId, getAttributeListChanges } from '../utils/dom.js'; +import { clickOutsideElementBounds, generateId } from '../utils/dom.js'; import { attachInternals } from '../utils/a11y.js'; import { focusElement } from '../utils/focus.js'; import { getHostAnchor, getHostTrigger, hasOpenPopover } from './type-native-popover.utils.js'; @@ -164,18 +164,18 @@ export class TypeNativePopoverController implements Rea async #setupLegacyTriggers() { await new Promise(r => requestAnimationFrame(r)); - // setup hidden updates for legacy triggers - this.#observers.push( - getAttributeListChanges(this.host, ['hidden'], () => { - if (this.host.isConnected && !this.host.hidden && !this.host.matches(':popover-open')) { - this.host.showPopover(); - } + // setup host hidden updates for legacy triggers + const observer = new MutationObserver(() => { + if (this.host.isConnected && !this.host.hidden && !this.host.matches(':popover-open')) { + this.host.showPopover(); + } - if (this.host.isConnected && this.host.hidden && this.host.matches(':popover-open')) { - this.host.hidePopover(); - } - }) - ); + if (this.host.isConnected && this.host.hidden && this.host.matches(':popover-open')) { + this.host.hidePopover(); + } + }); + observer.observe(this.host, { attributes: true, attributeFilter: ['hidden'] }); + this.#observers.push(observer); // determine if popover is open by default if ( diff --git a/projects/core/src/tooltip/tooltip.test.ts b/projects/core/src/tooltip/tooltip.test.ts index f0d5157e0a..3f81e0f421 100644 --- a/projects/core/src/tooltip/tooltip.test.ts +++ b/projects/core/src/tooltip/tooltip.test.ts @@ -96,6 +96,43 @@ describe(Tooltip.metadata.tag, () => { vi.advanceTimersByTime(1); expect(element.matches(':popover-open')).toBe(true); }); + + it('should stay dismissed when tooltip content changes visibility', async () => { + await new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(resolve))); + const child = document.createElement('span'); + element.append(child); + + const open = untilEvent(element, 'open'); + element.showPopover(); + await open; + const close = untilEvent(element, 'close'); + element.hidePopover(); + await close; + expect(element.hidden).toBe(false); + + child.hidden = true; + await elementIsStable(element); + child.hidden = false; + await elementIsStable(element); + + expect(element.matches(':popover-open')).toBe(false); + }); + + it('should preserve legacy host hidden visibility commands', async () => { + element.hidden = true; + await new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(resolve))); + expect(element.matches(':popover-open')).toBe(false); + + const open = untilEvent(element, 'open'); + element.hidden = false; + await open; + expect(element.matches(':popover-open')).toBe(true); + + const close = untilEvent(element, 'close'); + element.hidden = true; + await close; + expect(element.matches(':popover-open')).toBe(false); + }); }); describe(`${Tooltip.metadata.tag}: popover interest handover`, () => {