Skip to content

Commit 325e4a0

Browse files
authored
refactor: reflect opened property to attribute in tooltip (#9978)
1 parent 9eae0a5 commit 325e4a0

File tree

4 files changed

+48
-29
lines changed

4 files changed

+48
-29
lines changed

packages/tooltip/src/vaadin-tooltip-mixin.d.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,9 @@ export declare class TooltipMixinClass {
6868
manual: boolean;
6969

7070
/**
71-
* When true, the tooltip is opened programmatically.
72-
* Only works if `manual` is set to `true`.
71+
* When true, the tooltip is opened.
72+
* In manual mode, this can be set programmatically.
73+
* In automatic mode, this is set automatically by internal logic.
7374
*/
7475
opened: boolean;
7576

packages/tooltip/src/vaadin-tooltip-mixin.js

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,6 @@ class TooltipStateController {
4444
this.host = host;
4545
}
4646

47-
/** @private */
48-
get openedProp() {
49-
return this.host.manual ? 'opened' : '_autoOpened';
50-
}
51-
5247
/** @private */
5348
get focusDelay() {
5449
const tooltip = this.host;
@@ -114,12 +109,12 @@ class TooltipStateController {
114109

115110
/** @private */
116111
_isOpened() {
117-
return this.host[this.openedProp];
112+
return this.host.opened;
118113
}
119114

120115
/** @private */
121116
_setOpened(opened) {
122-
this.host[this.openedProp] = opened;
117+
this.host.opened = opened;
123118
}
124119

125120
/** @private */
@@ -300,12 +295,15 @@ export const TooltipMixin = (superClass) =>
300295
},
301296

302297
/**
303-
* When true, the tooltip is opened programmatically.
304-
* Only works if `manual` is set to `true`.
298+
* When true, the tooltip is opened.
299+
* In manual mode, this can be set programmatically.
300+
* In automatic mode, this is set automatically by internal logic.
305301
*/
306302
opened: {
307303
type: Boolean,
308304
value: false,
305+
reflectToAttribute: true,
306+
observer: '__openedChanged',
309307
sync: true,
310308
},
311309

@@ -330,17 +328,6 @@ export const TooltipMixin = (superClass) =>
330328
type: String,
331329
},
332330

333-
/**
334-
* Set to true when the overlay is opened using auto-added
335-
* event listeners: mouseenter and focusin (keyboard only).
336-
* @protected
337-
*/
338-
_autoOpened: {
339-
type: Boolean,
340-
observer: '__autoOpenedChanged',
341-
sync: true,
342-
},
343-
344331
/**
345332
* Element used to link with the `aria-describedby`
346333
* attribute. When not set, defaults to `target`.
@@ -432,7 +419,7 @@ export const TooltipMixin = (superClass) =>
432419
disconnectedCallback() {
433420
super.disconnectedCallback();
434421

435-
if (this._autoOpened) {
422+
if (this.opened && !this.manual) {
436423
this._stateController.close(true);
437424
}
438425
this._isConnected = false;
@@ -467,7 +454,7 @@ export const TooltipMixin = (superClass) =>
467454
}
468455

469456
/** @private */
470-
__autoOpenedChanged(opened, oldOpened) {
457+
__openedChanged(opened, oldOpened) {
471458
if (opened) {
472459
document.addEventListener('keydown', this.__onKeyDown, true);
473460
} else if (oldOpened) {
@@ -530,7 +517,7 @@ export const TooltipMixin = (superClass) =>
530517

531518
this.__focusInside = true;
532519

533-
if (!this.__isTargetHidden && (!this.__hoverInside || !this._autoOpened)) {
520+
if (!this.__isTargetHidden && (!this.__hoverInside || !this.opened)) {
534521
this._stateController.open({ focus: true });
535522
}
536523
}
@@ -555,6 +542,10 @@ export const TooltipMixin = (superClass) =>
555542

556543
/** @private */
557544
__onKeyDown(event) {
545+
if (this.manual) {
546+
return;
547+
}
548+
558549
if (event.key === 'Escape') {
559550
event.stopPropagation();
560551
this._stateController.close(true);
@@ -587,7 +578,7 @@ export const TooltipMixin = (superClass) =>
587578

588579
this.__hoverInside = true;
589580

590-
if (!this.__isTargetHidden && (!this.__focusInside || !this._autoOpened)) {
581+
if (!this.__isTargetHidden && (!this.__focusInside || !this.opened)) {
591582
this._stateController.open({ hover: true });
592583
}
593584
}
@@ -647,6 +638,10 @@ export const TooltipMixin = (superClass) =>
647638

648639
/** @private */
649640
__onTargetVisibilityChange(isVisible) {
641+
if (this.manual) {
642+
return;
643+
}
644+
650645
const oldHidden = this.__isTargetHidden;
651646
this.__isTargetHidden = !isVisible;
652647

@@ -657,7 +652,7 @@ export const TooltipMixin = (superClass) =>
657652
}
658653

659654
// Close the overlay when the target is no longer fully visible.
660-
if (!isVisible && this._autoOpened) {
655+
if (!isVisible && this.opened) {
661656
this._stateController.close(true);
662657
}
663658
}

packages/tooltip/src/vaadin-tooltip.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ class Tooltip extends TooltipMixin(ThemePropertyMixin(ElementMixin(PolylitMixin(
7676
id="overlay"
7777
.owner="${this}"
7878
theme="${ifDefined(this._theme)}"
79-
.opened="${this._isConnected && (this.manual ? this.opened : this._autoOpened)}"
79+
.opened="${this._isConnected && this.opened}"
8080
.positionTarget="${this.target}"
8181
.position="${effectivePosition}"
8282
?no-horizontal-overlap="${this.__computeNoHorizontalOverlap(effectivePosition)}"

packages/tooltip/test/tooltip.test.js

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,13 +282,14 @@ describe('vaadin-tooltip', () => {
282282

283283
it('should still target correct element after sorting the items differently', async () => {
284284
const container = fixtureSync('<div></div>');
285+
285286
function renderTooltips(items) {
286287
render(
287288
html`
288289
${items.map(
289290
(item) => html`
290291
<vaadin-tooltip for="${item}"></vaadin-tooltip>
291-
<div id=${item}></div>
292+
<div id="${item}"></div>
292293
`,
293294
)}
294295
`,
@@ -581,6 +582,18 @@ describe('vaadin-tooltip', () => {
581582

582583
expect(overlay.opened).to.be.false;
583584
});
585+
586+
it('should reflect opened attribute', async () => {
587+
mouseenter(target);
588+
await nextUpdate(tooltip);
589+
expect(tooltip.hasAttribute('opened')).to.be.true;
590+
expect(overlay.opened).to.be.true;
591+
592+
mouseleave(target);
593+
await nextUpdate(tooltip);
594+
expect(tooltip.hasAttribute('opened')).to.be.false;
595+
expect(overlay.opened).to.be.false;
596+
});
584597
});
585598

586599
describe('inside a scrollable container', () => {
@@ -865,6 +878,16 @@ describe('vaadin-tooltip', () => {
865878

866879
expect(overlay.opened).to.be.true;
867880
});
881+
882+
it('should reflect opened attribute', async () => {
883+
tooltip.opened = true;
884+
await nextUpdate(tooltip);
885+
expect(tooltip.hasAttribute('opened')).to.be.true;
886+
887+
tooltip.opened = false;
888+
await nextUpdate(tooltip);
889+
expect(tooltip.hasAttribute('opened')).to.be.false;
890+
});
868891
});
869892

870893
describe('moving target', () => {

0 commit comments

Comments
 (0)