shipshapecode / shepherd

Guide your users through a tour of your app
https://shepherdjs.dev
Other
13.02k stars 649 forks source link

Overlay path covering element to highlight when outside of the initial window #1984

Open cmazereau opened 2 years ago

cmazereau commented 2 years ago

When an element to highlight is outside of the initial window (and needs to be scrolled to), the overlay covers this element.

It appears that the svg path of the overlay is calculated using _getVisibleHeight(), which returns 0 in that case. This seems to be due to the fact that _getVisibleHeight() uses scrollParent.getBoundingClientRect(), without taking window.scrollY into account. The modification of _getVisibleHeight() as follow seems to resolve the issue :

  if (scrollParent) {
    const scrollRect = scrollParent.getBoundingClientRect();
    const scrollTop = scrollRect.y || scrollRect.top;
    const scrollBottom = scrollRect.bottom || scrollTop + scrollRect.height;
    top = Math.max(top, window.scrollY + scrollTop);
    bottom = Math.min(bottom, window.scrollY + scrollBottom);
  }
RobbieTheWagner commented 2 years ago

@cmazereau do you have a reproduction? I am not seeing this issue on https://shepherdjs.dev/

cmazereau commented 2 years ago

On https://shepherdjs.dev/, it seems that the issue does not reproduce because no scrollParent is found. But by setting the css property overflow of the body element to "auto", I could reproduce the issue on https://shepherdjs.dev/.

RobbieTheWagner commented 2 years ago

@cmazereau gotcha, if you have a fix in mind, would you mind opening a PR?

cmazereau commented 2 years ago

@rwwagner90 I don't have a satisfying fix just yet; I'm still trying to understand what exactly causes this behavior (after further investigation, it seems more complicated than I initially thought).

cmazereau commented 2 years ago

I had a little bit of time to look into this today, and found out that the problematic behavior happens when :

In this case, the height property should, from my understanding, behave as "auto" (cf https://www.w3.org/TR/CSS2/visudet.html#propdef-height). But it seems that the scrollParent actually behaves as if it were the root element (its height is expressed in percentage of the viewport - not of its content), even if it's not the "html" element.
So if the element to highlight is inside an overflown portion of the page, the function _getVisibleHeight() will consider this element to be invisible (unless the scrollParent's height is set to more than 100%), and the overlay will not show the expected opening.
I don't really know how to solve this. As a workaround, I have (for now) removed the css overflow property on my current scrollParent element, so that no scrollParent is found.

RobbieTheWagner commented 2 years ago

@cmazereau sounds like a possible browser limitation or bug?

cmazereau commented 2 years ago

@rwwagner90 I'm not sure it's a bug or a browser limitation, since I have the same behavior on 3 different browsers: