Modal opening overflow when target is in a scrollable area

[VladimirAnaniev]

The issue

When the target element is in a scrollable area and is overflowing, the highlight does not take into account that a part of the element is not visible. The popover position alsodoes not go outside the scrollable parent even if there is available space.

Issue demonstrated in Stackblitz: https://stackblitz.com/edit/shepherd-target-scrollable?file=index.html

Proposed solution for highlight overflowing

On show of the step we can get the scrollable parent if there is one:

function getScrollParent(element: HTMLElement): HTMLElement {
      if (!element) {
         return null;
      }

      const overflowY = window.getComputedStyle(element).overflowY;
      const isScrollable = overflowY !== 'hidden' && overflowY !== 'visible';

      if (isScrollable && element.scrollHeight >= element.clientHeight) {
         return element;
      }

      return getScrollParent(element.parentElement) || document.body;
   }

Then in the render loop in shepherd-modal.svelte we can compare the scroll parent's position and our target position like so:

function getVisibleHeight(element: HTMLElement, scrollParent: HTMLElement): {top: number, height: number} {
      const elementTop = element.getBoundingClientRect().top;
      const elementBottom = element.getBoundingClientRect().bottom;

      const scrollTop = scrollParent.getBoundingClientRect().top;
      const scrollBottom = scrollParent.getBoundingClientRect().bottom;

      const top = Math.max(elementTop, scrollTop);
      const bottom = Math.min(elementBottom, scrollBottom);
      const height = bottom - top;

      return {top, height};
   }

For a horisontal scroll the implementation should be analogous. I can open a PR about this in the coming days.

Popover position

This might be an issue in the popper library. Will Tether handle this use case correctly? Is there a solution for this issue in the meantime?

[RobbieTheWagner]

I believe what you might want here is to disable scrolling entirely. If you disabled scrolling and used scrollTo would that fix the issue?

[VladimirAnaniev]

Disabling the scroll works for the smaller elements - the last two in the example. When the target element is bigger (e.g the second one in the example) we want our users to be able to scroll the whole content, so disabling the scroll is not ideal.
Still then the placement of the popover sometimes is inside the scrollable parent, even though there is space outside

[RobbieTheWagner]

The popper is going to attach to whatever selector you pass to it, so if you want it to be outside of the scroll box, you likely need to set the scroll box as the target and scroll the element in the scroll box into view manually.

[VladimirAnaniev]

In our case the layout is a bit more complex and highlighting the scrollable wrapper would be confusing. The ideal case would be to be able to target a big overflowing element like the second step in the stackblitz example while having the highlight to not overflow and the popper to be able to leave the scroll parent.

We were able to workaround the highlight overflowing with the snippets I shared, but think it would be better to have the feature in the library itself :)

[VladimirAnaniev]

Managed to fix the popover being stuck inside the scroll parent by passing tippyOptions, though it is not listed in the documentation and could not find it in the code in master. Will it be no longer available in the next versions? Is there an alternative?

[RobbieTheWagner]

@VladimirAnaniev the new version does not use tippy. popperOptions will be available though. We may or may not convert back to tether before 6.0. The docs are documenting the current state of master, so they do not include the old stuff, unfortunately.