Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query element by selector later if it's not exists during configuration #1679

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .jshintrc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"exports": true,
"define": true,
"SVGElement": true,
"MutationObserver": true,
"Element": true,
"module": true,
"console": true,
Expand Down
35 changes: 29 additions & 6 deletions src/core/fetchIntroSteps.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,37 @@ export default function fetchIntroSteps(targetElm) {
//use querySelector function only when developer used CSS selector
if (typeof currentItem.element === "string") {
//grab the element with given selector from the page
currentItem.element = document.querySelector(currentItem.element);
}

//intro without element
if (
const el = document.querySelector(currentItem.element);
if (el !== null) {
currentItem.element = el;
} else {
// If element is not exists yet, we'll get it on step
const elSelector = currentItem.element;
Object.defineProperty(currentItem, "element", {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it important to have this object when the DOMElement doesn't exist? Can we just replace it with undefined?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, my first (local) implementation was with undefined, but it could be more changes in other code, that calls .element.

I'll try to do with undefined.

get() {
if (typeof this._element === "string") {
const result = document.querySelector(this._element);
if (result === null)
throw new Error(
"There is no element with given selector: " + this._element
);
return result;
} else {
return this._element;
}
},
set(value) {
this._element = value;
},
enumerable: true,
});
currentItem.element = elSelector;
}
} else if (
typeof currentItem.element === "undefined" ||
currentItem.element === null
) {
//intro without element
let floatingElementQuery = document.querySelector(
".introjsFloatingElement"
);
Expand All @@ -58,7 +81,7 @@ export default function fetchIntroSteps(targetElm) {
currentItem.disableInteraction = this._options.disableInteraction;
}

if (currentItem.element !== null) {
if (currentItem._element !== null || currentItem.element !== null) {
introItems.push(currentItem);
}
});
Expand Down
25 changes: 23 additions & 2 deletions src/core/steps.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import forEach from "../util/forEach";
import showElement from "./showElement";
import exitIntro from "./exitIntro";
import waitForElement from "../util/waitForElement";

/**
* Go to specific step of introduction
Expand Down Expand Up @@ -37,6 +38,7 @@ export function goToStepNumber(step) {
*/
export function nextStep() {
this._direction = "forward";
if (this._waitingNextElement) return;

if (typeof this._currentStepNumber !== "undefined") {
forEach(this._introItems, ({ step }, i) => {
Expand All @@ -59,7 +61,8 @@ export function nextStep() {
if (typeof this._introBeforeChangeCallback !== "undefined") {
continueStep = this._introBeforeChangeCallback.call(
this,
nextStep && nextStep.element
nextStep &&
(elementBySelectorNotExists(nextStep) ? undefined : nextStep.element)
);
}

Expand All @@ -79,7 +82,25 @@ export function nextStep() {
return;
}

showElement.call(this, nextStep);
if (elementBySelectorNotExists(nextStep)) {
this._waitingNextElement = true;
waitForElement(nextStep._element, () => {
this._waitingNextElement = false;
showElement.call(this, nextStep);
});
} else {
showElement.call(this, nextStep);
}
}

/**
* Return true if element locates by selector and doesn't exists yet
*/
function elementBySelectorNotExists(step) {
return (
typeof step._element === "string" &&
document.querySelector(step._element) === null
);
}

/**
Expand Down
62 changes: 62 additions & 0 deletions src/util/waitForElement.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/**
* Waits until Element will appear
*
* @api private
* @method _waitForElement
* @param {string} elSelector Selector to locate Element
* @param {() => void} callback Callback to be called after Element appearance
*/
export default function _waitForElement(elSelector, callback) {
if (document.querySelector(elSelector) !== null) {
callback();
return;
}

if (typeof MutationObserver !== "undefined") {
const observer = new MutationObserver(() => {
if (document.querySelector(elSelector) !== null) {
observer.disconnect();
callback();
}
});

observer.observe(document.body, {
childList: true,
subtree: true,
attributes: false,
characterData: false,
});
} else {
// Old browsers will wait by timeout
_waitForElementByTimeout(elSelector, callback, 1000, 10000);
}
}

/**
* @param {string} elSelector
* @param {() => void} callback
* @param {number} checkInterval In milliseconds
* @param {number} maxTimeout In milliseconds
*/
export function _waitForElementByTimeout(
elSelector,
callback,
checkInterval,
maxTimeout
) {
let startTimeInMs = Date.now();
(function loopSearch() {
if (document.querySelector(elSelector) !== null) {
callback();
return;
} else {
setTimeout(function () {
if (Date.now() - startTimeInMs > maxTimeout) {
callback();
return;
}
loopSearch();
}, checkInterval);
}
})();
}
38 changes: 36 additions & 2 deletions tests/core/fetchIntroSteps.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ describe("fetchIntroSteps", () => {
steps: [
{
element: "#element_does_not_exist",
position: "top",
intro: "hello world",
},
{
Expand All @@ -23,7 +24,7 @@ describe("fetchIntroSteps", () => {

expect(steps.length).toBe(2);

expect(steps[0].position).toBe("floating");
expect(steps[0].position).toBe("top");
expect(steps[0].intro).toBe("hello world");
expect(steps[0].step).toBe(1);

Expand Down Expand Up @@ -80,11 +81,44 @@ describe("fetchIntroSteps", () => {
expect(steps[1].intro).toBe("second");
expect(steps[1].step).toBe(2);

expect(steps[2].position).toBe("floating");
expect(steps[2].position).toBe("bottom");
expect(steps[2].intro).toBe("third");
expect(steps[2].step).toBe(3);
});

test("should throw an error on calling step.element if it is not exists yet and return element after adding", () => {
const targetElement = document.createElement("div");
const elId = "later_added";
const steps = fetchIntroSteps.call(
{
_options: {
tooltipPosition: "bottom",
steps: [
{
element: "#" + elId,
},
],
},
},
targetElement
);

expect(steps.length).toBe(1);

try {
const element = steps[0].element; // jshint ignore:line
} catch (e) {
if (!e.message.includes("There is no element with given selector:"))
throw e;
}
Comment on lines +108 to +113
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we checking here to make sure the steps array is empty?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we calling here .element of the first array item to check that Error "There is not element..." is throwing in case of element absence.


const laterAdded = document.createElement("div");
laterAdded.setAttribute("id", elId);
document.body.appendChild(laterAdded);

expect(steps[0].element).toBe(laterAdded);
});

test("should find the data-* elements from the DOM", () => {
const targetElement = document.createElement("div");

Expand Down
47 changes: 47 additions & 0 deletions tests/cypress/integration/tour/added-later-element.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
context("Added later element", () => {
const addedLaterElId = "later_added";
const stepOneText = "step one";
const stepTwoText = "step two, click on create btn";
const stepThreeText = "added later element";
const createDivBtnSelector = "#create-div-button";
beforeEach(() => {
cy.visit("./cypress/setup/create_div_btn.html").then((window) => {
window
.introJs()
.setOptions({
disableInteraction: false,
steps: [
{
intro: stepOneText,
},
{
intro: stepTwoText,
element: createDivBtnSelector,
},
{
intro: stepThreeText,
element: "#" + addedLaterElId,
},
],
})
.start();
});
});

it("should find by selector and highlight added later element", () => {
cy.get(".introjs-tooltiptext").contains(stepOneText);
cy.nextStep();
cy.get(".introjs-tooltiptext").contains(stepTwoText);
cy.wait(500);
cy.get(createDivBtnSelector).click();
cy.nextStep();
cy.wait(500);
cy.get("#" + addedLaterElId)
.filter(".introjs-showElement")
.contains("Later added div");
cy.wait(2000);
cy.compareSnapshot("added-later-element-end", 0.05);
cy.doneButton();
cy.get(".introjs-showElement").should("not.exist");
});
});
8 changes: 8 additions & 0 deletions tests/cypress/integration/tour/navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ context("Navigation", () => {
cy.get(".introjs-showElement").should("not.exist");
});

it("should exit the tour after right btn pressed at the end", () => {
cy.get(".introjs-tooltiptext").contains("step one");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful! Thank you

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, there wasn't such error in master before adding this feature. Possible error was taken into account in 87fa604

cy.nextStep();
cy.get(".introjs-tooltiptext").contains("step two");
cy.realPress("ArrowRight");
cy.get(".introjs-showElement").should("not.exist");
});

it("should close the tour after clicking on the exit button", () => {
cy.get(".introjs-showElement").should("exist");
cy.get(".introjs-skipbutton").click();
Expand Down