Skip to content

Commit 2976ba7

Browse files
authored
Merge pull request #22 from atom-ide-community/scroll
2 parents d041488 + 2c78720 commit 2976ba7

File tree

4 files changed

+78
-1
lines changed

4 files changed

+78
-1
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,4 @@ package-lock.json
1212
# Build directories
1313
package
1414
commons-atom
15+
commons-ui

package.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,16 @@
2828
"lint": "eslint . --fix",
2929
"test": "atom --test spec",
3030
"clean": "shx rm -rf package commons-atom",
31+
"tsc.commons-ui": "tsc -p src-commons-ui/tsconfig.json || echo done",
32+
"dev.commons-ui": "tsc -w -p src-commons-ui/tsconfig.json",
33+
"build.commons-ui": "npm run tsc.commons-ui",
3134
"tsc.commons-atom": "tsc -p src-commons-atom/tsconfig.json || echo done",
3235
"dev.commons-atom": "tsc -w -p src-commons-atom/tsconfig.json",
3336
"build.commons-atom": "npm run tsc.commons-atom",
3437
"tsc.package": "tsc -p src-package/tsconfig.json || echo done",
3538
"dev.package": "npm run clean && cross-env NODE_ENV=development cross-env BABEL_KEEP_MODULES=true rollup -c -w",
3639
"build.package": "npm run clean && cross-env NODE_ENV=production cross-env BABEL_KEEP_MODULES=true rollup -c",
37-
"build": "npm run build.package && npm run build.commons-atom",
40+
"build": "npm run build.package && npm run build.commons-atom && npm run build.commons-ui",
3841
"build-commit": "build-commit -o dist",
3942
"bump": "ncu -u -x coffeescript",
4043
"prepare": "npm run build"

src-commons-ui/scrollIntoView.ts

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/* globals getComputedStyle */
2+
3+
/**
4+
* Use these functions instead of `Element::scrollIntoView()` and
5+
* `Element::scrollIntoViewIfNeeded()`!
6+
*
7+
* We've had a recurring issue in Nuclide (e.g. T20028138) where the UI would shift, leaving part of
8+
* the workspace element offscreen and a blank area in the window. This was caused by called to the
9+
* native `scrollIntoView()` and `scrollIntoViewIfNeeded()` which, according to the spec, has two
10+
* potentially surprising behaviors:
11+
*
12+
* 1. [It scrolls every scrollable ancestor (not just the closest)][1], where
13+
* 2. "scrollable" is [explicitly defined][2] to include elements with `overflow: hidden`
14+
*
15+
* This is surprising because `overflow: hidden` is typically used to make elements *not
16+
* scrollable*.
17+
*
18+
* Once the `overflow: hidden` element is scrolled, the user has no way to return it to its original
19+
* position (as it has no scrollbars).
20+
*
21+
* Note that this API doesn't support smooth scrolling. If that becomes necessary, we'll need to
22+
* come up with a better fix.
23+
*
24+
* It's tempting to assume that using `scrollIntoViewIfNeeded()` would fix this issue, however, if
25+
* the window is small enough so that no amount of scrolling the desired scrollable element would
26+
* ever reveal the element you're trying to, the browser will keep scrolling ancestors.
27+
*
28+
* [1]: https://drafts.csswg.org/cssom-view/#element-scrolling-members
29+
* [2]: https://drafts.csswg.org/cssom-view/#scrolling-box
30+
*/
31+
32+
export function scrollIntoView(el: Element, alignToTop?: boolean): void {
33+
const scrollTops = getScrollTops(el)
34+
el.scrollIntoView(alignToTop)
35+
restoreOverflowHiddenScrollTops(scrollTops)
36+
}
37+
38+
export function scrollIntoViewIfNeeded(el: Element, center?: boolean): void {
39+
const scrollTops = getScrollTops(el)
40+
el?.scrollIntoViewIfNeeded(center) ?? el.scrollIntoView(center)
41+
restoreOverflowHiddenScrollTops(scrollTops)
42+
}
43+
44+
function getScrollTops(el_: Element): Map<Element, number> {
45+
let el: Element | null = el_
46+
const scrollTops = new Map()
47+
while (el != null) {
48+
scrollTops.set(el, el.scrollTop)
49+
el = el.parentElement
50+
}
51+
return scrollTops
52+
}
53+
54+
function restoreOverflowHiddenScrollTops(scrollTops: Map<Element, number>): void {
55+
scrollTops.forEach((scrollTop, el) => {
56+
if (el.scrollTop !== scrollTop && isOverflowHidden(el)) {
57+
el.scrollTop = scrollTop
58+
}
59+
})
60+
}
61+
62+
function isOverflowHidden(el: HTMLElement | SVGElement | Element): boolean {
63+
//@ts-ignore
64+
const overflowStyle = el?.style.overflow
65+
const overflow = overflowStyle ?? getComputedStyle(el).overflow
66+
return overflow === "hidden"
67+
}

src-commons-ui/tsconfig.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"extends": "../tsconfig.json",
3+
"compilerOptions": {
4+
"outDir": "../commons-ui"
5+
}
6+
}

0 commit comments

Comments
 (0)