gilbarbara / react-joyride

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

Errors in console if beacon is disabled and "reset" or "close" action is triggered #478

Closed thebesson closed 5 years ago

thebesson commented 5 years ago

🐛 Bug Report

Error in console screen shot 2019-01-29 at 16 12 18

To Reproduce

Steps to reproduce the behaviour:

  1. Disable beacon on steps.
  2. Trigger "close" or "reset" action

Expected behaviour

No error

In react-joyride/src/components/Beacon.js:63

   componentDidMount() {
    if (process.env.NODE_ENV !== 'production') {
      if (!is.domElement(this.beacon)) {
        console.warn('beacon is not a valid DOM element'); //eslint-disable-line no-console
      }
    }

    if (is.domElement(this.beacon)) {
      setTimeout(() => {
        this.beacon.focus();
      }, 0);
    }
  }

Looks like in this case, the callback in setTimeout is executed after the component is unmounted and link to the beacon is lost. Is there any particular reason why there is if condition outside of setTimeout? I tried this:

setTimeout(() => {
      if (is.domElement(this.beacon)) {
        this.beacon.focus();
      }
    }, 0);

It seems to be working. I checked it with our project and with the react-joy-demo project, there are no behaviour changes. Is it ok if I create PR with those changes?

gilbarbara commented 5 years ago

Good catch! Sure, you can submit a PR.