-
Notifications
You must be signed in to change notification settings - Fork 111
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
skipUndefinedTrip option should allow function as an alternative to Boolean #170
Comments
Hmm my original thought of this option is to help developers when they are trying to develop their trips at the first time. For trips, in my mind (please correct me if I am wrong), these guided tours should be well defined before presenting to users, so in production environment, there should not be any undefined trips. |
Many things can happen in production. Although we'd like to make our tours always right, there might be unforeseen situations that occur in production. The trips we implement rely on dynamic data that might not come back as expected. Someone else might be doing the testing in the organization. The application could be instrumented with handlers that report all errors in the same manner rather than in the console. My colleague who suggested that change is currently in the process of implementing about 30 different tours. He also has implemented a mode where tours run through automatically with no delay and it would be useful if all errors were to be reported to the application - the handler could record the problem but let the tour proceed by returning true. |
Hmm ok, I think your point is also right. In order to achieve this, I am thinking about the new way to transfer data. Take this discussion for example : var trip = new Trip();
trip.on('start', function() {
// like original onStart
});
trip.on('end', function() {
// like original onEnd
});
trip.on('error', function(tripIndex, tripObject) {
// 1. trip is not valid
// 2. in the future we can also report any error from Trip.js itself, and this cb will always be called.
}); I think this should be the first step to move some callbacks out to this event based style (mainly for global-used things), because I have noticed that it's a little bit confusing and hard to read if I put too many things into each What do you think about this change ?
Can you share more details behind this ? What's |
The I agree that setting callbacks for individual tripObject can get confusing but in this case we're talking about a global setting. Also, the trip options (with the callbacks) are sent along with the The more general error callback is a good idea although I would follow the current pattern and add a I don't have a strong opinion about this. I would keep in mind that there might be multiple trip instantiations. |
Yes i know what you mean, if it's just a global property called Maybe your use case is more complicated than normal users because there may be tons of trips that will be interacted across pages (sounds interesting).
No matter how, I agree with this part, instead of passing Ok, I think we can follow our current pattern first and see how things going later and change it if needed ! |
An undefined trip is most often likely to be a problem in an application. Allowing
skipUndefinedTrip
option to be a function (likecanGoPrev
/canGoNext
), would allow the application to take action upon a trip error (e.g. reporting an error to a back-end server).The text was updated successfully, but these errors were encountered: