Skip to content
This repository has been archived by the owner on Feb 17, 2021. It is now read-only.

Tour onError callback doesn't fire on showStep with bad target #274

Closed
travstone opened this issue Mar 24, 2016 · 5 comments
Closed

Tour onError callback doesn't fire on showStep with bad target #274

travstone opened this issue Mar 24, 2016 · 5 comments

Comments

@travstone
Copy link
Contributor

According to the use guide, we can provide an onError() callback for errors in which the target for a given step is not found. It seems like this callback doesn't fire in the case where the showStep() method is used.

CodePen example: http://codepen.io/trav_stone/pen/NNjJjW

After looking at the source briefly, it seems like this might be remedied by adding the following around line 1963 of hopscotch.js ( within the definition of this.showStep, in the if clause specifying false as the value of utils.getStepTarget(step) ):

utils.invokeEventCallbacks('error');

I'm not incredibly familiar with all the bits and pieces in there however, so perhaps there's a better approach.

If the onError() callback is not supposed to work with the showStep() method, can someone let me know why? Otherwise I think there's a wee bug in there.

The version I'm working with is hopscotch - v0.2.5

Thanks for an otherwise spectacular library!

Trav

@travstone
Copy link
Contributor Author

Oops- it might also be nice to include the line currStepNum = stepNum; right before utils.invoke...
It's of interest, to me at least, precisely which step failed.

@timlindvall
Copy link
Contributor

Huh... it would make sense that onError should work with showStep(). I'm guessing onError is only currently connected into the next/prevStep flow at present. =/

This probably won't be fixed in the current codebase, but the ES6 Refresh milestone should address this issue.

@timlindvall timlindvall added this to the ES6 Refresh milestone Jun 15, 2016
@travstone
Copy link
Contributor Author

Thanks zimmi... I believe I made a simple fix for this in our project, which I'd be happy to turn into a PR at some point, just let me know when/how/where.

@timlindvall
Copy link
Contributor

If there's a patch ready to go, I'll consider it for the 0.2.6 maintenance release. (Previously, there wasn't a plan for a maintenance release, but we have enough PRs where it's worth it to make a new release.)

Feel free to submit a PR and I'll take a look. Thanks!

@travstone
Copy link
Contributor Author

I've posted a PR for this, hopefully it's good.

timlindvall pushed a commit that referenced this issue Jun 26, 2016
* Add error callback for showStep

Calling showStep without a proper step target fails, but the tour error
callback is not invoked. This change fixes that, and sets the current
step number to the step that failed for debugging purposes

* Revert step num after error callback

Not sure if this will work

* remove all stepNum stuff, see if it still breaks

just a test to see what's breaking

* Add templates stuff

* Revert "Add templates stuff"

This reverts commit 8dfed46.

* Resubmit error callback patch

temporarily set currStepNum then revert

* deglobalizing temporary var

* Hoist temp var to top of function
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants