Remove getOffset function used by scrollIntoView

[binlabs]

I noticed recently that src/internal/scroll.ts uses a getOffset() function from src/internal/offset.ts, seen below:

/**
 * Returns an element's offset relative to its parent. Similar to element.offsetTop and element.offsetLeft, except the
 * parent doesn't have to be positioned relative or absolute.
 *
 * NOTE: This was created to work around what appears to be a bug in Chrome where a slotted element's offsetParent seems
 * to ignore elements inside the surrounding shadow DOM: https://bugs.chromium.org/p/chromium/issues/detail?id=920069
 */
export function getOffset(element: HTMLElement, parent: HTMLElement) {
  return {
    top: Math.round(element.getBoundingClientRect().top - parent.getBoundingClientRect().top),
    left: Math.round(element.getBoundingClientRect().left - parent.getBoundingClientRect().left)
  };
}

The comment in the jsdoc block suggests that the function was added to get around a specific Chrome bug regarding offsets on slotted elements.

The Chrome bug that is being referenced was fixed earlier this year, so I asked @claviska if this could be removed, and he believes it can.

I tried removing the function myself but I couldn't figure out the proper offset calculations in scrollIntoView once I had it removed.

[KonnorRogers]

Hmmmm...I think we could only remove the getBoundingClientRect if we can guarantee either element won't be in a shadowRoot. For example, we point to an element in the ShadowDOM inside <sl-select> which will return 0 for offsetTop.

https://github.com/shoelace-style/shoelace/blob/next/src/components/select/select.component.ts#L639

https://www.npmjs.com/package/composed-offset-position

w3c/csswg-drafts#159

It may not be removable, but we may be able to keep getBoundClientRect as a fallback.

So The final function would look like this:

export function getOffset(element: HTMLElement, parent: HTMLElement) {
  let elementOffsetTop = element.offsetTop
  let elementOffsetLeft = element.offsetLeft
  let parentOffsetTop = parent.offsetTop
  let parentOffsetLeft = parent.offsetLeft
  
  // check we if we have an offsetParent. If this is null or undefined, we may be in a shadow root and need to do calculations.
  if (element.offsetParent == null) {
    if (elementOffsetTop === 0) {
      elementOffsetTop = element.getBoundingClientRect().top
    }
    
    if (elementOffsetLeft === 0) {
      elementOffsetLeft = element.getBoundingClientRect().left
    }
  }

  // check we if we have an offsetParent. If this is null, we may be in a shadow root and need to do calculations.
  if (parent.offsetParent == null) {
    if (parentOffsetTop === 0) {
      parentOffsetTop = parent.getBoundingClientRect().top
    }
    
    if (parentOffsetLeft === 0) {
      parentOffsetLeft = parent.getBoundingClientRect().left
    }
  }

  return {
    top: Math.round(elementOffsetTop - parentOffsetTop),
    left: Math.round(elementOffsetLeft - parentOffsetLeft)
  };
}

[KonnorRogers]

I tried to get the old / new values to match up, but something about the calculations of .offsetTop / .offsetLeft seems to be slightly different than getBoundingClientRect.left / .top values.

I'm not sure this is a huge priority fix, so I'll be closing for now, feel free to re-open with a PR.