shepherd-pro / react-shepherd

A React wrapper for the site tour library Shepherd
https://react-shepherd.vercel.app
641 stars 36 forks source link

Tour renders at top left of page after component updates, creates new tour instance for component #713

Open dchersey opened 1 year ago

dchersey commented 1 year ago

Hi, I am using react-shepherd 4.2.0, and have defined a single Shepherd context in App.jsx as the highest level component.

        <ShepherdTour steps={[]} tourOptions={tourOptions}>
          {children}
        {modal.openModals.map((m) => {
          const modalId = m;
          const modalProps = modal.modalInfo[m];
          return (
            <ModalErrorBoundary key={`error-boundary-${modalId}}`}>
                <ModalWrapper
                  key={modalId}
                  id={modalId || ''}
                  type={modalProps.type}
                  modal={modalProps}
                  close={
                    modalProps.props && modalProps.props.onClose
                      ? modalProps.props.onClose
                      : this.props.closeModal
                  }
                />
            </ModalErrorBoundary>
          );
        })}
        </ShepherdTour>

I have created a component that is rendered on one of my modal views, with steps for that view, to which I pass in step definitions in props.

import React, { useEffect, useContext } from 'react';
import { ShepherdTourContext } from 'react-shepherd';

export default function GuideMe(props) {
  const tour = useContext(ShepherdTourContext);
  const { steps } = props;

  steps.forEach((step) => {
    const buttons = step.buttons.map((button) => {
      let { action } = button;
      switch (button.action) {
        case 'next':
          action = tour.next.bind(tour);
          break;
        case 'back':
          action = tour.back.bind(tour);
          break;
        case 'cancel':
          action = tour.cancel.bind(tour);
          break;
        case 'complete':
          action = tour.complete.bind(tour);
          break;
        default:
      }
      return { ...button, action };
    });
    tour.addStep({ ...step, buttons });
  });

  return (
    <button type="button" className="button dark" onClick={tour.start}>
      Guide Me
    </button>
  );
}

The step in question looks like:

{
      id: 'organization-detail',
      text: 'Add important information for your organization on the Details tab',
      attachTo: {
        element: 'a#details',
        on: 'bottom'
      },
      classes: '',
      buttons: [
        {
          text: 'Next',
          action: 'next'
        }
      ]
    }

The problem I am having is that when a user hits one of the tabs on my modal, causing it to re-render, if the tour is active it now displays in the top left of the full page.
Screenshot 2023-05-30 at 6 28 49 PM

Screenshot 2023-05-30 at 6 31 27 PM

You can see that the little arrow is still visible on the left, so it does not seem to be the case that it can't resolve my selector. (I tried using a function to return the selector but that did not change anything).

What is even more interesting is that if I click my GuideMe button again while it is in this state, a second tour opens in the proper place.

Screenshot 2023-05-30 at 6 34 11 PM

I am thinking this could be related to #587 but I also see that has been fixed by memoizing -- still when I add this code:

  useEffect(() => {
    console.log('Shep tour', tour.id, tour.isActive());
  }, [props.tourActive]);

to my GuidMe component, I get this output:

GuideMe.jsx:9 Shep tour tour--a4f64767-8f9c-42cf-9bb4-5b75ea7a3524 false
18:38:26.839 GuideMe.jsx:9 Shep tour tour--a4f64767-8f9c-42cf-9bb4-5b75ea7a3524 true
18:38:26.849 GuideMe.jsx:9 Shep tour tour--2aa87e5f-6639-43d6-ab7f-70dd167d241e false
18:38:34.254 GuideMe.jsx:9 Shep tour tour--2c9118c2-da3e-46f4-9a70-d0dd1430439c false
18:38:34.263 GuideMe.jsx:9 Shep tour tour--a89b67cd-c41b-415e-90b8-afb76981402a false

as I click around between tabs with the tour open. Note different tour.id and that the last 3 are inactive even though the first one is still showing.

So somehow, I am getting more than one tour, and the first one is getting disconnected from the ui? If I click the button again, the new one renders in the right place but the orphaned one is still visible.

Any thoughts, most welcome!!!

dchersey commented 1 year ago

Bump on this....

chuckcarpenter commented 1 year ago

@dchersey sorry, haven't had time to chime in on this. It seems odd that on rerender it doesn't pickup the active tour, but perhaps something is different enough in memory that it's creating a new one. Would you be able to share a codesandbox or something to help replicate the issue for debugging?

chuckcarpenter commented 1 year ago

@dchersey I'm having a pretty tough time trying to replicate. I'm wondering if using the beforeShowPromise would be useful here. Also, no clear on why the Tour is the top level component when you want it in the modal context. Docs on that method: https://shepherdjs.dev/docs/tutorial-02-usage.html

AHarmlessPyro commented 9 months ago

If you're using nextjs, part of the issue seems to be that the provider is mounted not on the layout file, but in a component/page. So it may get rendered twice, once on the server and once on the client. Moving the provider to the layout file helped me. https://nextjs.org/docs/app/building-your-application/routing/pages-and-layouts#root-layout-required

So, something like:

export default function RootLayout({
  children,
}: {
  children: React.ReactNode;

}) {
// OnboardingComponent is a wrapper for the ShepherdTour component
  return (
    <html lang="en">
      <body className={inter.className}>
        <>
          <OnboardingComponent steps={[]}>
            {children}
            <Toaster />
          </OnboardingComponent>
        </>
      </body>
    </html>
  );
}
Battjmo commented 9 months ago

If you're using nextjs, part of the issue seems to be that the provider is mounted not on the layout file, but in a component/page. So it may get rendered twice, once on the server and once on the client. Moving the provider to the layout file helped me. https://nextjs.org/docs/app/building-your-application/routing/pages-and-layouts#root-layout-required

So, something like:

export default function RootLayout({
  children,
}: {
  children: React.ReactNode;

}) {
// OnboardingComponent is a wrapper for the ShepherdTour component
  return (
    <html lang="en">
      <body className={inter.className}>
        <>
          <OnboardingComponent steps={[]}>
            {children}
            <Toaster />
          </OnboardingComponent>
        </>
      </body>
    </html>
  );
}

This is with pages router yeah?

AHarmlessPyro commented 9 months ago

It's the app router in this case iirc, but it's been a while since I last looked at this. I would imagine that it's largely similar for the pages router as well.

Battjmo commented 9 months ago

Ahh I see, <OnboardingComponent> is a client component, so it works, yeah?

Battjmo commented 9 months ago

@AHarmlessPyro While I've got someone who uses this library in Next.js here (this is at least tangentially related since it involves positioning): have you ever had an issue where your tutorial popups are missing their left positioning until you scroll? Thanks 🙏