Merge pull request usablica#1124 from usablica/fixing-scrollToParent

fix(scrollParentToElement): should not run if scrollToElement is set to false

Author: afshinm

src/core/showElement.js

@@ -1,6 +1,5 @@
 import setShowElement from "../util/setShowElement";
 import scrollParentToElement from "../util/scrollParentToElement";
-import getScrollParent from "../util/getScrollParent";
 import addClass from "../util/addClass";
 import scrollTo from "../util/scrollTo";
 import exitIntro from "./exitIntro";
@@ -106,13 +105,8 @@ export default function _showElement(targetElement) {
     oldtooltipContainer.style.opacity = 0;
     oldtooltipContainer.style.display = "none";
 
-    // scroll to element
-    scrollParent = getScrollParent(targetElement.element);
-
-    if (scrollParent !== document.body) {
-      // target is within a scrollable element
-      scrollParentToElement(scrollParent, targetElement.element);
-    }
+    // if the target element is within a scrollable element
+    scrollParentToElement.call(self, targetElement);
 
     // set new position to helper layer
     setHelperLayerPosition.call(self, oldHelperLayer);
@@ -224,13 +218,8 @@ export default function _showElement(targetElement) {
       "box-shadow": `0 0 1px 2px rgba(33, 33, 33, 0.8), rgba(33, 33, 33, ${self._options.overlayOpacity.toString()}) 0 0 0 5000px`,
     });
 
-    // scroll to element
-    scrollParent = getScrollParent(targetElement.element);
-
-    if (scrollParent !== document.body) {
-      // target is within a scrollable element
-      scrollParentToElement(scrollParent, targetElement.element);
-    }
+    // target is within a scrollable element
+    scrollParentToElement.call(self, targetElement);
 
     //set new position to helper layer
     setHelperLayerPosition.call(self, helperLayer);

src/util/scrollParentToElement.js

@@ -1,10 +1,18 @@
+import getScrollParent from "./getScrollParent";
+
 /**
  * scroll a scrollable element to a child element
  *
- * @param Element parent
- * @param Element element
- * @return Null
+ * @param {Object} targetElement
  */
-export default function scrollParentToElement(parent, { offsetTop }) {
-  parent.scrollTop = offsetTop - parent.offsetTop;
+export default function scrollParentToElement(targetElement) {
+  const element = targetElement.element;
+
+  if (!this._options.scrollToElement) return;
+
+  const parent = getScrollParent(element);
+
+  if (parent === document.body) return;
+
+  parent.scrollTop = element.offsetTop - parent.offsetTop;
 }

src/util/scrollTo.js

@@ -5,7 +5,6 @@ import elementInViewport from "./elementInViewport";
  * To change the scroll of `window` after highlighting an element
  *
  * @api private
- * @method _scrollTo
  * @param {String} scrollTo
  * @param {Object} targetElement
  * @param {Object} tooltipLayer

tests/helper.js

@@ -1,13 +1,13 @@
 export function find(selector) {
-  if (typeof selector === 'string') {
+  if (typeof selector === "string") {
     return document.querySelector(selector);
   }
 
   if (selector instanceof Element) {
     return selector;
   }
 
-  throw Error('invalid selector');
+  throw Error("invalid selector");
 }
 
 export function content(selector) {
@@ -31,21 +31,34 @@ export function className(selector) {
 }
 
 export function skipButton() {
-  return find('.introjs-skipbutton');
+  return find(".introjs-skipbutton");
 }
 
 export function nextButton() {
-  return find('.introjs-nextbutton');
+  return find(".introjs-nextbutton");
 }
 
 export function prevButton() {
-  return find('.introjs-prevbutton');
+  return find(".introjs-prevbutton");
 }
 
 export function doneButton() {
-  return find('.introjs-donebutton');
+  return find(".introjs-donebutton");
 }
 
 export function tooltipText() {
-  return find('.introjs-tooltiptext');
+  return find(".introjs-tooltiptext");
+}
+
+/**
+ * @returns {Element}
+ */
+export function appendDummyElement(name, text, style) {
+  const el = document.createElement(name || "p");
+  el.innerHTML = text || "hello world";
+  el.style = style;
+
+  document.body.appendChild(el);
+
+  return el;
 }

tests/index.test.js

@@ -1,5 +1,14 @@
 import introJs from "../src";
-import {content, className, skipButton, nextButton, prevButton, doneButton, tooltipText} from "./helper";
+import {
+  content,
+  className,
+  skipButton,
+  nextButton,
+  prevButton,
+  doneButton,
+  tooltipText,
+  appendDummyElement,
+} from "./helper";
 
 describe("intro", () => {
   beforeEach(() => {
@@ -131,4 +140,75 @@ describe("intro", () => {
       "introjs-relativePosition"
     );
   });
+
+  test("should highlight the target element", () => {
+    const p = appendDummyElement();
+
+    introJs()
+      .setOptions({
+        steps: [
+          {
+            intro: "step one",
+            element: document.querySelector("p"),
+          },
+        ],
+      })
+      .start();
+
+    expect(p.className).toContain("introjs-showElement");
+    expect(p.className).toContain("introjs-relativePosition");
+  });
+
+  test("should not highlight the target element if queryString is incorrect", () => {
+    const p = appendDummyElement();
+
+    introJs()
+      .setOptions({
+        steps: [
+          {
+            intro: "step one",
+            element: document.querySelector("div"),
+          },
+        ],
+      })
+      .start();
+
+    expect(p.className).not.toContain("introjs-showElement");
+    expect(className(".introjs-showElement")).toContain(
+      "introjsFloatingElement"
+    );
+  });
+
+  test("should not add relativePosition if target element is fixed or absolute", () => {
+    const absolute = appendDummyElement(
+      "h1",
+      "hello world",
+      "position: absolute"
+    );
+    const fixed = appendDummyElement("h2", "hello world", "position: fixed");
+
+    const intro = introJs();
+    intro
+      .setOptions({
+        steps: [
+          {
+            intro: "step one",
+            element: document.querySelector("h1"),
+          },
+          {
+            intro: "step two",
+            element: document.querySelector("h2"),
+          },
+        ],
+      })
+      .start();
+
+    expect(absolute.className).toContain("introjs-showElement");
+    expect(absolute.className).not.toContain("introjs-relativePosition");
+
+    intro.nextStep();
+
+    expect(fixed.className).toContain("introjs-showElement");
+    expect(fixed.className).not.toContain("introjs-relativePosition");
+  });
 });

tests/util/addClass.test.js

@@ -17,21 +17,21 @@ describe("addClass", () => {
   });
 
   test("should append when element is SVG", () => {
-    const el = document.createElementNS('http://www.w3.org/2000/svg', 'svg');
-    el.setAttribute('class', "firstClass");
+    const el = document.createElementNS("http://www.w3.org/2000/svg", "svg");
+    el.setAttribute("class", "firstClass");
 
     addClass(el, "secondClass");
 
-    expect(el.getAttribute('class')).toBe("firstClass secondClass");
+    expect(el.getAttribute("class")).toBe("firstClass secondClass");
   });
 
   test("should not append duplicate classNames to svg elements", () => {
-    const el = document.createElementNS('http://www.w3.org/2000/svg', 'svg');
-    el.setAttribute('class', "firstClass");
+    const el = document.createElementNS("http://www.w3.org/2000/svg", "svg");
+    el.setAttribute("class", "firstClass");
 
     addClass(el, "firstClass");
 
-    expect(el.getAttribute('class')).toBe("firstClass");
+    expect(el.getAttribute("class")).toBe("firstClass");
   });
 
   test("should not append duplicate classNames to elements", () => {