From 3de16bdf33ff0eb9e9ae53b46758568a0f16bbf0 Mon Sep 17 00:00:00 2001 From: Alexander Chudesnov Date: Mon, 9 Dec 2024 18:10:43 +0100 Subject: [PATCH] fix: in if-needed mode, skip bounds checking for non-scrollable scrollingElement --- .../viewport-100-percent.test.js.snap | 85 +++++++++++ integration/viewport-100-percent.html | 47 ++++++ integration/viewport-100-percent.test.js | 139 ++++++++++++++++++ src/index.ts | 11 +- 4 files changed, 278 insertions(+), 4 deletions(-) create mode 100644 integration/__snapshots__/viewport-100-percent.test.js.snap create mode 100644 integration/viewport-100-percent.html create mode 100644 integration/viewport-100-percent.test.js diff --git a/integration/__snapshots__/viewport-100-percent.test.js.snap b/integration/__snapshots__/viewport-100-percent.test.js.snap new file mode 100644 index 00000000..e552a003 --- /dev/null +++ b/integration/__snapshots__/viewport-100-percent.test.js.snap @@ -0,0 +1,85 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`scrollMode: if-needed (outside the scrollingElement bounding box) horizontal completely in view 1`] = `[]`; + +exports[`scrollMode: if-needed (outside the scrollingElement bounding box) horizontal completely overflowing 1`] = ` +[ + { + "el": "html", + "left": 100, + "top": 0, + }, +] +`; + +exports[`scrollMode: if-needed (outside the scrollingElement bounding box) horizontal fully negative overflowing 1`] = ` +[ + { + "el": "html", + "left": 800, + "top": 0, + }, +] +`; + +exports[`scrollMode: if-needed (outside the scrollingElement bounding box) horizontal partially negative overflowing 1`] = ` +[ + { + "el": "html", + "left": 800, + "top": 0, + }, +] +`; + +exports[`scrollMode: if-needed (outside the scrollingElement bounding box) horizontal partially overflowing 1`] = ` +[ + { + "el": "html", + "left": 100, + "top": 0, + }, +] +`; + +exports[`scrollMode: if-needed (outside the scrollingElement bounding box) vertical completely above the fold 1`] = ` +[ + { + "el": "html", + "left": 0, + "top": 350, + }, +] +`; + +exports[`scrollMode: if-needed (outside the scrollingElement bounding box) vertical completely below the fold 1`] = ` +[ + { + "el": "html", + "left": 0, + "top": 350, + }, +] +`; + +exports[`scrollMode: if-needed (outside the scrollingElement bounding box) vertical completely in view 1`] = `[]`; + +exports[`scrollMode: if-needed (outside the scrollingElement bounding box) vertical partially above the fold 1`] = ` +[ + { + "el": "html", + "left": 0, + "top": 350, + }, +] +`; + +exports[`scrollMode: if-needed (outside the scrollingElement bounding box) vertical partially below the fold 1`] = ` +[ + { + "el": "html", + "left": 0, + "top": 350, + }, +] +`; diff --git a/integration/viewport-100-percent.html b/integration/viewport-100-percent.html new file mode 100644 index 00000000..8f4bf7e2 --- /dev/null +++ b/integration/viewport-100-percent.html @@ -0,0 +1,47 @@ + + + + + +
+
+
+
+
+
+
+
+
+
diff --git a/integration/viewport-100-percent.test.js b/integration/viewport-100-percent.test.js new file mode 100644 index 00000000..7696fcd7 --- /dev/null +++ b/integration/viewport-100-percent.test.js @@ -0,0 +1,139 @@ +beforeAll(async () => { + await page.goto('http://localhost:3000/integration/viewport-100-percent') +}) + +describe('scrollMode: if-needed (outside the scrollingElement bounding box)', () => { + describe('vertical', () => { + test('completely below the fold', async () => { + const actual = await page.evaluate(() => { + window.scrollTo(0, 0) + return window + .computeScrollIntoView(document.querySelector('.vertical-scroll .target'), { + scrollMode: 'if-needed', + }) + .map(window.mapActions) + }) + expect(actual).toHaveLength(1) + expect(actual).toMatchSnapshot() + }) + + test('partially below the fold', async () => { + const actual = await page.evaluate(() => { + window.scrollTo(0, 50) + return window + .computeScrollIntoView(document.querySelector('.vertical-scroll .target'), { + scrollMode: 'if-needed', + }) + .map(window.mapActions) + }) + expect(actual).toHaveLength(1) + expect(actual).toMatchSnapshot() + }) + + test('completely in view', async () => { + const actual = await page.evaluate(() => { + window.scrollTo(0, window.innerHeight / 2); + return window + .computeScrollIntoView(document.querySelector('.vertical-scroll .target'), { + scrollMode: 'if-needed', + }) + .map(window.mapActions) + }) + expect(actual).toHaveLength(0) + expect(actual).toMatchSnapshot() + }) + + test('partially above the fold', async () => { + const actual = await page.evaluate(() => { + window.scrollTo(0, window.innerHeight + 50) + return window + .computeScrollIntoView(document.querySelector('.vertical-scroll .target'), { + scrollMode: 'if-needed', + }) + .map(window.mapActions) + }) + expect(actual).toHaveLength(1) + expect(actual).toMatchSnapshot() + }) + + test('completely above the fold', async () => { + const actual = await page.evaluate(() => { + window.scrollTo(0, window.innerHeight + 100) + return window + .computeScrollIntoView(document.querySelector('.vertical-scroll .target'), { + scrollMode: 'if-needed', + }) + .map(window.mapActions) + }) + expect(actual).toHaveLength(1) + expect(actual).toMatchSnapshot() + }) + }) + + describe('horizontal', () => { + test('completely overflowing', async () => { + const actual = await page.evaluate(() => { + window.scrollTo(0, 0) + return window + .computeScrollIntoView(document.querySelector('.horizontal-scroll .target'), { + scrollMode: 'if-needed', + }) + .map(window.mapActions) + }) + expect(actual).toHaveLength(1) + expect(actual).toMatchSnapshot() + }) + + test('partially overflowing', async () => { + const actual = await page.evaluate(() => { + window.scrollTo(50, 0) + return window + .computeScrollIntoView(document.querySelector('.horizontal-scroll .target'), { + scrollMode: 'if-needed', + }) + .map(window.mapActions) + }) + expect(actual).toHaveLength(1) + expect(actual).toMatchSnapshot() + }) + + test('completely in view', async () => { + const actual = await page.evaluate(() => { + window.scrollTo(window.innerWidth / 2, 0); + return window + .computeScrollIntoView(document.querySelector('.horizontal-scroll .target'), { + scrollMode: 'if-needed', + }) + .map(window.mapActions) + }) + expect(actual).toHaveLength(0) + expect(actual).toMatchSnapshot() + }) + + test('partially negative overflowing', async () => { + const actual = await page.evaluate(() => { + window.scrollTo(window.innerWidth + 50, 0) + return window + .computeScrollIntoView(document.querySelector('.horizontal-scroll .target'), { + scrollMode: 'if-needed', + }) + .map(window.mapActions) + }) + expect(actual).toHaveLength(1) + expect(actual).toMatchSnapshot() + }) + + test('fully negative overflowing', async () => { + const actual = await page.evaluate(() => { + window.scrollTo(window.innerWidth + 100, 0) + return window + .computeScrollIntoView(document.querySelector('.horizontal-scroll .target'), { + scrollMode: 'if-needed', + }) + .map(window.mapActions) + }) + expect(actual).toHaveLength(1) + expect(actual).toMatchSnapshot() + }) + }) +}) diff --git a/src/index.ts b/src/index.ts index 0921df1a..83f1e702 100644 --- a/src/index.ts +++ b/src/index.ts @@ -392,10 +392,13 @@ export const compute = (target: Element, options: Options): ScrollAction[] => { targetLeft >= 0 && targetBottom <= viewportHeight && targetRight <= viewportWidth && - targetTop >= top && - targetBottom <= bottom && - targetLeft >= left && - targetRight <= right + + // scrollingElement is added to the frames array even if it's not scrollable, in which case checking its bounds is not required + ((frame === scrollingElement && !isScrollable(frame)) || + (targetTop >= top && + targetBottom <= bottom && + targetLeft >= left && + targetRight <= right)) ) { // Break the loop and return the computations for things that are not fully visible return computations