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.

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

On the last step in the tour the next button is disabled, but can still be clicked #54

Open
DancingDad opened this issue Apr 26, 2020 · 5 comments

Comments

@DancingDad
Copy link

"bootstrap-tourist": "^0.3.2",

Error:
Noticed that on the last step in the tour the next button is greyed out but can still be clicked resulting in the user being locked in a backdrop they cannot navigate from without refreshing the page.

Solution:
The next button should be disabled and unclickable on the last step of the tour. The only clickable options should be "Prev" and "End Tour".

@DancingDad
Copy link
Author

DancingDad commented Apr 27, 2020

Think I found the issue.

Root Cause:
The data-role="next" is still in place, even though the button is visibly disabled. Due to the data-role being in place a click on the button still performs an action.

Fix:
In addition to adding the disabled class, added a line of code to remove the data-role attribute. This prevents the button from performing any action on click.

Code, recommended change shown with comment.

if (step.next < 0) {
    $next.addClass('disabled').prop('disabled', true).prop('tabindex', -1);
    $next.removeAttr('data-role');  // Added this line of code.
}

Testing:
After making the change locally:
All worked as expected:

  • Next button did not perform any action when on the last step.
  • Previous and End Tour buttons worked on last step.
  • Next button worked (obviously) on all earlier steps
  • Ran forward and back through the steps on multiple tours.
  • Ended and restarted the tour multiple times.

Leaving this issue open as I have not done a PR for this. Would classify myself as newbie at JS and some of this code looks a bit scary. :)

@IGreatlyDislikeJavascript
Copy link
Owner

Thanks for finding and fixing this. I'm hoping to be in a position to start updating code again shortly, a full lockdown still exists in my country.

Due to the data-role being in place a click on the button still performs an action.

That's odd! I need to look into this, my expectation was that jq click event (and handler) wouldn't fire on a disabled element. Looks like you've tested your fix comprehensively too, thank you.

Would classify myself as newbie at JS and some of this code looks a bit scary. :)

Haha yes, you and me both. As I've said all along, I'm not a js coder! Some of the scary code is because the original developers wrote Tour in coffee script, which I've never heard of and since found out does some weird transpilation stuff, with no comments.

Thanks again.

@vladrusu
Copy link

vladrusu commented May 13, 2020

The bug manifests itself on both last step, but on first step also (clicking on Previous on the first step closes the tour but leaves the backdrop/highlighted element visible!).

Elaborating on @DancingDad's solution, I came with this code:

if (step.prev <= 0) {
	$prev.addClass('disabled').prop('disabled', true).prop('tabindex', -1);
	$prev.removeAttr('data-role');
}
if (step.next < 0) {
	$next.addClass('disabled').prop('disabled', true).prop('tabindex', -1);
	$next.removeAttr('data-role');
}

@igor-stojcic
Copy link

Regards to all. This is a great correction, I just think that in the first if statement step.prev <= 0 should only be step.prev < 0 . Correct me if I'm wrong. Thanks.

killerbobjr added a commit to killerbobjr/bootstrap-tourist that referenced this issue Jan 24, 2021
…d issues a pause on click if true.

Also changes right-arrow to match the behavior, issuing a pause if last step is true.
@AlexOulapourHCTX
Copy link

Will killbobjr's fix for this be merged into master?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants