Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions packages/tooltip/src/vaadin-tooltip-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -553,17 +553,11 @@ export const TooltipMixin = (superClass) =>
}

/** @private */
__onMouseDown(event) {
__onMouseDown() {
if (this.manual) {
return;
}

// Do not close on bubbling mousedown events when
// the tooltip is slotted into the target element
if (event.composedPath().includes(this)) {
return;
}

this._stateController.close(true);
}

Expand Down Expand Up @@ -617,6 +611,20 @@ export const TooltipMixin = (superClass) =>
}
}

/** @protected */
__onOverlayMouseDown(event) {
// Prevent mousedown listeners from being called when
// the tooltip is slotted into the target element
event.stopPropagation();
}

/** @protected */
__onOverlayClick(event) {
// Prevent click listeners from being called when
// the tooltip is slotted into the target element
event.stopPropagation();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The button still triggers its click animation when clicking the tooltip. Maybe it would be good to fix that as well.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed. Changed to stop propagation for mousedown too - it's a better fix than the previous one. The active state used by click animation is set on mousedown event in ActiveMixin so stopping it makes sense.

}

/** @private */
__handleMouseLeave() {
if (this.manual) {
Expand Down
2 changes: 2 additions & 0 deletions packages/tooltip/src/vaadin-tooltip.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ class Tooltip extends TooltipMixin(ThemePropertyMixin(ElementMixin(PolylitMixin(
?no-vertical-overlap="${this.__computeNoVerticalOverlap(effectivePosition)}"
.horizontalAlign="${this.__computeHorizontalAlign(effectivePosition)}"
.verticalAlign="${this.__computeVerticalAlign(effectivePosition)}"
@click="${this.__onOverlayClick}"
@mousedown="${this.__onOverlayMouseDown}"
@mouseenter="${this.__onOverlayMouseEnter}"
@mouseleave="${this.__onOverlayMouseLeave}"
modeless
Expand Down
22 changes: 22 additions & 0 deletions test/integration/component-tooltip.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { expect } from '@vaadin/chai-plugins';
import { resetMouse, sendKeys, sendMouseToElement } from '@vaadin/test-runner-commands';
import { fixtureSync, mousedown, nextRender, tabKeyDown } from '@vaadin/testing-helpers';
import sinon from 'sinon';
import './not-animated-styles.js';
import { AccordionPanel } from '@vaadin/accordion/src/vaadin-accordion-panel.js';
import { Button } from '@vaadin/button/src/vaadin-button.js';
Expand Down Expand Up @@ -162,6 +163,27 @@ before(() => {
expect(tooltipOverlay.opened).to.be.true;
});

it('should not fire target click listeners on overlay click', () => {
const spy = sinon.spy();
tooltip.target.addEventListener('click', spy);

mouseenter(tooltip.target);
tooltipOverlay.click();

expect(spy.called).to.be.false;
});

it('should not fire target mousedown listeners on overlay mousedown', () => {
const spy = sinon.spy();
tooltip.target.addEventListener('mousedown', spy);

mouseenter(tooltip.target);
const content = tooltip.querySelector('[slot="overlay"]');
mousedown(content);

expect(spy.called).to.be.false;
});

it('should set has-tooltip attribute on the element', () => {
expect(element.hasAttribute('has-tooltip')).to.be.true;
});
Expand Down