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.

Sign up for GitHub

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

Jump to bottom

Reload page after tour ends, onStart fires but not onEnd #720

Open
ds00424 opened this issue Feb 13, 2019 · 6 comments
Open

Reload page after tour ends, onStart fires but not onEnd #720

ds00424 opened this issue Feb 13, 2019 · 6 comments

Comments

@ds00424
Copy link

ds00424 commented Feb 13, 2019

Load page with unstarted BSTour and onStart fires, Tour is shown. Click End Tour and onEnd fires.
Reload the page. onStart fires, Tour is not shown and BSTour debug reports:

Bootstrap Tour 'tour' | Tour ended, init prevented.
OnStart (my console.log in the onStart function)
Bootstrap Tour 'tour' | Tour ended, showStep prevented.

But no onEnd fires.

Either onStart should not fire when tour is not actually started OR onEnd should fire to show that the tour has ended.

Here is the code used in this test:

<html>
	<head>
		<script type="text/javascript" src="https://ajax.googleapis.com/ajax/libs/jquery/2.2.0/jquery.min.js"></script>
		<script type="text/javascript" src="bootstrap-tour-standalone.min.js"></script>
		<link rel="stylesheet" href="bootstrap-tour-standalone.min.css">
		<script>
			$(document).ready(function() {
				var tour = new Tour({
					name: "tour",
					debug: true,
					onStart: function() {
								console.log("OnStart");
					},
					onEnd: function() {
								console.log("OnEnd");
							},
					steps: [
						{
							element: "#TextBlock",
							title: "Step 1",
							content: "Step 1",
							placement: "bottom",
						},
						{
							element: "#TextBlock",
							title: "Step 2",
							content: "Step 2",
							placement: "bottom",
						},
					],
				});
				
//				tour.init();
				tour.start();
			});		
		</script>
	</head>
	<body>
		<div id='TextBlock'>Bootstrap Tour Test</div>
	</body>
</html>
@ds00424
Copy link
Author

ds00424 commented Feb 16, 2019

Firing onEnd when it is decided NOT to start the tour -- Looks like the showStep function needs the following 1 line addition:
(Note this is in bootstrap-tour-standalone.js)
Looks similar in the coffeescript -- but i am not familiar with coffeescript.

    Tour.prototype.showStep = function(i) {
      var path, promise, showDelay, showStepHelper, skipToPrevious, step;
      if (this.ended()) {
        this._debug('Tour ended, showStep prevented.');
		if (this._options.onEnd != null) this._options.onEnd.call(this); //*** ADDED ***
		return this;
      }

Not firing onStart (and not calling showStep) can be done in Tour.prototype.start:

	  if (force == true || !this.ended()) {  //**** ADDED ****
		  if (this._current === null) {
			promise = this._makePromise(this._options.onStart != null ? this._options.onStart(this) : void 0);
			this._callOnPromiseDone(promise, this.showStep, 0);
		  }
	  }  // **** ADDED ****
      return this;
    };

Open
@IGreatlyDislikeJavascript
Copy link

I'm going to add this to my fork, but with slightly altered behavior - if tour start is called when tour has previously ended, it'll call onPreviouslyEnded (or something similar).

I think calling the end callback when the tour hasn't shown itself could cause some unintended side effects in app logic. You wouldn't want to call an end without calling start, because logically why would the end callback get fired without a start callback (introduces potential for bugs if, say, end callback expects something to be done in start callback). And you wouldn't call start callback if the tour isn't actually started.

@ds00424
Copy link
Author

ds00424 commented Feb 17, 2019

Agree with your logic, but I implemented both of what I posted. I don't know the code as well as you probably do, but looks like showStep is called to show each step. So it might be possible that two browser tabs are open showing the tour and tab #2 ends the tour. Then in tab #1 when show step is called will end the tour and really should call the onEnd callback. Just my two cents.

I also found a scrolling bug and will file another issue with my fix. Perhaps you are aware of it in your fork, but in case not....

@IGreatlyDislikeJavascript
Copy link

That's a very good - but somewhat strange! - case, I didn't think of that one. I've implemented a slightly different solution because we need to handle 2 scenarios.

  • If tour.start() is called, and "tour ended" cookie is set, tour calls onPreviouslyEnded() and immediately exits. This prevents calling onStart() / onEnd() plus internal code for a tour that never showed anything to the user. That enables UX affecting code to be in those callbacks if required.

  • If showStep() is called, and tour "ended" cookie is set, tour calls onEnd() as per your fix. This solves the case you describe where 2 tabs are open. The tour starts on tab 1, firing onStart(). The tour continues on tab 2, firing onStart() but picking up from previous step on tab 1 using cookie. The tour then ends on one of the tabs, firing onEnd(). As soon as the tour is touched on the other tab, onEnd() also fires.

This solves the 2 use cases, I think?

@ErnestSenyo
Copy link
Collaborator

ErnestSenyo commented Feb 18, 2019 •
edited
Loading

if after the endTour and you recharge your page your guide replay no longer uses the init. use the tour.start like this
document.querySelector('.stat-tour').addEventListener('click', startTour);

function startTour(e) {
tour.start(true);
}

@IGreatlyDislikeJavascript
Copy link

I've created a new repo for the fork of bootstrap-tour:

https://github.com/IGreatlyDislikeJavascript/bootstrap-tourist

This should help avoid confusion in the fork etc.

I've fixed this in the fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@ds00424 @ErnestSenyo @IGreatlyDislikeJavascript