Merge pull request #501 from rackerlabs/surf-1572-bug-hx-popover-arrow-placement

fix(#450): hx-popover open on render

Author: 100stacks

docs/components/popover/test.html

@@ -7,8 +7,10 @@
 {% extends 'test.njk' %}
 
 {% block content %}
+  {# TODO: Write tests to set shadow position on open and render. #}
+
   <section>
-    <h2>Appearance</h2>
+    <h2 id="appearance">Appearance</h2>
     <div class="popover-appearance">
       <hx-popover>
         <hx-div>
@@ -27,7 +29,7 @@ <h2>Appearance</h2>
   </section>
 
   <section>
-    <h2>Full Markup Appearance</h2>
+    <h2 id="full-markup-appearance">Full Markup Appearance</h2>
     <div class="popover-appearance">
       <hx-popover position="right-bottom">
         <header>
@@ -158,7 +160,7 @@ <h2>Scrollable Content (configured hx-div)</h2>
   </section>
 
   <section>
-    <h2>Scrolling Content (Body and Foot)</h2>
+    <h2 id="scrolling-content-body-foot">Scrolling Content (Body and Foot)</h2>
     <div class="popover-appearance">
       <hx-popover position="left">
         <hx-div scroll="vertical">
@@ -196,7 +198,7 @@ <h2>Scrolling Content (Body and Foot)</h2>
   </section>
 
   <section>
-    <h2>Scrolling Content (Body only)</h2>
+    <h2 id="scrolling-content-body">Scrolling Content (Body only)</h2>
     <div class="popover-appearance">
       <hx-popover position="left">
         <hx-div scroll="vertical">
@@ -368,6 +370,35 @@ <h2>Within Scrollable Container</h2>
     </hx-div>
   </section>
 
+  <section>
+    <header>
+      <h2 id="popover-in-table-header">Popover Inside a Table Header</h2>
+    </header>
+    <table class="hxTable">
+      <thead>
+        <tr>
+          <th>
+            Table Header
+            <hx-disclosure aria-controls="testPopover" class="hxBtn" open>
+              <hx-icon type="cog"></hx-icon>
+            </hx-disclosure>
+            <hx-popover id="testPopover" open position="right">
+              <header>
+                Popover Header
+              </header>
+
+              <hx-div>
+                <p>
+                  This is my popover body.
+                </p>
+              </hx-div>
+            </hx-popover>
+          </th>
+        </tr>
+      </thead>
+    </table>
+  </section>
+
   <h2>Deprecated Markup</h2>
 
   <section>
@@ -523,33 +554,4 @@ <h3>Deprecated Scrolling Content</h3>
       </hx-popover>
     </div>
   </section>
-
-  <section>
-    <header>
-      <h2 id="popover-in-table-header">Popover Inside a Table Header</h2>
-    </header>
-    <table class="hxTable">
-      <thead>
-        <tr>
-          <th>
-            Table Header
-            <hx-disclosure aria-controls="testPopover" class="hxBtn" open>
-              <hx-icon type="cog"></hx-icon>
-            </hx-disclosure>
-            <hx-popover id="testPopover" open position="right">
-              <header>
-                Popover Header
-              </header>
-
-              <hx-div>
-                <p>
-                  This is my popover body.
-                </p>
-              </hx-div>
-            </hx-popover>
-          </th>
-        </tr>
-      </thead>
-    </table>
-  </section>
 {% endblock %}

src/helix-ui/elements/HXPopoverElement.js

@@ -31,44 +31,16 @@ export class HXPopoverElement extends _ProtoClass {
         this.POSITION_OFFSET = 20;
     }
 
-    /** @override */
-    $onConnect () {
-        super.$onConnect();
-        this.addEventListener('reposition', this._onReposition);
-    }
-
-    $onDisconnect () {
-        super.$onDisconnect();
-        this.removeEventListener('reposition', this._onReposition);
-    }
-
-    /** @override */
-    $onAttributeChange (attr, oldVal, newVal) {
-        super.$onAttributeChange(attr, oldVal, newVal);
-
-        if (attr === 'position') {
-            this._setShadowPosition(newVal);
-        }
-    }
-
     /** @private */
     get _elRoot () {
         return this.shadowRoot.getElementById('hxPopover');
     }
 
     /**
-     * Update visual display of arrow in Shadow DOM based on optimal position.
-     * @private
-     */
-    _onReposition () {
-        this._setShadowPosition(this.optimumPosition);
-    }
-
-    /**
-     * @private
+     * @override
      * @param {NormalizedPositionString}
      */
-    _setShadowPosition (position) {
+    setShadowPosition (position) {
         this._elRoot.setAttribute('position', position);
     }
 }

src/helix-ui/elements/HXTooltipElement.js

@@ -53,15 +53,13 @@ export class HXTooltipElement extends _ProtoClass {
         this.$defaultAttribute('id', `tip-${generateId()}`);
         this.$defaultAttribute('role', 'tooltip');
 
-        this.addEventListener('reposition', this._onReposition);
         this._connectToControl();
     }
 
     /** @override */
     $onDisconnect () {
         super.$onDisconnect();
 
-        this.removeEventListener('reposition', this._onReposition);
         this._detachListeners();
     }
 
@@ -74,14 +72,8 @@ export class HXTooltipElement extends _ProtoClass {
     $onAttributeChange (attr, oldVal, newVal) {
         super.$onAttributeChange(attr, oldVal, newVal);
 
-        switch (attr) {
-            case 'for':
-                this._connectToControl();
-                break;
-
-            case 'position':
-                this._setShadowPosition(newVal);
-                break;
+        if (attr === 'for') {
+            this._connectToControl();
         }
     }
 
@@ -111,6 +103,14 @@ export class HXTooltipElement extends _ProtoClass {
         this.setAttribute('for', value);
     }
 
+    /**
+     * @override
+     * @param {NormalizedPositionString}
+     */
+    setShadowPosition (position) {
+        this._elRoot.setAttribute('position', position);
+    }
+
     /** @private */
     get _elRoot () {
         return this.shadowRoot.getElementById('hxTooltip');
@@ -288,16 +288,6 @@ export class HXTooltipElement extends _ProtoClass {
         }
     }
 
-    /** @private */
-    _onReposition () {
-        this._setShadowPosition(this.optimumPosition);
-    }
-
-    /** @private */
-    _setShadowPosition (position) {
-        this._elRoot.setAttribute('position', position);
-    }
-
     /**
      * Show Tooltip after delay
      *

src/helix-ui/mixins/Positionable.js

@@ -114,6 +114,7 @@ export const Positionable = (superclass) => {
             super.$onAttributeChange(attr, oldVal, newVal);
 
             if (attr === 'position') {
+                this.setShadowPosition(newVal);
                 this.reposition();
             }
         }
@@ -212,11 +213,21 @@ export const Positionable = (superclass) => {
                 this.style.left = `${x}px`;
 
                 this._optimumPosition = position;
+                this.setShadowPosition(position);
 
                 this.$emit('reposition');
             }
         }
 
+        /**
+         * Used to communicate position changes to Shadow DOM for subclasses that care.
+         *
+         * @abstract
+         * @ignore
+         * @param {NormalizedPositionString}
+         */
+        setShadowPosition (position) {} // eslint-disable-line no-unused-vars
+
         /**
          * Add active event listeners (e.g, document `click`)
          * These listeners rely on `this.controlElement` to manipulate