Skip to content

Commit 77ae1e8

Browse files
ChudesnovChudesnov
authored
fix: in if-needed mode, skip bounds checking for non-scrollable scrollingElement (#915)
1 parent e2902b8 commit 77ae1e8

File tree

4 files changed

+278
-4
lines changed

4 files changed

+278
-4
lines changed

‎integration/__snapshots__/viewport-100-percent.test.js.snap

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`scrollMode: if-needed (outside the scrollingElement bounding box) horizontal completely in view 1`] = `[]`;
4+
5+
exports[`scrollMode: if-needed (outside the scrollingElement bounding box) horizontal completely overflowing 1`] = `
6+
[
7+
{
8+
"el": "html",
9+
"left": 100,
10+
"top": 0,
11+
},
12+
]
13+
`;
14+
15+
exports[`scrollMode: if-needed (outside the scrollingElement bounding box) horizontal fully negative overflowing 1`] = `
16+
[
17+
{
18+
"el": "html",
19+
"left": 800,
20+
"top": 0,
21+
},
22+
]
23+
`;
24+
25+
exports[`scrollMode: if-needed (outside the scrollingElement bounding box) horizontal partially negative overflowing 1`] = `
26+
[
27+
{
28+
"el": "html",
29+
"left": 800,
30+
"top": 0,
31+
},
32+
]
33+
`;
34+
35+
exports[`scrollMode: if-needed (outside the scrollingElement bounding box) horizontal partially overflowing 1`] = `
36+
[
37+
{
38+
"el": "html",
39+
"left": 100,
40+
"top": 0,
41+
},
42+
]
43+
`;
44+
45+
exports[`scrollMode: if-needed (outside the scrollingElement bounding box) vertical completely above the fold 1`] = `
46+
[
47+
{
48+
"el": "html",
49+
"left": 0,
50+
"top": 350,
51+
},
52+
]
53+
`;
54+
55+
exports[`scrollMode: if-needed (outside the scrollingElement bounding box) vertical completely below the fold 1`] = `
56+
[
57+
{
58+
"el": "html",
59+
"left": 0,
60+
"top": 350,
61+
},
62+
]
63+
`;
64+
65+
exports[`scrollMode: if-needed (outside the scrollingElement bounding box) vertical completely in view 1`] = `[]`;
66+
67+
exports[`scrollMode: if-needed (outside the scrollingElement bounding box) vertical partially above the fold 1`] = `
68+
[
69+
{
70+
"el": "html",
71+
"left": 0,
72+
"top": 350,
73+
},
74+
]
75+
`;
76+
77+
exports[`scrollMode: if-needed (outside the scrollingElement bounding box) vertical partially below the fold 1`] = `
78+
[
79+
{
80+
"el": "html",
81+
"left": 0,
82+
"top": 350,
83+
},
84+
]
85+
`;

‎integration/viewport-100-percent.html

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<!DOCTYPE html>
2+
<meta charset="utf-8" />
3+
<meta
4+
name="viewport"
5+
content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no"
6+
/>
7+
<script type="module" src="./utils.js"></script>
8+
<style>
9+
html,
10+
body {
11+
width: 100%;
12+
height: 100%;
13+
}
14+
body {
15+
margin: 0;
16+
}
17+
.padding {
18+
width: 100vw;
19+
height: 100vh;
20+
}
21+
.target {
22+
background: crimson;
23+
height: 100px;
24+
width: 100px;
25+
}
26+
27+
.horizontal-scroll {
28+
display: flex;
29+
position: absolute;
30+
top: 0;
31+
height: 100vh;
32+
}
33+
34+
.horizontal-scroll * {
35+
flex-shrink: 0;
36+
}
37+
</style>
38+
<div class="vertical-scroll">
39+
<div class="padding"></div>
40+
<div class="target"></div>
41+
<div class="padding"></div>
42+
</div>
43+
<div class="horizontal-scroll">
44+
<div class="padding"></div>
45+
<div class="target"></div>
46+
<div class="padding"></div>
47+
</div>

‎integration/viewport-100-percent.test.js

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
beforeAll(async () => {
2+
await page.goto('http://localhost:3000/integration/viewport-100-percent')
3+
})
4+
5+
describe('scrollMode: if-needed (outside the scrollingElement bounding box)', () => {
6+
describe('vertical', () => {
7+
test('completely below the fold', async () => {
8+
const actual = await page.evaluate(() => {
9+
window.scrollTo(0, 0)
10+
return window
11+
.computeScrollIntoView(document.querySelector('.vertical-scroll .target'), {
12+
scrollMode: 'if-needed',
13+
})
14+
.map(window.mapActions)
15+
})
16+
expect(actual).toHaveLength(1)
17+
expect(actual).toMatchSnapshot()
18+
})
19+
20+
test('partially below the fold', async () => {
21+
const actual = await page.evaluate(() => {
22+
window.scrollTo(0, 50)
23+
return window
24+
.computeScrollIntoView(document.querySelector('.vertical-scroll .target'), {
25+
scrollMode: 'if-needed',
26+
})
27+
.map(window.mapActions)
28+
})
29+
expect(actual).toHaveLength(1)
30+
expect(actual).toMatchSnapshot()
31+
})
32+
33+
test('completely in view', async () => {
34+
const actual = await page.evaluate(() => {
35+
window.scrollTo(0, window.innerHeight / 2);
36+
return window
37+
.computeScrollIntoView(document.querySelector('.vertical-scroll .target'), {
38+
scrollMode: 'if-needed',
39+
})
40+
.map(window.mapActions)
41+
})
42+
expect(actual).toHaveLength(0)
43+
expect(actual).toMatchSnapshot()
44+
})
45+
46+
test('partially above the fold', async () => {
47+
const actual = await page.evaluate(() => {
48+
window.scrollTo(0, window.innerHeight + 50)
49+
return window
50+
.computeScrollIntoView(document.querySelector('.vertical-scroll .target'), {
51+
scrollMode: 'if-needed',
52+
})
53+
.map(window.mapActions)
54+
})
55+
expect(actual).toHaveLength(1)
56+
expect(actual).toMatchSnapshot()
57+
})
58+
59+
test('completely above the fold', async () => {
60+
const actual = await page.evaluate(() => {
61+
window.scrollTo(0, window.innerHeight + 100)
62+
return window
63+
.computeScrollIntoView(document.querySelector('.vertical-scroll .target'), {
64+
scrollMode: 'if-needed',
65+
})
66+
.map(window.mapActions)
67+
})
68+
expect(actual).toHaveLength(1)
69+
expect(actual).toMatchSnapshot()
70+
})
71+
})
72+
73+
describe('horizontal', () => {
74+
test('completely overflowing', async () => {
75+
const actual = await page.evaluate(() => {
76+
window.scrollTo(0, 0)
77+
return window
78+
.computeScrollIntoView(document.querySelector('.horizontal-scroll .target'), {
79+
scrollMode: 'if-needed',
80+
})
81+
.map(window.mapActions)
82+
})
83+
expect(actual).toHaveLength(1)
84+
expect(actual).toMatchSnapshot()
85+
})
86+
87+
test('partially overflowing', async () => {
88+
const actual = await page.evaluate(() => {
89+
window.scrollTo(50, 0)
90+
return window
91+
.computeScrollIntoView(document.querySelector('.horizontal-scroll .target'), {
92+
scrollMode: 'if-needed',
93+
})
94+
.map(window.mapActions)
95+
})
96+
expect(actual).toHaveLength(1)
97+
expect(actual).toMatchSnapshot()
98+
})
99+
100+
test('completely in view', async () => {
101+
const actual = await page.evaluate(() => {
102+
window.scrollTo(window.innerWidth / 2, 0);
103+
return window
104+
.computeScrollIntoView(document.querySelector('.horizontal-scroll .target'), {
105+
scrollMode: 'if-needed',
106+
})
107+
.map(window.mapActions)
108+
})
109+
expect(actual).toHaveLength(0)
110+
expect(actual).toMatchSnapshot()
111+
})
112+
113+
test('partially negative overflowing', async () => {
114+
const actual = await page.evaluate(() => {
115+
window.scrollTo(window.innerWidth + 50, 0)
116+
return window
117+
.computeScrollIntoView(document.querySelector('.horizontal-scroll .target'), {
118+
scrollMode: 'if-needed',
119+
})
120+
.map(window.mapActions)
121+
})
122+
expect(actual).toHaveLength(1)
123+
expect(actual).toMatchSnapshot()
124+
})
125+
126+
test('fully negative overflowing', async () => {
127+
const actual = await page.evaluate(() => {
128+
window.scrollTo(window.innerWidth + 100, 0)
129+
return window
130+
.computeScrollIntoView(document.querySelector('.horizontal-scroll .target'), {
131+
scrollMode: 'if-needed',
132+
})
133+
.map(window.mapActions)
134+
})
135+
expect(actual).toHaveLength(1)
136+
expect(actual).toMatchSnapshot()
137+
})
138+
})
139+
})

‎src/index.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -392,10 +392,13 @@ export const compute = (target: Element, options: Options): ScrollAction[] => {
392392
targetLeft >= 0 &&
393393
targetBottom <= viewportHeight &&
394394
targetRight <= viewportWidth &&
395-
targetTop >= top &&
396-
targetBottom <= bottom &&
397-
targetLeft >= left &&
398-
targetRight <= right
395+
396+
// scrollingElement is added to the frames array even if it's not scrollable, in which case checking its bounds is not required
397+
((frame === scrollingElement && !isScrollable(frame)) ||
398+
(targetTop >= top &&
399+
targetBottom <= bottom &&
400+
targetLeft >= left &&
401+
targetRight <= right))
399402
) {
400403
// Break the loop and return the computations for things that are not fully visible
401404
return computations

0 commit comments

Comments
 (0)