From 151e6e94d0894ae3d0ec14eb4e75c4a7d29bacb4 Mon Sep 17 00:00:00 2001
From: Juanma Hidalgo <juanma06@gmail.com>
Date: Tue, 17 Dec 2024 14:30:20 +0100
Subject: [PATCH] fix: infinite scroll issue (#2341)

* fix: infinite scroll issue

* test: add tests for new InfiniteScroll

* test: mock the IntersectionObserver for all tests

* test: fix tests

* fix: rollback beforeSetupTest
---
 .../InfiniteScroll/InfiniteScroll.spec.tsx    | 140 +++++++++++++++---
 .../InfiniteScroll/InfiniteScroll.tsx         |  67 ++++-----
 .../src/components/ListPage/ListPage.spec.tsx |  55 +++++++
 .../components/ListsPage/ListsPage.spec.tsx   |  55 +++++++
 4 files changed, 254 insertions(+), 63 deletions(-)

diff --git a/webapp/src/components/InfiniteScroll/InfiniteScroll.spec.tsx b/webapp/src/components/InfiniteScroll/InfiniteScroll.spec.tsx
index d9d6dcdf75..65898e328d 100644
--- a/webapp/src/components/InfiniteScroll/InfiniteScroll.spec.tsx
+++ b/webapp/src/components/InfiniteScroll/InfiniteScroll.spec.tsx
@@ -1,44 +1,138 @@
 import { render } from '@testing-library/react'
-import userEvent from '@testing-library/user-event'
-import { t } from 'decentraland-dapps/dist/modules/translation/utils'
 import { InfiniteScroll } from './InfiniteScroll'
 import { Props } from './InfiniteScroll.types'
 
+let mockObservers: MockIntersectionObserver[] = []
+
+class MockIntersectionObserver implements IntersectionObserver {
+  root: Element | Document | null = null
+  rootMargin: string = ''
+  thresholds: ReadonlyArray<number> = []
+  callback: IntersectionObserverCallback
+  elements: Element[] = []
+
+  constructor(callback: IntersectionObserverCallback) {
+    this.callback = callback
+    mockObservers.push(this)
+  }
+
+  observe(element: Element) {
+    this.elements.push(element)
+  }
+
+  unobserve(element: Element) {
+    this.elements = this.elements.filter(el => el !== element)
+  }
+
+  disconnect() {
+    this.elements = []
+  }
+
+  takeRecords(): IntersectionObserverEntry[] {
+    return []
+  }
+
+  // Helper method to simulate intersection
+  triggerIntersect(isIntersecting = true) {
+    const entries = this.elements.map(element => ({
+      isIntersecting,
+      target: element,
+      intersectionRatio: isIntersecting ? 1 : 0,
+      intersectionRect: {} as DOMRectReadOnly,
+      boundingClientRect: {} as DOMRectReadOnly,
+      rootBounds: null,
+      time: 0
+    })) as IntersectionObserverEntry[]
+
+    this.callback(entries, this)
+  }
+}
+
+beforeAll(() => {
+  // Properly cast IntersectionObserver to avoid TS warnings
+  window.IntersectionObserver = MockIntersectionObserver as unknown as typeof IntersectionObserver
+})
+
+beforeEach(() => {
+  mockObservers = []
+})
+
 function renderInfiniteScroll(props: Partial<Props>) {
   return render(
-    <InfiniteScroll maxScrollPages={0} hasMorePages onLoadMore={jest.fn()} page={0} {...props}>
+    <InfiniteScroll hasMorePages={true} isLoading={false} onLoadMore={jest.fn()} page={0} {...props}>
       <span>My container</span>
     </InfiniteScroll>
   )
 }
 
-describe('when maxScrollPages is 0', () => {
-  describe('and hasMorePages is true', () => {
-    it('should show load button from the start', () => {
-      const screen = renderInfiniteScroll({ maxScrollPages: 0 })
-      expect(screen.getByRole('button', { name: t('global.load_more') })).toBeInTheDocument()
+describe('InfiniteScroll', () => {
+  describe('when hasMorePages is true', () => {
+    it('should render the sentinel element', () => {
+      const { container } = renderInfiniteScroll({ hasMorePages: true })
+      expect(container.querySelector('div[role="feed"] div')).toBeInTheDocument()
     })
 
-    describe('and load more button is clicked', () => {
-      it('should call onLoadMore function', async () => {
-        const loadMoreMock = jest.fn()
-        const screen = renderInfiniteScroll({
-          maxScrollPages: 0,
-          onLoadMore: loadMoreMock
-        })
-        await userEvent.click(screen.getByRole('button', { name: t('global.load_more') }))
-        expect(loadMoreMock).toHaveBeenCalledWith(1)
+    it('should call onLoadMore when the sentinel intersects', () => {
+      const onLoadMoreMock = jest.fn()
+      const { container } = renderInfiniteScroll({
+        hasMorePages: true,
+        onLoadMore: onLoadMoreMock,
+        page: 0
       })
+
+      const sentinel = container.querySelector('div[role="feed"] div')
+      expect(sentinel).toBeInTheDocument()
+
+      // Trigger intersection on the created observer
+      expect(mockObservers.length).toBe(1)
+      mockObservers[0].triggerIntersect(true)
+
+      expect(onLoadMoreMock).toHaveBeenCalledWith(1) // page + 1
     })
   })
 
-  describe('and hasMorePages is false', () => {
-    it('should not show load more button', () => {
-      const screen = renderInfiniteScroll({
-        maxScrollPages: 0,
-        hasMorePages: false
+  describe('when hasMorePages is false', () => {
+    it('should not render the sentinel element', () => {
+      const { container } = renderInfiniteScroll({ hasMorePages: false })
+      const sentinel = container.querySelector('div[role="feed"] div')
+      expect(sentinel).toBeNull()
+    })
+
+    it('should not call onLoadMore', () => {
+      const onLoadMoreMock = jest.fn()
+      renderInfiniteScroll({
+        hasMorePages: false,
+        onLoadMore: onLoadMoreMock,
+        page: 0
       })
-      expect(screen.queryByRole('button', { name: t('global.load_more') })).not.toBeInTheDocument()
+
+      // No sentinel, no intersection. Just ensure onLoadMore not called.
+      expect(onLoadMoreMock).not.toHaveBeenCalled()
+      expect(mockObservers.length).toBe(1)
+      // The observer may be created but no elements observed, so no intersection occurs.
+    })
+  })
+
+  describe('when isLoading is true', () => {
+    it('should not call onLoadMore even if intersection occurs', () => {
+      const onLoadMoreMock = jest.fn()
+      const { container } = renderInfiniteScroll({
+        hasMorePages: true,
+        isLoading: true,
+        onLoadMore: onLoadMoreMock,
+        page: 0
+      })
+
+      // Sentinel is rendered since hasMorePages=true
+      const sentinel = container.querySelector('div[role="feed"] div')
+      expect(sentinel).toBeInTheDocument()
+
+      // Because isLoading is true at render time, no observer should have been created
+      expect(mockObservers.length).toBe(0)
+
+      // Since no observer is created, we cannot trigger intersection.
+      // Just ensure onLoadMore was not called.
+      expect(onLoadMoreMock).not.toHaveBeenCalled()
     })
   })
 })
diff --git a/webapp/src/components/InfiniteScroll/InfiniteScroll.tsx b/webapp/src/components/InfiniteScroll/InfiniteScroll.tsx
index 19a083c472..c94d9d78f6 100644
--- a/webapp/src/components/InfiniteScroll/InfiniteScroll.tsx
+++ b/webapp/src/components/InfiniteScroll/InfiniteScroll.tsx
@@ -1,54 +1,41 @@
-import { useCallback, useEffect, useState } from 'react'
-import { t } from 'decentraland-dapps/dist/modules/translation/utils'
-import { Button } from 'decentraland-ui'
+import { useEffect, useRef } from 'react'
 import { Props } from './InfiniteScroll.types'
 
-export function InfiniteScroll({ page, hasMorePages, isLoading, children, maxScrollPages, onLoadMore }: Props) {
-  const [scrollPage, setScrollPage] = useState(0)
-  const [showLoadMoreButton, setShowLoadMoreButton] = useState(maxScrollPages === 0)
-
-  const onScroll = useCallback(() => {
-    const scrollTop = document.documentElement.scrollTop
-    const scrollHeight = document.documentElement.scrollHeight
-    const clientHeight = document.documentElement.clientHeight
-    if (!isLoading && scrollTop + clientHeight >= scrollHeight && hasMorePages && (!maxScrollPages || scrollPage < maxScrollPages)) {
-      setScrollPage(scrollPage + 1)
-      onLoadMore(page + 1)
-    }
-  }, [page, hasMorePages, isLoading, scrollPage, maxScrollPages, onLoadMore])
+export function InfiniteScroll({ page, hasMorePages, isLoading, children, onLoadMore }: Props) {
+  const bottomRef = useRef(null)
 
   useEffect(() => {
-    if (!isLoading && maxScrollPages !== undefined && scrollPage === maxScrollPages) {
-      setShowLoadMoreButton(true)
-    } else {
-      setShowLoadMoreButton(false)
-    }
-  }, [isLoading, scrollPage, maxScrollPages, page, setShowLoadMoreButton])
+    if (isLoading) return // don't observe while loading more
 
-  useEffect(() => {
-    window.addEventListener('scroll', onScroll)
-    return () => window.removeEventListener('scroll', onScroll)
-  }, [onScroll])
+    const observer = new IntersectionObserver(
+      entries => {
+        const [entry] = entries
+        if (entry.isIntersecting && hasMorePages) {
+          onLoadMore(page + 1)
+        }
+      },
+      {
+        root: null, // the viewport
+        rootMargin: '0px',
+        threshold: 0.1 // Trigger when the sentinel is in view
+      }
+    )
 
-  const handleLoadMore = useCallback(() => {
-    onLoadMore(page + 1)
-    setScrollPage(0)
-  }, [onLoadMore, page])
+    if (bottomRef.current) {
+      observer.observe(bottomRef.current)
+    }
 
-  if (!hasMorePages) {
-    return children
-  }
+    return () => {
+      if (bottomRef.current) {
+        observer.unobserve(bottomRef.current)
+      }
+    }
+  }, [page, hasMorePages, isLoading, onLoadMore])
 
   return (
     <div role="feed">
       {children}
-      {showLoadMoreButton && (
-        <div className="load-more">
-          <Button loading={isLoading} inverted primary onClick={handleLoadMore}>
-            {t('global.load_more')}
-          </Button>
-        </div>
-      )}
+      {hasMorePages && <div ref={bottomRef} style={{ height: '1px' }} />}
     </div>
   )
 }
diff --git a/webapp/src/components/ListPage/ListPage.spec.tsx b/webapp/src/components/ListPage/ListPage.spec.tsx
index a96bac90d8..0e7a827c55 100644
--- a/webapp/src/components/ListPage/ListPage.spec.tsx
+++ b/webapp/src/components/ListPage/ListPage.spec.tsx
@@ -23,6 +23,61 @@ import {
 import ListPage from './ListPage'
 import { Props } from './ListPage.types'
 
+let mockObservers: MockIntersectionObserver[] = []
+
+class MockIntersectionObserver implements IntersectionObserver {
+  root: Element | Document | null = null
+  rootMargin: string = ''
+  thresholds: ReadonlyArray<number> = []
+  callback: IntersectionObserverCallback
+  elements: Element[] = []
+
+  constructor(callback: IntersectionObserverCallback) {
+    this.callback = callback
+    mockObservers.push(this)
+  }
+
+  observe(element: Element) {
+    this.elements.push(element)
+  }
+
+  unobserve(element: Element) {
+    this.elements = this.elements.filter(el => el !== element)
+  }
+
+  disconnect() {
+    this.elements = []
+  }
+
+  takeRecords(): IntersectionObserverEntry[] {
+    return []
+  }
+
+  // Helper method to simulate intersection
+  triggerIntersect(isIntersecting = true) {
+    const entries = this.elements.map(element => ({
+      isIntersecting,
+      target: element,
+      intersectionRatio: isIntersecting ? 1 : 0,
+      intersectionRect: {} as DOMRectReadOnly,
+      boundingClientRect: {} as DOMRectReadOnly,
+      rootBounds: null,
+      time: 0
+    })) as IntersectionObserverEntry[]
+
+    this.callback(entries, this)
+  }
+}
+
+beforeAll(() => {
+  // Properly cast IntersectionObserver to avoid TS warnings
+  window.IntersectionObserver = MockIntersectionObserver as unknown as typeof IntersectionObserver
+})
+
+beforeEach(() => {
+  mockObservers = []
+})
+
 // This is to avoid errors with the canvas
 jest.mock('../LinkedProfile', () => {
   return {
diff --git a/webapp/src/components/ListsPage/ListsPage.spec.tsx b/webapp/src/components/ListsPage/ListsPage.spec.tsx
index 69d9f7f91b..e51b80adf1 100644
--- a/webapp/src/components/ListsPage/ListsPage.spec.tsx
+++ b/webapp/src/components/ListsPage/ListsPage.spec.tsx
@@ -6,6 +6,61 @@ import { LOADER_TEST_ID, ERROR_TEST_ID, CREATE_LIST_TEST_ID } from './constants'
 import ListsPage from './ListsPage'
 import { Props } from './ListsPage.types'
 
+let mockObservers: MockIntersectionObserver[] = []
+
+class MockIntersectionObserver implements IntersectionObserver {
+  root: Element | Document | null = null
+  rootMargin: string = ''
+  thresholds: ReadonlyArray<number> = []
+  callback: IntersectionObserverCallback
+  elements: Element[] = []
+
+  constructor(callback: IntersectionObserverCallback) {
+    this.callback = callback
+    mockObservers.push(this)
+  }
+
+  observe(element: Element) {
+    this.elements.push(element)
+  }
+
+  unobserve(element: Element) {
+    this.elements = this.elements.filter(el => el !== element)
+  }
+
+  disconnect() {
+    this.elements = []
+  }
+
+  takeRecords(): IntersectionObserverEntry[] {
+    return []
+  }
+
+  // Helper method to simulate intersection
+  triggerIntersect(isIntersecting = true) {
+    const entries = this.elements.map(element => ({
+      isIntersecting,
+      target: element,
+      intersectionRatio: isIntersecting ? 1 : 0,
+      intersectionRect: {} as DOMRectReadOnly,
+      boundingClientRect: {} as DOMRectReadOnly,
+      rootBounds: null,
+      time: 0
+    })) as IntersectionObserverEntry[]
+
+    this.callback(entries, this)
+  }
+}
+
+beforeAll(() => {
+  // Properly cast IntersectionObserver to avoid TS warnings
+  window.IntersectionObserver = MockIntersectionObserver as unknown as typeof IntersectionObserver
+})
+
+beforeEach(() => {
+  mockObservers = []
+})
+
 function renderListsPage(props: Partial<Props> = {}) {
   return renderWithProviders(
     <ListsPage isLoading={false} lists={[]} count={0} error={null} onFetchLists={jest.fn()} onCreateList={jest.fn()} {...props} />