refactor: reflect opened property to attribute in tooltip (#9978)

Author: sissbruecker

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

@@ -68,8 +68,9 @@ export declare class TooltipMixinClass {
   manual: boolean;
 
   /**
-   * When true, the tooltip is opened programmatically.
-   * Only works if `manual` is set to `true`.
+   * When true, the tooltip is opened.
+   * In manual mode, this can be set programmatically.
+   * In automatic mode, this is set automatically by internal logic.
    */
   opened: boolean;
 

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

@@ -44,11 +44,6 @@ class TooltipStateController {
     this.host = host;
   }
 
-  /** @private */
-  get openedProp() {
-    return this.host.manual ? 'opened' : '_autoOpened';
-  }
-
   /** @private */
   get focusDelay() {
     const tooltip = this.host;
@@ -114,12 +109,12 @@ class TooltipStateController {
 
   /** @private */
   _isOpened() {
-    return this.host[this.openedProp];
+    return this.host.opened;
   }
 
   /** @private */
   _setOpened(opened) {
-    this.host[this.openedProp] = opened;
+    this.host.opened = opened;
   }
 
   /** @private */
@@ -300,12 +295,15 @@ export const TooltipMixin = (superClass) =>
         },
 
         /**
-         * When true, the tooltip is opened programmatically.
-         * Only works if `manual` is set to `true`.
+         * When true, the tooltip is opened.
+         * In manual mode, this can be set programmatically.
+         * In automatic mode, this is set automatically by internal logic.
          */
         opened: {
           type: Boolean,
           value: false,
+          reflectToAttribute: true,
+          observer: '__openedChanged',
           sync: true,
         },
 
@@ -330,17 +328,6 @@ export const TooltipMixin = (superClass) =>
           type: String,
         },
 
-        /**
-         * Set to true when the overlay is opened using auto-added
-         * event listeners: mouseenter and focusin (keyboard only).
-         * @protected
-         */
-        _autoOpened: {
-          type: Boolean,
-          observer: '__autoOpenedChanged',
-          sync: true,
-        },
-
         /**
          * Element used to link with the `aria-describedby`
          * attribute. When not set, defaults to `target`.
@@ -432,7 +419,7 @@ export const TooltipMixin = (superClass) =>
     disconnectedCallback() {
       super.disconnectedCallback();
 
-      if (this._autoOpened) {
+      if (this.opened && !this.manual) {
         this._stateController.close(true);
       }
       this._isConnected = false;
@@ -467,7 +454,7 @@ export const TooltipMixin = (superClass) =>
     }
 
     /** @private */
-    __autoOpenedChanged(opened, oldOpened) {
+    __openedChanged(opened, oldOpened) {
       if (opened) {
         document.addEventListener('keydown', this.__onKeyDown, true);
       } else if (oldOpened) {
@@ -530,7 +517,7 @@ export const TooltipMixin = (superClass) =>
 
       this.__focusInside = true;
 
-      if (!this.__isTargetHidden && (!this.__hoverInside || !this._autoOpened)) {
+      if (!this.__isTargetHidden && (!this.__hoverInside || !this.opened)) {
         this._stateController.open({ focus: true });
       }
     }
@@ -555,6 +542,10 @@ export const TooltipMixin = (superClass) =>
 
     /** @private */
     __onKeyDown(event) {
+      if (this.manual) {
+        return;
+      }
+
       if (event.key === 'Escape') {
         event.stopPropagation();
         this._stateController.close(true);
@@ -587,7 +578,7 @@ export const TooltipMixin = (superClass) =>
 
       this.__hoverInside = true;
 
-      if (!this.__isTargetHidden && (!this.__focusInside || !this._autoOpened)) {
+      if (!this.__isTargetHidden && (!this.__focusInside || !this.opened)) {
         this._stateController.open({ hover: true });
       }
     }
@@ -647,6 +638,10 @@ export const TooltipMixin = (superClass) =>
 
     /** @private */
     __onTargetVisibilityChange(isVisible) {
+      if (this.manual) {
+        return;
+      }
+
       const oldHidden = this.__isTargetHidden;
       this.__isTargetHidden = !isVisible;
 
@@ -657,7 +652,7 @@ export const TooltipMixin = (superClass) =>
       }
 
       // Close the overlay when the target is no longer fully visible.
-      if (!isVisible && this._autoOpened) {
+      if (!isVisible && this.opened) {
         this._stateController.close(true);
       }
     }

packages/tooltip/src/vaadin-tooltip.js

@@ -76,7 +76,7 @@ class Tooltip extends TooltipMixin(ThemePropertyMixin(ElementMixin(PolylitMixin(
         id="overlay"
         .owner="${this}"
         theme="${ifDefined(this._theme)}"
-        .opened="${this._isConnected && (this.manual ? this.opened : this._autoOpened)}"
+        .opened="${this._isConnected && this.opened}"
         .positionTarget="${this.target}"
         .position="${effectivePosition}"
         ?no-horizontal-overlap="${this.__computeNoHorizontalOverlap(effectivePosition)}"

packages/tooltip/test/tooltip.test.js

@@ -282,13 +282,14 @@ describe('vaadin-tooltip', () => {
 
       it('should still target correct element after sorting the items differently', async () => {
         const container = fixtureSync('<div></div>');
+
         function renderTooltips(items) {
           render(
             html`
               ${items.map(
                 (item) => html`
                   <vaadin-tooltip for="${item}"></vaadin-tooltip>
-                  <div id=${item}></div>
+                  <div id="${item}"></div>
                 `,
               )}
             `,
@@ -581,6 +582,18 @@ describe('vaadin-tooltip', () => {
 
       expect(overlay.opened).to.be.false;
     });
+
+    it('should reflect opened attribute', async () => {
+      mouseenter(target);
+      await nextUpdate(tooltip);
+      expect(tooltip.hasAttribute('opened')).to.be.true;
+      expect(overlay.opened).to.be.true;
+
+      mouseleave(target);
+      await nextUpdate(tooltip);
+      expect(tooltip.hasAttribute('opened')).to.be.false;
+      expect(overlay.opened).to.be.false;
+    });
   });
 
   describe('inside a scrollable container', () => {
@@ -865,6 +878,16 @@ describe('vaadin-tooltip', () => {
 
       expect(overlay.opened).to.be.true;
     });
+
+    it('should reflect opened attribute', async () => {
+      tooltip.opened = true;
+      await nextUpdate(tooltip);
+      expect(tooltip.hasAttribute('opened')).to.be.true;
+
+      tooltip.opened = false;
+      await nextUpdate(tooltip);
+      expect(tooltip.hasAttribute('opened')).to.be.false;
+    });
   });
 
   describe('moving target', () => {