Improve performance and fix positioning issue with popover

kamranahmedse · Feb 7, 2019 · 5309108 · 5309108
1 parent b074110
commit 5309108
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 18 deletions.
diff --git a/demo/scripts/demo.js b/demo/scripts/demo.js
@@ -6,6 +6,7 @@ document.addEventListener('DOMContentLoaded', function () {
     {
       element: document.getElementById('driver-demo-head'),
       popover: {
+        className: 'scoped-driver-popover',
         title: 'Before we start',
         description: 'This is just one use-case, make sure to check out the rest of the docs below.',
         nextBtnText: 'Okay, Start!',
@@ -102,6 +103,7 @@ document.addEventListener('DOMContentLoaded', function () {
     opacity: 0.8,
     padding: 5,
     showButtons: true,
+    className: 'boring-scope',
   });
 
   boringTourDriver.defineSteps(tourSteps);
@@ -331,6 +333,7 @@ document.addEventListener('DOMContentLoaded', function () {
     {
       element: '#first-element-introduction',
       popover: {
+        className: 'first-step-popover-class',
         title: 'Title on Popover',
         description: 'Body of the popover',
         position: 'top'

diff --git a/src/common/utils.js b/src/common/utils.js
@@ -1,9 +1,8 @@
 /**
  * Turn a string into a node
  * @param  {String} htmlString to convert
- * @return {Node}   Converted node element
+ * @return {HTMLElement|Node}   Converted node element
  */
-// eslint-disable-next-line
 export const createNodeFromString = (htmlString) => {
   const div = document.createElement('div');
   div.innerHTML = htmlString.trim();

diff --git a/src/core/element.js b/src/core/element.js
@@ -38,14 +38,13 @@ export default class Element {
     this.overlay = overlay;
     this.popover = popover;
     this.stage = stage;
-
     this.animationTimeout = null;
   }
 
   /**
    * Checks if the current element is visible in viewport
    * @returns {boolean}
-   * @private
+   * @public
    */
   isInView() {
     let top = this.node.offsetTop;
@@ -83,10 +82,11 @@ export default class Element {
 
   /**
    * Brings the element to middle of the view port if not in view
-   * @private
+   * @public
    */
   bringInView() {
-    if (this.isInView()) {
+    // If the node is not there or already is in view
+    if (!this.node || this.isInView()) {
       return;
     }
 
@@ -196,12 +196,6 @@ export default class Element {
     this.addHighlightClasses();
 
     const highlightedElement = this;
-    const popoverElement = this.popover;
-
-    if (popoverElement && !popoverElement.isInView()) {
-      popoverElement.bringInView();
-    }
-
     if (!highlightedElement.isInView()) {
       highlightedElement.bringInView();
     }

diff --git a/src/core/popover.js b/src/core/popover.js
@@ -42,9 +42,6 @@ export default class Popover extends Element {
 
     this.window = window;
     this.document = document;
-
-    this.attachNode();
-    this.hide();
   }
 
   /**
@@ -53,11 +50,13 @@ export default class Popover extends Element {
    */
   attachNode() {
     let popover = this.document.getElementById(ID_POPOVER);
-    if (!popover) {
-      popover = createNodeFromString(POPOVER_HTML(this.options.className));
-      document.body.appendChild(popover);
+    if (popover) {
+      popover.parentElement.removeChild(popover);
     }
 
+    popover = createNodeFromString(POPOVER_HTML(this.options.className));
+    document.body.appendChild(popover);
+
     this.node = popover;
     this.tipNode = popover.querySelector(`.${CLASS_POPOVER_TIP}`);
     this.titleNode = popover.querySelector(`.${CLASS_POPOVER_TITLE}`);
@@ -117,6 +116,7 @@ export default class Popover extends Element {
    * @public
    */
   show(position) {
+    this.attachNode();
     this.setInitialState();
 
     // Set the title and descriptions
@@ -172,6 +172,9 @@ export default class Popover extends Element {
         this.autoPosition(position);
         break;
     }
+
+    // Bring the popover in view port once it is displayed
+    this.bringInView();
   }
 
   /**