Skip to content

Adds skipOverflowHiddenElements boolean option #225

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Adds skipOverflowHiddenElements boolean option + docs
chore(deps): update dependency lint-staged to v7.1.0 (#224)

chore(deps): update dependency lint-staged to v7.1.0 (#224)

fix(typings): Improve TS typings (#223)

* prepare to add flowtype tests

* improve the quality of the typescript libdefs

* Improve typescript declarations

* improve further

* update readme etc

* Update index.ts

chore(deps): update dependency flowgen to v1.2.2 (#226)

fix(compute): custom behavior lack block and inline defaults (#227)

fix(compute): custom behavior missing block and inline defaults

chore(deps): update dependency semantic-release to v15.4.0 (#229)

chore(readme): I think about Jest… a LOT! (#230)
  • Loading branch information
icd2k3 committed May 10, 2018
commit 1fed7e8be59fe49345751000c5adc834058ca478
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice level of detail on the documentation here 😄


# TypeScript support

When the library itself is built on TypeScript there's no excuse for not publishing great library definitions!
Expand Down
28 changes: 21 additions & 7 deletions src/compute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
*
* ┏ ━ ━ ━ ━ ┓
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
}

Expand Down
4 changes: 4 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work on making sure things are well documented 👍

// 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
Expand Down
Empty file added types.js
Empty file.