Skip to content

Commit 7e7fd23

Browse files
thetthet
committed
fix(pat scroll-box): Immediately and correctly set CSS classes.
Fix pat-scroll-box to immediately and correctly set the CSS classes by using requestAnimationFrame instead timeouts.
1 parent 949b1dc commit 7e7fd23

File tree

2 files changed

+62
-89
lines changed

2 files changed

+62
-89
lines changed

‎src/pat/scroll-box/scroll-box.js‎

Lines changed: 19 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,12 @@
11
import Base from "../../core/base";
2-
3-
export const TIMEOUT_FIRST_CALLBACK = 40; // Timeout for first run of the callback
4-
export const TIMEOUT_CALLBACK = 200; // Timeout for subsequent runs of the callback
5-
export const TIMEOUT_PAUSE = 600; // Timeout to detect scrolling pause and reset for TIMEOUT_FIRST_CALLBACK
2+
import events from "../../core/events";
63

74
export default Base.extend({
85
name: "scroll-box",
96
trigger: ".pat-scroll-box",
107

118
scroll_listener: null,
129
last_known_scroll_position: 0,
13-
timeout_id: null,
1410
timeout_id__scroll_stop: null,
1511

1612
init() {
@@ -26,43 +22,28 @@ export default Base.extend({
2622
return;
2723
}
2824

29-
this.scroll_listener.addEventListener("scroll", () => {
30-
if (this.timeout_id === null) {
31-
// Run first callback early and don't clear it's scheduled run.
32-
// This sets up classes early for a better user experience in
33-
// contrast of setting classes at the end of scrolling.
34-
window.setTimeout(() => {
35-
const scroll_y = this.get_scroll_y();
36-
this.set_scroll_classes(scroll_y);
37-
this.last_known_scroll_position = scroll_y;
38-
}, TIMEOUT_FIRST_CALLBACK);
39-
// Set a dummy timeout_id and return.
40-
// Next scroll event should not reach this block but start with
41-
// default callback scheduling.
42-
this.timeout_id = 0;
43-
return;
25+
let ticking = false;
26+
events.add_event_listener(
27+
this.scroll_listener,
28+
"scroll",
29+
"pat-scroll-box--scroll_listener",
30+
() => {
31+
if (!ticking) {
32+
window.requestAnimationFrame(() => {
33+
this.set_scroll_classes();
34+
ticking = false;
35+
});
36+
ticking = true;
37+
}
4438
}
45-
46-
window.clearTimeout(this.timeout_id);
47-
this.timeout_id = window.setTimeout(() => {
48-
const scroll_y = this.get_scroll_y();
49-
this.set_scroll_classes(scroll_y);
50-
this.last_known_scroll_position = scroll_y;
51-
}, TIMEOUT_CALLBACK);
52-
53-
// Reset the timeout_id after when he user stops scrolling.
54-
// When scrolling again the scroll classes are set again after TIMEOUT_FIRST_CALLBACK
55-
window.clearTimeout(this.timeout_id__scroll_stop);
56-
this.timeout_id__scroll_stop = window.setTimeout(() => {
57-
this.timeout_id = null;
58-
}, TIMEOUT_PAUSE);
59-
});
39+
);
6040

6141
// Set initial state
62-
this.set_scroll_classes(this.get_scroll_y());
42+
this.set_scroll_classes();
6343
},
6444

65-
set_scroll_classes(scroll_pos) {
45+
set_scroll_classes() {
46+
const scroll_pos = this.get_scroll_y();
6647
const el = this.el;
6748
el.classList.remove("scroll-up");
6849
el.classList.remove("scroll-down");
@@ -88,6 +69,7 @@ export default Base.extend({
8869
) {
8970
el.classList.add("scroll-position-bottom");
9071
}
72+
this.last_known_scroll_position = scroll_pos;
9173
},
9274

9375
get_scroll_y() {

‎src/pat/scroll-box/scroll-box.test.js‎

Lines changed: 43 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,73 +1,64 @@
1-
import pattern, {
2-
TIMEOUT_CALLBACK,
3-
TIMEOUT_FIRST_CALLBACK,
4-
TIMEOUT_PAUSE,
5-
} from "./scroll-box";
1+
import pattern from "./scroll-box";
62
import utils from "../../core/utils";
3+
import events from "../../core/events";
74

85
describe("pat-scroll-box", function () {
96
afterEach(function () {
107
jest.restoreAllMocks();
118
document.body.innerHTML = "";
129
});
1310

14-
it("1 - First callback invoked early", async function () {
11+
it("1 - Basic functionality", async function () {
1512
document.body.innerHTML = `
1613
<div id="el1" style="overflow: scroll"></div>
1714
`;
1815
const el = document.querySelector("#el1");
1916

20-
const instance = new pattern(el);
17+
// mocks
18+
Object.defineProperty(el, "clientHeight", { value: 100, writable: false });
19+
Object.defineProperty(el, "scrollHeight", { value: 300, writable: false });
20+
Object.defineProperty(el, "scrollTop", { value: 0, writable: true });
21+
22+
new pattern(el);
2123
await utils.timeout(1);
2224

23-
const spy_set_scroll_classes = jest.spyOn(instance, "set_scroll_classes");
25+
el.scrollTop = 0;
26+
el.dispatchEvent(events.scroll_event());
27+
expect(el.classList).toContain("scroll-position-top");
28+
expect(el.classList).not.toContain("scroll-position-bottom");
29+
expect(el.classList).not.toContain("scroll-up");
30+
expect(el.classList).not.toContain("scroll-down");
2431

25-
// First invocation is after TIMEOUT_FIRST_CALLBACK
26-
el.dispatchEvent(new Event("scroll"));
27-
await utils.timeout(1);
28-
expect(spy_set_scroll_classes).toHaveBeenCalledTimes(0);
29-
await utils.timeout(TIMEOUT_FIRST_CALLBACK - 1);
30-
expect(spy_set_scroll_classes).toHaveBeenCalledTimes(1);
31-
// No other callback invocations with just one scroll event.
32-
await utils.timeout(TIMEOUT_CALLBACK - TIMEOUT_FIRST_CALLBACK - 1);
33-
expect(spy_set_scroll_classes).toHaveBeenCalledTimes(1);
32+
el.scrollTop = 100;
33+
el.dispatchEvent(events.scroll_event());
34+
await utils.animation_frame();
35+
expect(el.classList).not.toContain("scroll-position-top");
36+
expect(el.classList).not.toContain("scroll-position-bottom");
37+
expect(el.classList).not.toContain("scroll-up");
38+
expect(el.classList).toContain("scroll-down");
3439

35-
// Now, subsequent scroll events invoke the callback after TIMEOUT_CALLBACK
36-
// But multiple scroll events don't lead to multiple callback invocations.
37-
el.dispatchEvent(new Event("scroll"));
38-
await utils.timeout(1);
39-
el.dispatchEvent(new Event("scroll"));
40-
await utils.timeout(1);
41-
el.dispatchEvent(new Event("scroll"));
42-
// After TIMEOUT_FIRST_CALLBACK no NEW cb invocation
43-
await utils.timeout(TIMEOUT_FIRST_CALLBACK);
44-
expect(spy_set_scroll_classes).toHaveBeenCalledTimes(1);
45-
// After TIMEOUT_CALLBACK there should be another cb invocation
46-
await utils.timeout(TIMEOUT_CALLBACK - TIMEOUT_FIRST_CALLBACK);
47-
expect(spy_set_scroll_classes).toHaveBeenCalledTimes(2);
48-
// But without a new scroll event no more callback invocations.
49-
await utils.timeout(TIMEOUT_CALLBACK);
50-
expect(spy_set_scroll_classes).toHaveBeenCalledTimes(2);
40+
el.scrollTop = 50;
41+
el.dispatchEvent(events.scroll_event());
42+
await utils.animation_frame();
43+
expect(el.classList).not.toContain("scroll-position-top");
44+
expect(el.classList).not.toContain("scroll-position-bottom");
45+
expect(el.classList).toContain("scroll-up");
46+
expect(el.classList).not.toContain("scroll-down");
5147

52-
// Another scroll event, another callback invocation after 200ms
53-
// We have to dispatch again
54-
el.dispatchEvent(new Event("scroll"));
55-
// After TIMEOUT_FIRST_CALLBACK no NEW cb invocation
56-
await utils.timeout(TIMEOUT_FIRST_CALLBACK);
57-
expect(spy_set_scroll_classes).toHaveBeenCalledTimes(2);
58-
// After TIMEOUT_CALLBACK there should be another cb invocation
59-
await utils.timeout(TIMEOUT_CALLBACK - TIMEOUT_FIRST_CALLBACK);
60-
expect(spy_set_scroll_classes).toHaveBeenCalledTimes(3);
48+
el.scrollTop = 200;
49+
el.dispatchEvent(events.scroll_event());
50+
await utils.animation_frame();
51+
expect(el.classList).not.toContain("scroll-position-top");
52+
expect(el.classList).toContain("scroll-position-bottom");
53+
expect(el.classList).not.toContain("scroll-up");
54+
expect(el.classList).toContain("scroll-down");
6155

62-
// Let's wait for TIMEOUT_PAUSE to reset and start again with the
63-
// first call after TIMEOUT_FIRST_CALLBACK.
64-
// The user probably stopped scrolling and starts over again
65-
await utils.timeout(TIMEOUT_PAUSE);
66-
el.dispatchEvent(new Event("scroll"));
67-
await utils.timeout(TIMEOUT_FIRST_CALLBACK);
68-
expect(spy_set_scroll_classes).toHaveBeenCalledTimes(4);
69-
el.dispatchEvent(new Event("scroll"));
70-
await utils.timeout(TIMEOUT_CALLBACK);
71-
expect(spy_set_scroll_classes).toHaveBeenCalledTimes(5);
56+
el.scrollTop = 0;
57+
el.dispatchEvent(events.scroll_event());
58+
await utils.animation_frame();
59+
expect(el.classList).toContain("scroll-position-top");
60+
expect(el.classList).not.toContain("scroll-position-bottom");
61+
expect(el.classList).toContain("scroll-up");
62+
expect(el.classList).not.toContain("scroll-down");
7263
});
7364
});

0 commit comments

Comments
 (0)