From e74939ab0fde422e2c4327ad3fb26053f5c68dad Mon Sep 17 00:00:00 2001 From: iamnathanj Date: Thu, 3 Nov 2022 21:08:12 +0000 Subject: [PATCH 1/4] fix: scrollable calls onScrolledToBottom only when container is overflowing --- polaris-react/src/components/Scrollable/Scrollable.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/polaris-react/src/components/Scrollable/Scrollable.tsx b/polaris-react/src/components/Scrollable/Scrollable.tsx index 0edb4ab38b4..6d92c4f79ed 100644 --- a/polaris-react/src/components/Scrollable/Scrollable.tsx +++ b/polaris-react/src/components/Scrollable/Scrollable.tsx @@ -67,6 +67,7 @@ export function Scrollable({ requestAnimationFrame(() => { const {scrollTop, clientHeight, scrollHeight} = currentScrollArea; + const canScroll = Boolean(scrollHeight > clientHeight); const isBelowTopOfScroll = Boolean(scrollTop > 0); const isAtBottomOfScroll = Boolean( scrollTop + clientHeight >= scrollHeight - LOW_RES_BUFFER, @@ -75,7 +76,7 @@ export function Scrollable({ setTopShadow(isBelowTopOfScroll); setBottomShadow(!isAtBottomOfScroll); - if (isAtBottomOfScroll && onScrolledToBottom) { + if (canScroll && isAtBottomOfScroll && onScrolledToBottom) { onScrolledToBottom(); } }); From 6b60a0e85e5bfa0aff20187556ea0daaca684d90 Mon Sep 17 00:00:00 2001 From: iamnathanj Date: Fri, 4 Nov 2022 11:24:20 -0400 Subject: [PATCH 2/4] add tests for onScrolledToBottom --- .../Scrollable/tests/Scrollable.test.tsx | 57 +++++++++++++++++++ polaris-react/tests/setup/tests.ts | 5 ++ 2 files changed, 62 insertions(+) diff --git a/polaris-react/src/components/Scrollable/tests/Scrollable.test.tsx b/polaris-react/src/components/Scrollable/tests/Scrollable.test.tsx index 5572dbcb86a..480a0bba49d 100644 --- a/polaris-react/src/components/Scrollable/tests/Scrollable.test.tsx +++ b/polaris-react/src/components/Scrollable/tests/Scrollable.test.tsx @@ -3,6 +3,7 @@ import {mountWithApp} from 'tests/utilities'; import {Scrollable} from '../Scrollable'; import {ScrollableContext} from '../context'; +import type {ScrollableProps} from '../Scrollable'; describe('', () => { it('mounts', () => { @@ -71,4 +72,60 @@ describe('', () => { tabIndex: 0, }); }); + + it('calls onScrolledToBottom when scrolled to bottom', () => { + const onScrolledToBottom = jest.fn(); + + const scrollArea = mountWithApp( + +

Hello

+
, + ); + + const scrollNode = scrollArea.find('div', { + 'data-test-id': 'scroll-element', + } as ScrollableProps)?.domNode!; + + // defineProperty needed to assign values to readonly node properties + Object.defineProperty(scrollNode, 'clientHeight', {get: () => 0}); + Object.defineProperty(scrollNode, 'scrollHeight', {get: () => 10}); + Object.defineProperty(scrollNode, 'scrollTop', {get: () => 10}); + + scrollArea.act(() => { + scrollNode.dispatchEvent(new Event('scroll')); + }); + + expect(onScrolledToBottom).toHaveBeenCalledTimes(1); + }); + + it(`doesn't call onScrolledToBottom when the scroll area is not overflowing`, () => { + const onScrolledToBottom = jest.fn(); + + const scrollArea = mountWithApp( + +

Hello

+
, + ); + + const scrollNode = scrollArea.find('div', { + 'data-test-id': 'scroll-element', + } as ScrollableProps)?.domNode!; + + // defineProperty needed to assign values to readonly node properties + Object.defineProperty(scrollNode, 'clientHeight', {get: () => 10}); + Object.defineProperty(scrollNode, 'scrollHeight', {get: () => 10}); + Object.defineProperty(scrollNode, 'scrollTop', {get: () => 10}); + + scrollArea.act(() => { + scrollNode.dispatchEvent(new Event('scroll')); + }); + + expect(onScrolledToBottom).not.toHaveBeenCalled(); + }); }); diff --git a/polaris-react/tests/setup/tests.ts b/polaris-react/tests/setup/tests.ts index a18d7daf9e8..489a2a0b02c 100644 --- a/polaris-react/tests/setup/tests.ts +++ b/polaris-react/tests/setup/tests.ts @@ -6,3 +6,8 @@ import './matchers'; afterEach(() => { destroyAll(); }); + +globalThis.requestAnimationFrame = (cb) => { + cb(Date.now()); + return Math.random(); +}; From 5f7cdeeafc599e8c858b34d7759f640e5dcb702c Mon Sep 17 00:00:00 2001 From: iamnathanj Date: Fri, 4 Nov 2022 12:26:39 -0400 Subject: [PATCH 3/4] mock rAF locally in new tests --- .../Scrollable/tests/Scrollable.test.tsx | 15 +++++++++++++++ polaris-react/tests/setup/tests.ts | 5 ----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/polaris-react/src/components/Scrollable/tests/Scrollable.test.tsx b/polaris-react/src/components/Scrollable/tests/Scrollable.test.tsx index 480a0bba49d..c04bdc41de3 100644 --- a/polaris-react/src/components/Scrollable/tests/Scrollable.test.tsx +++ b/polaris-react/src/components/Scrollable/tests/Scrollable.test.tsx @@ -6,6 +6,21 @@ import {ScrollableContext} from '../context'; import type {ScrollableProps} from '../Scrollable'; describe('', () => { + let rafSpy: jest.SpyInstance; + + beforeAll(() => { + rafSpy = jest + .spyOn(window, 'requestAnimationFrame') + .mockImplementation((cb) => { + cb(Date.now()); + return Math.random(); + }); + }); + + afterAll(() => { + rafSpy.mockRestore(); + }); + it('mounts', () => { const scrollable = mountWithApp(); expect(scrollable).toBeDefined(); diff --git a/polaris-react/tests/setup/tests.ts b/polaris-react/tests/setup/tests.ts index 489a2a0b02c..a18d7daf9e8 100644 --- a/polaris-react/tests/setup/tests.ts +++ b/polaris-react/tests/setup/tests.ts @@ -6,8 +6,3 @@ import './matchers'; afterEach(() => { destroyAll(); }); - -globalThis.requestAnimationFrame = (cb) => { - cb(Date.now()); - return Math.random(); -}; From b8942ce5a07b8a3bf5077f7024b6bd0111bd5134 Mon Sep 17 00:00:00 2001 From: iamnathanj Date: Fri, 4 Nov 2022 14:40:28 -0400 Subject: [PATCH 4/4] add changeset --- .changeset/thick-clocks-applaud.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/thick-clocks-applaud.md diff --git a/.changeset/thick-clocks-applaud.md b/.changeset/thick-clocks-applaud.md new file mode 100644 index 00000000000..676b4fdbf64 --- /dev/null +++ b/.changeset/thick-clocks-applaud.md @@ -0,0 +1,5 @@ +--- +'@shopify/polaris': patch +--- + +Fixed Scrollable component to match existing onScrolledToBottom logic