Query element by selector later if it's not exists during configuration #1679
Conversation
When creating list of steps, we take into account the possible absence of element by selector If .element is a selector for which there is no element at the configuration stage, then we replace .element specifying a getter/setter for it. In the getter we return the element via document.querySelector Since that moment we assume that .element should return HTMLElement by selector and if there is still not element - throwing Error. Thus to check raw field below we need use ._element
|
And it's not finished yet. If element is added with delay (for example, after async request to server), an error will occur |
Fixed commit b61ffab and added test gooddaytoday@8716a1a for that error |
gooddaytoday
force-pushed
the
process-absense-element-by-selector
branch
from
6446dd6
to
43146e2
Compare
Previously, if you press right arrow button at the end of tour, it ends. Now it doesn't ends and any action after right arrow do nothing
gooddaytoday
force-pushed
the
process-absense-element-by-selector
branch
from
7930cd6
to
7580560
Compare
|
Rebased and edited branch in order to:
|
| try { | ||
| const element = steps[0].element; // jshint ignore:line | ||
| } catch (e) { | ||
| if (!e.message.includes("There is no element with given selector:")) | ||
| throw e; | ||
| } |
There was a problem hiding this comment.
Are we checking here to make sure the steps array is empty?
There was a problem hiding this comment.
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.
| } else { | ||
| // If element is not exists yet, we'll get it on step | ||
| const elSelector = currentItem.element; | ||
| Object.defineProperty(currentItem, "element", { |
There was a problem hiding this comment.
Is it important to have this object when the DOMElement doesn't exist? Can we just replace it with undefined?
There was a problem hiding this comment.
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.
src/util/waitForElement.js
Outdated
| } | ||
|
|
||
| if (typeof MutationObserver !== "undefined") { | ||
| /* jshint ignore:start */ |
There was a problem hiding this comment.
Why are we suppressing jshint errors? Happy to help if there are any issues with our jshint setup
There was a problem hiding this comment.
Yep, it could be deleted. Added MutationObserver to globals in jshint config and removed jshint ignore gooddaytoday@33e560b
src/util/waitForElement.js
Outdated
| * @param {number} checkInterval In milliseconds | ||
| * @param {number} maxTimeout In milliseconds | ||
| */ | ||
| function waitForElementByTimeout( |
There was a problem hiding this comment.
Can we test this function please? I'm a little concerned since it's doing a recursive call.
There was a problem hiding this comment.
Added tests in gooddaytoday@d5eaf0d
| @@ -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"); | |||
There was a problem hiding this comment.
In fact, there wasn't such error in master before adding this feature. Possible error was taken into account in 87fa604
gooddaytoday
force-pushed
the
process-absense-element-by-selector
branch
from
9db181e
to
6c25ffa
Compare
gooddaytoday
force-pushed
the
process-absense-element-by-selector
branch
from
6c25ffa
to
d5eaf0d
Compare
I'm sorry I didn't wait for approval #1674 🏎️ . Just need to buy commercial license with this feature 🙂 .
Processing possible absence of an element, configured with string selector: when creating list of steps, we take into account the possible absence of element.
step.elementis a selector for which there is no element at the configuration stage, then we replace.elementspecifying a getter/setter for it. In the getter we return the element viadocument.querySelector.elementshould returnHTMLElementby selector and if there is still no element - throwingError. Thus to check raw field below we need use also._element.The behavior has changed: earlier if there wasn't element by selector, IntroJS silently created floating tooltip at that step.
Also:
Closes #1674.