Highlight spotlight sometimes is displayed in improper position (scroll related)

[keul]

Expected behavior

The highlight overlay should be placed normally

Actual behavior

In some cases if I use scrollToFirstStep the highlight overlay (please note: not the overlay of the first step! Weird...) is placed at the wrong position.

See image below:

The highlight overlay should be placed on the plus icon (note that the Floater is at the right position). All is working perfectly when i put scrollToFirstStep={false}.

Steps to reproduce the problem

That's the sad part... I tried to reproduce with same React and Joyride versions at https://codesandbox.io/s/q7oopyqwkq with no luck. I'm not able to find what is going on there.

If you have any suggestion helping debug this issue let me know, I understand maybe I'm able to provide enough information, so feel free to close the issue if it's not enough for an idea from your side.

React version

15.6.2

React-Joyride version

2.0.0-11

[keul]

I modified the issue title because I found that scrollToFirstStep is not the real issue here. This is related to page scrolling (so this is why probably scrollToFirstStep trigger the issue in my env): if I manually scroll the page, on some steps spotlight is badly positioned too. I think that the issue is on calculating the some values in stylesSpotlight.

Keep looking into this...

[keul]

OK, I think I found the origin of the issue so I was able to reproduce it on a codesandbox...

https://q7oopyqwkq.codesandbox.io/

To reproduce the issue you should resize the browser and use scrolling, for easily explain it here two animation.

Here we don't have the issue (I didn't scrolled anywhere):

Here the same application but I scrolled a little the page, so you can see the bug:

Reason

Seems that problem came from scrollparent.js dependency (so maybe is a bug in this library?).
There's an explicit management of overflow css prop: https://github.com/olahol/scrollparent.js/blob/dd25def14fa307cd4dc0c71460808232cb797100/scrollparent.js#L23

As soon as one of parent element use overflow we fall into this problem.

I don't fully understand why scrollparent.js is doing this. Any idea?

[nicubarbaros]

Did anyone get a fix for this?

[keul]

@nicubarbaros no, I just tried to remove overflow usage form my CSS where I was able to do so.

[nicubarbaros]

@keul Damn it. I just removed the spotlight for now. Great plugin but has this issue which is quite harmful.

[gilbarbara]

Can you please try [email protected] and let me know if the problem is there?
Please post an example in codesandbox.

[keul]

@gilbarbara tested the same codesandbox above (still https://codesandbox.io/s/q7oopyqwkq) after updating to 2.0.0-12 and I've the same issue (the gif I attached is still valid to reproduce the error).

[keul]

A quick-and-dirty test for posterity.

As I said above the issue seems to came from scrollparent.js but I get no answer from them.

If I manually modify the scrollparent.js source file inside my node_modules, removing the scroll check, the error is fixed in my environment.

See commented lines below.

(function (root, factory) {
  if (typeof define === "function" && define.amd) {
    define([], factory);
  } else if (typeof module === "object" && module.exports) {
    module.exports = factory();
  } else {
    root.Scrollparent = factory();
  }
}(this, function () {
  var regex = /(auto|scroll)/;

  var parents = function (node, ps) {
    if (node.parentNode === null) { return ps; }

    return parents(node.parentNode, ps.concat([node]));
  };

  var style = function (node, prop) {
    return getComputedStyle(node, null).getPropertyValue(prop);
  };

  var overflow = function (node) {
    return style(node, "overflow") + style(node, "overflow-y") + style(node, "overflow-x");
  };

  var scroll = function (node) {
   return regex.test(overflow(node));
  };

  var scrollParent = function (node) {
    if (!(node instanceof HTMLElement || node instanceof SVGElement)) {
      return ;
    }

    var ps = parents(node.parentNode, []);

    // for (var i = 0; i < ps.length; i += 1) {
    //   if (scroll(ps[i])) {
    //     return ps[i];
    //   }
    // }

    return document.scrollingElement || document.documentElement;
  };

  return scrollParent;
}));

I'm 100% sure that this code is there for a reason, I guess for handling overflow: scroll or auto usecases. So I'm sure that this fix can't work in every case, but seems good enough for me.

[j0o009k]

This fix does cause other issues with some components, in my experience. For one component it is throwing the floater off in a big way. Very frustrating.

[keul]

@j0o009k as I said above: my fix worked in my environment. I guess you have components where the use of overflow matters.

[simonwaw]

@keul - I also have the same issues in my app - this hack to scrollParent also resolves the issue for me. @gilbarbara any chance we can remove the need for this dependancy in a future version?

[sturdynut]

Super hack
If you are in need of an immediate fix...this worked well enough for me. You may also want to isolate this to only run for a specific step.

I put this in the callback for the tour.

Code example:

if (lifecycle === 'tooltip') {
  const { target } = yourSteps[index];
  const targetEl = $(target);
  if (targetEl) {
    window.setTimeout(() => {
      const spotlight = $('.joyride-spotlight');
      if (spotlight) {
        const spotlightTop = spotlight.offset().top;
        const targetTop = targetEl.offset().top - 10;
        const diff = spotlightTop > targetTop
          ? spotlightTop - targetTop
          : targetTop - spotlightTop;
        if (diff > 1) {
          spotlight.css('transform', `translateY(${diff}px)`);
        }
      }
    }, 200);
  }
}

Disclaimer: I have not thoroughly tested this (Chrome only); it was just a quick hack that got me by until the issue is fixed.

[catamphetamine]

We've just tried upgrading to v2 and we immediately stumbled upon the issue.
If the user doesn't scroll at all while the "tour" is being shown then it's not buggy (it still doesn't automatically scroll the next step into view but that's some another issue).
If the user scrolls during the "tour" though then the "spotlight" is positioned incorrectly.
It's not a blocker though and we'll just roll back to v1 which works fine-enough.

[gilbarbara]

@catamphetamine Can you please try again with 2.0.0-rc and let me know?

[catamphetamine]

@gilbarbara
Same issue.
The tooltip is positioned correctly, but the "spotlight" is not.
It looks as if you were subtracting window.scrollY from spotlight.top: if window.scrollY is 0 then the "spotlight" is positioned correctly, but when window.scrollY is N then spotlight.top is N pixels less than it should have been.

[gilbarbara]

@catamphetamine
I still can't reproduce this behavior. The spotlight works in all 5 demos, including while scrolling.
Can you post a demo in codesandbox?

[catamphetamine]

@gilbarbara I guess it's our code then. Which I can't share. And I wouldn't want to waste your time on digging through some company's website structure. So don't bother, we're fine with v1. Thx.

[keul]

@gilbarbara is something changed in the lib packaging? I tried the same old codesandbox with latest 2.0.0-rc but now I get an error:

ModuleNotFoundError
Could not find module in path: 'react-joyride/es/constants' relative to '/src/index.js'

...while doing...

import { ACTIONS, EVENTS } from "react-joyride/es/constants";

[keul]

...ok, I changed my code to import { ACTIONS, EVENTS } from "react-joyride"; and now it compiles.

However I have the same position issue: https://codesandbox.io/s/q7oopyqwkq

[gilbarbara]

@keul I'll try to fix it today! :)

[gilbarbara]

@keul Can you please try this sandbox and see if the problem continues?

[keul]

Ehi @gilbarbara it's working there! 🎉

I don't fully understand what is doing the trick from my codesandbox to your ones! Can you enlight me?

[gilbarbara]

@keul The .node class had an overflow-y property, which messes up with the scrolling detection.
I'm testing a new scroll parent detection to avoid this problem.

[gilbarbara]

2.0.0-rc.1 adds a dirty fix for unused scroll parents with overflow that should handle most cases.
If you still are having problems, let me know

[keul]

I can confirm, my codesandbox above is now working! Thanks for your time, really appreciated.

[harunurhan]

This still happens to me with the very basic use case, when step target out of viewport (down), so it scrolls down, tooltip is correctly placed, but the spotlight is somewhere up in the page.

@gilbarbara

[gilbarbara]

Hey @harunurhan

Please provide a https://codesandbox.io/ demo or similar.

[harunurhan]

@gilbarbara I think my issue is similar to #548, and as it was said in that issue, it is tricky to provide a sandbox reproduction.

[gilbarbara]

@harunurhan Ok, but that's impossible for me to help without seeing what's going on with the code...

[ekfuhrmann]

For those who come across this issue and are looking for a solution, I was having a similar problem with the spotlight being offset when the user had scrolled slightly down the page. It ended up being that in my case I had height: 100% on my html element and that was throwing off the auto scroller and by extension the spotlight positioning. Removing height: 100% in my case resolved the issue and might be worth trying for those who run into a similar problem.