Dependency injection and move bringInView to element

assets/scripts/src/element.js

@@ -5,12 +5,16 @@ export default class Element {
    * DOM element object
    * @param node
    * @param options
+   * @param overlay
+   * @param window
+   * @param document
    */
-  constructor(node, options) {
+  constructor(node, options, overlay, window, document) {
     this.node = node;
     this.document = document;
     this.window = window;
     this.options = options;
+    this.overlay = overlay;
     this.popover = this.getPopover();
   }
 
@@ -62,10 +66,10 @@ export default class Element {
     }
 
     const elementRect = this.getCalculatedPosition();
-    const absoluteElementTop = elementRect.top + window.pageYOffset;
-    const middle = absoluteElementTop - (window.innerHeight / 2);
+    const absoluteElementTop = elementRect.top + this.window.pageYOffset;
+    const middle = absoluteElementTop - (this.window.innerHeight / 2);
 
-    window.scrollTo(0, middle);
+    this.window.scrollTo(0, middle);
   }
 
   /**
@@ -106,6 +110,18 @@ export default class Element {
 
   onHighlighted() {
     this.showPopover();
+
+    const highlightedElement = this;
+    const lastHighlightedElement = this.overlay.getLastHighlightedElement();
+
+    const highlightedNode = this.node;
+    const lastHighlightedNode = lastHighlightedElement && lastHighlightedElement.node;
+
+    // If this element is not already highlighted (because this call could
+    // be from the resize or scroll) and is not in view
+    if (highlightedNode !== lastHighlightedNode && !highlightedElement.isInView()) {
+      highlightedElement.bringInView();
+    }
   }
 
   showPopover() {

assets/scripts/src/overlay.js

@@ -9,12 +9,14 @@ export default class Overlay {
    * @param opacity number
    * @param padding number
    * @param animate bool
+   * @param window
+   * @param document
    */
   constructor({
     opacity = 0.75,
     padding = 10,
     animate = true,
-  }) {
+  }, window, document) {
     this.opacity = opacity; // Fixed opacity for the layover
     this.padding = padding; // Padding around the highlighted item
     this.animate = animate; // Should animate between the transitions
@@ -24,6 +26,7 @@ export default class Overlay {
     this.highlightedPosition = new Position({}); // position at which layover is patched currently
     this.redrawAnimation = null;                 // used to cancel the redraw animation
     this.highlightedElement = null;              // currently highlighted dom element (instance of Element)
+    this.lastHighlightedElement = null;          // element that was highlighted before current one
 
     this.draw = this.draw.bind(this);  // To pass the context of class, as it is to be used in redraw animation callback
 
@@ -58,6 +61,7 @@ export default class Overlay {
    */
   highlight(element, animate = true) {
     if (!element || !element.node) {
+      console.warn('Invalid element to highlight. Must be an instance of `Element`');
       return;
     }
 
@@ -75,6 +79,7 @@ export default class Overlay {
       return;
     }
 
+    this.lastHighlightedElement = this.highlightedElement;
     this.highlightedElement = element;
     this.positionToHighlight = position;
 
@@ -95,6 +100,14 @@ export default class Overlay {
     return this.highlightedElement;
   }
 
+  /**
+   * Gets the element that was highlighted before current element
+   * @returns {null|*}
+   */
+  getLastHighlightedElement() {
+    return this.lastHighlightedElement;
+  }
+
   /**
    * Removes the overlay and cancel any listeners
    */
@@ -102,6 +115,7 @@ export default class Overlay {
     this.positionToHighlight = new Position();
     this.highlightedElement.onDeselected();
     this.highlightedElement = null;
+    this.lastHighlightedElement = null;
 
     this.draw();
   }
@@ -138,8 +152,8 @@ export default class Overlay {
 
     // Cut the chunk of overlay that is over the highlighted item
     this.removeCloak({
-      posX: this.highlightedPosition.left - window.scrollX - this.padding,
-      posY: this.highlightedPosition.top - window.scrollY - this.padding,
+      posX: this.highlightedPosition.left - this.window.scrollX - this.padding,
+      posY: this.highlightedPosition.top - this.window.scrollY - this.padding,
       width: (this.highlightedPosition.right - this.highlightedPosition.left) + (this.padding * 2),
       height: (this.highlightedPosition.bottom - this.highlightedPosition.top) + (this.padding * 2),
     });
@@ -164,7 +178,7 @@ export default class Overlay {
     if (canHighlight || this.overlayAlpha > 0) {
       // Add the overlay if not already there
       if (!this.overlay.parentNode) {
-        document.body.appendChild(this.overlay);
+        this.document.body.appendChild(this.overlay);
       }
 
       // Stage a new animation frame only if the position has not been reached

assets/scripts/src/sholo.js

@@ -16,11 +16,11 @@ export default class Sholo {
       opacity: 0.75,
     }, options);
 
-    this.overlay = new Overlay(options);
-
     this.document = document;
     this.window = window;
 
+    this.overlay = new Overlay(options, window, document);
+
     this.steps = [];            // steps to be presented if any
     this.currentStep = 0;       // index for the currently highlighted step
 
@@ -57,7 +57,7 @@ export default class Sholo {
     }
 
     const highlightedElement = this.overlay.getHighlightedElement();
-    const popover = document.getElementById('sholo-popover-item');
+    const popover = this.document.getElementById('sholo-popover-item');
 
     const clickedHighlightedElement = highlightedElement.node.contains(e.target);
     const clickedPopover = popover && popover.contains(e.target);
@@ -82,13 +82,6 @@ export default class Sholo {
     } else if (prevClicked) {
       this.movePrevious();
     }
-
-    // @todo - move to onHighlighted hook and add the check if not visible then do this
-    if (this.overlay.highlightedElement) {
-      window.setTimeout(() => {
-        this.overlay.highlightedElement.bringInView();
-      }, 800);
-    }
   }
 
   /**
@@ -167,12 +160,19 @@ export default class Sholo {
     this.steps = [];
 
     steps.forEach((step, index) => {
-      if (!step.element) {
-        throw new Error(`Element (query selector or a dom element) missing in step ${index}`);
+      if (!step.element || typeof step.element !== 'string') {
+        throw new Error(`Element (query selector string) missing in step ${index}`);
       }
 
-      const domElement = Sholo.findDomElement(step.element);
-      const element = new Element(domElement, Object.assign({}, this.options, step));
+      const elementOptions = Object.assign({}, this.options, step);
+
+      const domElement = this.document.querySelector(step.element);
+      if (!domElement) {
+        console.warn(`Element to highlight ${step.element} not found`);
+        return;
+      }
+
+      const element = new Element(domElement, elementOptions, this.overlay, this.window, this.document);
 
       this.steps.push(element);
     });
@@ -189,24 +189,17 @@ export default class Sholo {
 
   /**
    * Highlights the given selector
-   * @param selector
+   * @param selector string query selector
+   * @todo make it accept json or query selector
    */
   highlight(selector) {
-    const domElement = Sholo.findDomElement(selector);
-
-    const element = new Element(domElement, this.options);
-    this.overlay.highlight(element);
-  }
-
-  static findDomElement(selector) {
-    if (typeof selector === 'string') {
-      return document.querySelector(selector);
-    }
-
-    if (typeof selector === 'object') {
-      return selector;
+    const domElement = this.document.querySelector(selector);
+    if (!domElement) {
+      console.warn(`Element to highlight ${selector} not found`);
+      return;
     }
 
-    throw new Error('Element can only be string or the dom element');
+    const element = new Element(domElement, this.options, this.overlay, this.window, this.document);
+    this.overlay.highlight(element);
   }
 }