shepherd
shepherd copied to clipboard
fix: show overlay again after cancel
If the tour was cancelled at some point the overlay is only shown if you use tour.start()
if you want to continue the tour afterwards (tour.next()
or tour.show(x)
the overlay is missing.
While i get that you should use tour.start()
after the tour is done, i don't really want to go through everything from the beginning again if i cancel mid tour... especially not since tour.getCurrentStep()
keeps reporting the last step before the tour was cancelled.
This fixes the need of a workaround by manually calling _setupActiveTour
and _setupModal
when the tour was cancelled and should be resumed.
EDIT: oh well... this is basically a enhanced version of https://github.com/shipshapecode/shepherd/pull/1237
EDIT2: Since you where talking about "coming up with a good fix" in the pull-request linked above... i'm not sure if this would be considered a "good" fix. It's a fix without changing to much of the existing code.
a proposed "better" fix (which requires a bit more changes to the code) might be:
- move the setup part of
tour.start()
logic in a_init
function - keep track of the
initialized
state which then will be reset in_done
- when calling
step.show()
make sure to check ifinitialized
is still valid, else fire_init
again
That would do more or less the same, but IF something should change on the start/setup
part it would only require one place to keep updated
@mutschler the only issue I'd have with the proposed solution is the assumption that the use case is to always start a tour where you left off. For me, I'd assume start()
is always going to start from the beginning. Perhaps we need a better method that would allow you to startFrom? Also, we intend this library to be kind of "dumb" to state/memory in general, so that the implementor can decide how that's tracked in their application. Certainly open to options and a PR, if you're up for it.
@chuckcarpenter makes sense!
I never ment to change how start()
works, since i think it's pretty clear that it start's from the beginning. No issue with that ;)
and im with you as well on the "dump" part regarting state: i don't think the library should keep track of this, an example on how i implemented it currently is provided later.
I've hacked together a quick example of what i ment with the proposal, see: https://github.com/shipshapecode/shepherd/commit/2862cf90fdcca692354feea9575a8b832c5ee604
Basically this won't change anything for start()
but would allow you to keep going when calling hide()
which i use for "pausing" and then continue with tour.next()
or step.show()
sinde currentStep
is only reset when the tour is done
or canceled
But even if the tour was canceled
at some time, it would be nice to give the user an option to resume
. While i guess you could make that happen through some kind of startFrom
on the tour... I thinl you're right: that should be handled by implementation...
Here's how i do the "logic" part of it:
- listen to the
show
event of a tour and save the step that's shown to a user var ex:lastTourStep
- when the tour is
canceled
(becuase the user hit esc or quitt) we keep that 'lastTourStep` saved - if the user hits the help button again, we check if
lastTourStep
is defined and if so we ask him if he want's to continue where he left of - if the user choose to continue i call
show
on the step saved inlastTourStep
This is absolutely fine and i don't think the library should keep track of that BUT this has currently an issue with the modal:
since the modal get's removed when canceled
or done
and ONLY created in tour.start()
so i basiccally do what i did in this PR but inside my own code so when i ask to resume, i run something like this:
if (!document.querySelector('.shepherd-modal-overlay-container')) {
lastTourStep.tour._setupModal()
}
lastTourStep.show()
This is the part i think the library should handle
i hope this makes it a little bit clearer what i was talking about and what i propose ;)
EDIT: so tl;dr: not gonna channge how start()
works, just make sure the modal and other setup is properly done when calling show()
on a step
Hey there. We found the same issue in developing our product and I'd like to know if this fix can be merged soon. Thanks!