gilbarbara / react-joyride

Create guided tours in your apps
https://react-joyride.com/
MIT License
6.64k stars 519 forks source link

Controlled tour does not work when the tour needs to be suspended during step:before #837

Closed dcworldwide closed 1 year ago

dcworldwide commented 1 year ago

🐛 Bug Report

When a new tour step is started, I need to pause the tour, change the app route, resume the tour.

step:before can't be used for this purpose. It creates an infinite loop.

I have assumed that the DOM binding to the target happens after step:before as that seems logical.

Thank you

Step:before logic with callback


if (step?.route) {

                setState({
                    ...state,
                    running: false
                })

                wait(seconds(2)).then(() => {
                    emitRoute(step.route)
                    wait(seconds(2)).then(() => {
                        setState({
                            ...state,
                            running: true
                        })
                    })
                })
            }

This code works if applied during step:after... however I must use step:before...because the first step in the tour may require a route change. All of my step configuration is dynamically injected based on configuration. I can't hardcode any tour specific logic within the callback. Which is why I pass a route parameter per step.


Looking at the code shows that step:before is not called, unless the DOM element exists... hence why controlled tours can never use step:before for conditional logic.

What is the impact of moving the above to occur before the DOM element check?

https://github.com/gilbarbara/react-joyride/blob/6eba9cd04e06c79efcef81ce0921cb14b9bfad51/src/components/Step.js#L157

gilbarbara commented 1 year ago

hey @dcworldwide

This behavior isn't a bug, it was designed that way.

Did you check the demo source code? The MultiRoute example shows how to achieve that

dcworldwide commented 1 year ago

There is a bug in your example.

If the user is on page X and the first step of the tour is page Y, your example won't support that, due to the issue i mentioned earlier.

I think this should be re-opened please.

gilbarbara commented 1 year ago

@dcworldwide

The example works for that particular case, so it can be changed to suit other needs.

There's no bug in the lifecycle, it just doesn't match your expectation. If you want to suggest this change, add it to Discussions so we gather feedback.

dcworldwide commented 1 year ago

Ok. If you tried it yourself, you'll see that your library can't support my use case.

Controlled support is incomplete, if a tour can only be suspended after a step has run.

Will you accept a PR? It makes sense that a tour can be suspended before a step is actioned within a fully controlled use case.

gilbarbara commented 1 year ago

You can change the route and then start the tour.

I can't accept a PR for this change as it would break the current flow and require a major version, which needs some other changes that I can't deal with now.

dcworldwide commented 1 year ago

👌 I will branch or work around it for now

For context. Our tours are opened from a high level menu within our software. At the time of a tour being started, the users route could be anywhere within the software. In controlled mode, not only do we change routes, we also use events per tour step to interact with our view model ..which in turns renders to affect dom updates (i.e. open a drop down, add some text to a field).