feat: new skipOverflowHiddenElements boolean option (@icd2k3 #225)

README.md

@@ -250,6 +250,16 @@ scrollIntoView(target, {
 })
 ```
 
+#### skipOverflowHiddenElements
+
+Type: `Boolean`<br> Default: `false`
+
+> Introduced in `v2.2.0`
+
+By default the [spec](https://drafts.csswg.org/cssom-view/#scrolling-box) states that `overflow: hidden` elements should be scrollable because it has [been used to allow programatic scrolling](https://drafts.csswg.org/css-overflow-3/#valdef-overflow-hidden). This behavior can sometimes lead to [scrolling issues](https://github.com/stipsan/scroll-into-view-if-needed/pull/225#issue-186419520) when you have a node that is a child of an `overflow: hidden` node.
+
+This package follows the convention [adopted by Firefox](https://hg.mozilla.org/integration/fx-team/rev/c48c3ec05012#l7.18) of setting a boolean option to _not_ scroll all nodes with `overflow: hidden` set.
+
 # TypeScript support
 
 When the library itself is built on TypeScript there's no excuse for not publishing great library definitions!

src/compute.ts

@@ -30,15 +30,25 @@ const hasScrollableSpace = (el, axis: 'Y' | 'X') => {
 
   return false
 }
-const canOverflow = (el, axis: 'Y' | 'X') => {
+const canOverflow = (
+  el,
+  axis: 'Y' | 'X',
+  skipOverflowHiddenElements: boolean
+) => {
   const overflowValue = getComputedStyle(el, null)['overflow' + axis]
 
+  if (skipOverflowHiddenElements && overflowValue === 'hidden') {
+    return false
+  }
+
   return overflowValue !== 'visible' && overflowValue !== 'clip'
 }
 
-const isScrollable = el =>
-  (hasScrollableSpace(el, 'Y') && canOverflow(el, 'Y')) ||
-  (hasScrollableSpace(el, 'X') && canOverflow(el, 'X'))
+const isScrollable = (el, skipOverflowHiddenElements: boolean) =>
+  (hasScrollableSpace(el, 'Y') &&
+    canOverflow(el, 'Y', skipOverflowHiddenElements)) ||
+  (hasScrollableSpace(el, 'X') &&
+    canOverflow(el, 'X', skipOverflowHiddenElements))
 
 /**
  * Find out which edge to align against when logical scroll position is "nearest"
@@ -67,7 +77,7 @@ const alignNearest = (
    *          │  │
    *        ┗━│━━│━┛
    *          └──┘
-   * 
+   *
    *  If element edge C and element edge D are both outside scrolling box edge C and scrolling box edge D
    *
    *    ┏ ━ ━ ━ ━ ┓
@@ -105,7 +115,7 @@ const alignNearest = (
    *          │  │             └──┘
    *          │  │
    *          └──┘
-   * 
+   *
    * If element edge C is outside scrolling box edge C and element width is less than scrolling box width
    *
    *       from                 to
@@ -191,6 +201,7 @@ export default (
     block = 'center',
     inline = 'nearest',
     boundary,
+    skipOverflowHiddenElements = false,
   } = options
   // Allow using a callback to check the boundary
   // The default behavior is to check if the current target matches the boundary element or not
@@ -209,7 +220,10 @@ export default (
   const frames: Element[] = []
   let parent
   while (isElement((parent = target.parentNode)) && checkBoundary(target)) {
-    if (isScrollable(parent) || parent === viewport) {
+    if (
+      isScrollable(parent, skipOverflowHiddenElements) ||
+      parent === viewport
+    ) {
       frames.push(parent)
     }
 

src/types.ts

@@ -3,12 +3,16 @@ export type ScrollBehavior = 'auto' | 'instant' | 'smooth'
 export type ScrollLogicalPosition = 'start' | 'center' | 'end' | 'nearest'
 // This new option is tracked in this PR, which is the most likely candidate at the time: https://github.com/w3c/csswg-drafts/pull/1805
 export type ScrollMode = 'always' | 'if-needed'
+// New option that skips auto-scrolling all nodes with overflow: hidden set
+// See FF implementation: https://hg.mozilla.org/integration/fx-team/rev/c48c3ec05012#l7.18
+export type SkipOverflowHiddenElements = boolean
 
 export interface Options {
   block?: ScrollLogicalPosition
   inline?: ScrollLogicalPosition
   scrollMode?: ScrollMode
   boundary?: CustomScrollBoundary
+  skipOverflowHiddenElements?: SkipOverflowHiddenElements
 }
 
 // Custom behavior, not in any spec