From e9a3097dc70ff6fded6e5b8c9f090c6e81bc14df Mon Sep 17 00:00:00 2001 From: Afshin Mehrabani Date: Sat, 19 Dec 2020 11:20:02 +0000 Subject: [PATCH 1/2] fix(scrollParentToElement): should not run if scrollToElement is set to false --- src/core/showElement.js | 19 ++++--------------- src/util/scrollParentToElement.js | 18 +++++++++++++----- src/util/scrollTo.js | 1 - tests/helper.js | 14 +++++++------- tests/index.test.js | 10 +++++++++- tests/util/addClass.test.js | 12 ++++++------ 6 files changed, 39 insertions(+), 35 deletions(-) diff --git a/src/core/showElement.js b/src/core/showElement.js index 629f9d801..a2b6d56cb 100644 --- a/src/core/showElement.js +++ b/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); diff --git a/src/util/scrollParentToElement.js b/src/util/scrollParentToElement.js index d4357eca6..d853ddf7c 100644 --- a/src/util/scrollParentToElement.js +++ b/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; } diff --git a/src/util/scrollTo.js b/src/util/scrollTo.js index ea71cd940..cb6720fd1 100644 --- a/src/util/scrollTo.js +++ b/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 diff --git a/tests/helper.js b/tests/helper.js index 516510392..bf50c50ee 100644 --- a/tests/helper.js +++ b/tests/helper.js @@ -1,5 +1,5 @@ export function find(selector) { - if (typeof selector === 'string') { + if (typeof selector === "string") { return document.querySelector(selector); } @@ -7,7 +7,7 @@ export function find(selector) { return selector; } - throw Error('invalid selector'); + throw Error("invalid selector"); } export function content(selector) { @@ -31,21 +31,21 @@ 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"); } diff --git a/tests/index.test.js b/tests/index.test.js index c64ec3fa2..69c2f9298 100644 --- a/tests/index.test.js +++ b/tests/index.test.js @@ -1,5 +1,13 @@ import introJs from "../src"; -import {content, className, skipButton, nextButton, prevButton, doneButton, tooltipText} from "./helper"; +import { + content, + className, + skipButton, + nextButton, + prevButton, + doneButton, + tooltipText, +} from "./helper"; describe("intro", () => { beforeEach(() => { diff --git a/tests/util/addClass.test.js b/tests/util/addClass.test.js index b1d5785e2..7cc249aaf 100644 --- a/tests/util/addClass.test.js +++ b/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", () => { From e0fdfc6fa58f99960933fe0625b58eddd82105a7 Mon Sep 17 00:00:00 2001 From: Afshin Mehrabani Date: Sat, 19 Dec 2020 12:22:15 +0000 Subject: [PATCH 2/2] chore: tour tests --- tests/helper.js | 13 ++++++++ tests/index.test.js | 72 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/tests/helper.js b/tests/helper.js index bf50c50ee..217b94117 100644 --- a/tests/helper.js +++ b/tests/helper.js @@ -49,3 +49,16 @@ export function doneButton() { export function 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; +} diff --git a/tests/index.test.js b/tests/index.test.js index 69c2f9298..5a458ad68 100644 --- a/tests/index.test.js +++ b/tests/index.test.js @@ -7,6 +7,7 @@ import { prevButton, doneButton, tooltipText, + appendDummyElement, } from "./helper"; describe("intro", () => { @@ -139,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"); + }); });