Skip to content

Commit

Permalink
fix(tooltip): fix infinite loop in self-managed tooltips (#4269)
Browse files Browse the repository at this point in the history
* fix(tooltip): fix infinite loop in self-managed tooltips

* chore: return null instead of document when no trigger is found

* chore: add a unit test to verify that event listeners are no longer attached to document

* chore: update tests to verify against triggerElement

---------

Co-authored-by: rmanole <rmanole@adobe.com>
  • Loading branch information
Rocss and rmanole committed Apr 29, 2024
1 parent 1df4c34 commit b66ee49
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 8 deletions.
14 changes: 7 additions & 7 deletions packages/tooltip/src/Tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,12 +205,12 @@ export class Tooltip extends SpectrumElement {
);
}

private get triggerElement(): HTMLElement {
private get triggerElement(): HTMLElement | null {
// Resolve the parent element of the assigned slot (if one exists) or of the Tooltip.
let start: HTMLElement = this.assignedSlot || this;
let root = start.getRootNode();
if (window.__swc.DEBUG) {
if (root === document) {
if (root === document) {
if (window.__swc.DEBUG) {
window.__swc.warn(
this,
`Self managed <${this.localName}> elements walk up the composed tree to acquire a trigger element. No trigger element was found before the document.`,
Expand All @@ -219,8 +219,8 @@ export class Tooltip extends SpectrumElement {
level: 'high',
}
);
return root as HTMLElement;
}
return null;
}
let triggerElement = (start.parentElement ||
(root as ShadowRoot).host ||
Expand All @@ -231,8 +231,8 @@ export class Tooltip extends SpectrumElement {
triggerElement.assignedSlot || (triggerElement as HTMLElement);
root = start.getRootNode();
/* c8 ignore next 13 */
if (window.__swc.DEBUG) {
if (root === document) {
if (root === document) {
if (window.__swc.DEBUG) {
window.__swc.warn(
this,
`Self managed <${this.localName}> elements walk up the composed tree to acquire a trigger element. No trigger element was found before the document.`,
Expand All @@ -241,8 +241,8 @@ export class Tooltip extends SpectrumElement {
level: 'high',
}
);
return root as HTMLElement;
}
return null;
}
triggerElement = (start.parentElement ||
(root as ShadowRoot).host ||
Expand Down
16 changes: 15 additions & 1 deletion packages/tooltip/test/tooltip.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
} from '@open-wc/testing';
import { Button } from '@spectrum-web-components/button';
import '@spectrum-web-components/button/sp-button.js';
import { stub } from 'sinon';
import { spy, stub } from 'sinon';
import { testForLitDevWarnings } from '../../../test/testing-helpers.js';
import { sendMouse } from '../../../test/plugins/browser.js';

Expand Down Expand Up @@ -214,6 +214,20 @@ describe('Tooltip', () => {

expect(typeof el.tipElement).to.not.equal('undefined');
});

it('self managed does not attach event listeners if no trigger was found', async () => {
const documentEventsSpy = spy(document, 'addEventListener');
const el = await fixture<Tooltip>(
html`
<sp-tooltip self-managed>Help text.</sp-tooltip>
`
);

await elementUpdated(el);
expect(documentEventsSpy.callCount).to.equal(0);
expect(el.overlayElement?.triggerElement).to.be.null;
documentEventsSpy.restore();
});
describe('dev mode', () => {
let consoleWarnStub!: ReturnType<typeof stub>;
before(() => {
Expand Down

0 comments on commit b66ee49

Please sign in to comment.