shepherd icon indicating copy to clipboard operation
shepherd copied to clipboard

fix: show overlay again after cancel

Open mutschler opened this issue 10 months ago • 3 comments

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 if initialized 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 avatar Aug 08 '23 13:08 mutschler

@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 avatar Aug 11 '23 23:08 chuckcarpenter

@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 in lastTourStep

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

mutschler avatar Aug 12 '23 20:08 mutschler

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!

haoming29 avatar Sep 25 '23 16:09 haoming29