-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Allows for local step asynchronous events #1266
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,7 @@ import appendChild from "../util/appendChild"; | |
* @method _getProgress | ||
* @returns current progress percentage | ||
*/ | ||
function _getProgress() { | ||
function _getProgress () { | ||
// Steps are 0 indexed | ||
const currentStep = parseInt(this._currentStep + 1, 10); | ||
return (currentStep / this._introItems.length) * 100; | ||
|
@@ -32,7 +32,7 @@ function _getProgress() { | |
* @api private | ||
* @method _disableInteraction | ||
*/ | ||
function _disableInteraction() { | ||
function _disableInteraction () { | ||
let disableInteractionLayer = document.querySelector( | ||
".introjs-disableInteraction" | ||
); | ||
|
@@ -55,8 +55,10 @@ function _disableInteraction() { | |
* @method _showElement | ||
* @param {Object} targetElement | ||
*/ | ||
export default function _showElement(targetElement) { | ||
if (typeof this._introChangeCallback !== "undefined") { | ||
export default function _showElement (targetElement) { | ||
if (typeof targetElement.onchange === 'function') { | ||
targetElement.onchange(); | ||
} else if (typeof this._introChangeCallback !== "undefined") { | ||
this._introChangeCallback.call(this, targetElement.element); | ||
} | ||
|
||
|
@@ -69,7 +71,6 @@ export default function _showElement(targetElement) { | |
let nextTooltipButton; | ||
let prevTooltipButton; | ||
let skipTooltipButton; | ||
let scrollParent; | ||
|
||
//check for a current step highlight class | ||
if (typeof targetElement.highlightClass === "string") { | ||
|
@@ -94,6 +95,22 @@ export default function _showElement(targetElement) { | |
const oldtooltipContainer = oldReferenceLayer.querySelector( | ||
".introjs-tooltip" | ||
); | ||
const oldTooltipHeaderLayer = oldReferenceLayer.querySelector( | ||
".introjs-tooltip-header" | ||
); | ||
|
||
// remove previous step class | ||
oldReferenceLayer.classList.remove(`step-${targetElement.step - 1}`); | ||
// remove next step class in case going back | ||
oldReferenceLayer.classList.remove(`step-${targetElement.step + 1}`); | ||
// add current step class | ||
oldReferenceLayer.classList.add(`step-${targetElement.step}`); | ||
|
||
if (targetElement.title) { | ||
oldTooltipHeaderLayer.classList.remove('no-title'); | ||
} else { | ||
oldTooltipHeaderLayer.classList.add('no-title'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, it's for themes to use if they need it. For instance, in my use case, I needed to hide the header element if there's no title. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, got it. I personally think we should keep the codebase simple and avoid adding any classes that is not used right now. I understand that you need that no-title class in your application though. Can you use your initial input (e.g. your steps list) to see if a specific step as title or not? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, thought of that but that's JS. It's better to use CSS if possible instead of JS. About avoiding adding classes, it's not unusual for libraries to add css classes that may help developers customize the outlook, even if the library doesn't use the classes. |
||
} | ||
|
||
skipTooltipButton = oldReferenceLayer.querySelector(".introjs-skipbutton"); | ||
prevTooltipButton = oldReferenceLayer.querySelector(".introjs-prevbutton"); | ||
|
@@ -191,7 +208,7 @@ export default function _showElement(targetElement) { | |
className: highlightClass, | ||
}); | ||
const referenceLayer = createElement("div", { | ||
className: "introjs-tooltipReferenceLayer", | ||
className: `introjs-tooltipReferenceLayer step-${targetElement.step}`, | ||
}); | ||
const arrowLayer = createElement("div", { | ||
className: "introjs-arrow", | ||
|
@@ -232,6 +249,10 @@ export default function _showElement(targetElement) { | |
tooltipTextLayer.innerHTML = targetElement.intro; | ||
tooltipTitleLayer.innerHTML = targetElement.title; | ||
|
||
if (!targetElement.title) { | ||
tooltipHeaderLayer.classList.add('no-title'); | ||
} | ||
|
||
if (this._options.showBullets === false) { | ||
bulletsLayer.style.display = "none"; | ||
} | ||
|
@@ -314,6 +335,14 @@ export default function _showElement(targetElement) { | |
nextTooltipButton = createElement("a"); | ||
|
||
nextTooltipButton.onclick = () => { | ||
const refLayer = oldReferenceLayer || document.querySelector( | ||
".introjs-tooltipReferenceLayer" | ||
); | ||
|
||
if (refLayer && refLayer.classList.contains('waiting')) { | ||
return; | ||
} | ||
|
||
if (self._introItems.length - 1 !== self._currentStep) { | ||
nextStep.call(self); | ||
} else if (/introjs-donebutton/gi.test(nextTooltipButton.className)) { | ||
|
@@ -332,6 +361,14 @@ export default function _showElement(targetElement) { | |
prevTooltipButton = createElement("a"); | ||
|
||
prevTooltipButton.onclick = () => { | ||
const refLayer = oldReferenceLayer || document.querySelector( | ||
".introjs-tooltipReferenceLayer" | ||
); | ||
|
||
if (refLayer && refLayer.classList.contains('waiting')) { | ||
return; | ||
} | ||
|
||
if (self._currentStep !== 0) { | ||
previousStep.call(self); | ||
} | ||
|
@@ -356,7 +393,9 @@ export default function _showElement(targetElement) { | |
self._introCompleteCallback.call(self); | ||
} | ||
|
||
if (typeof self._introSkipCallback === "function") { | ||
if (typeof targetElement.onskip === 'function') { | ||
targetElement.onskip.call(self); | ||
} else if (typeof self._introSkipCallback === "function") { | ||
self._introSkipCallback.call(self); | ||
} | ||
|
||
|
@@ -503,7 +542,9 @@ export default function _showElement(targetElement) { | |
|
||
setShowElement(targetElement); | ||
|
||
if (typeof this._introAfterChangeCallback !== "undefined") { | ||
if (typeof targetElement.onafterchange === 'function') { | ||
targetElement.onafterchange(); | ||
} else if (typeof this._introAfterChangeCallback !== "undefined") { | ||
this._introAfterChangeCallback.call(this, targetElement.element); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a scary function to change mainly because we don't have any tests for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the point is to use a single loop, instead of two. As such, existing tests already cover this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if we have tests for this? there are two ways of adding the steps, using the data-* attributes and a JSON object. I don't think we have any tests for the data-* attr approach.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just started refactoring this function and adding some tests. I will ping you once it's merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WIP #1295