Skip to content

Commit

Permalink
fix: infinite scroll issue (#2341)
Browse files Browse the repository at this point in the history
* fix: infinite scroll issue

* test: add tests for new InfiniteScroll

* test: mock the IntersectionObserver for all tests

* test: fix tests

* fix: rollback beforeSetupTest
  • Loading branch information
juanmahidalgo authored Dec 17, 2024
1 parent 50b0157 commit 151e6e9
Show file tree
Hide file tree
Showing 4 changed files with 254 additions and 63 deletions.
140 changes: 117 additions & 23 deletions webapp/src/components/InfiniteScroll/InfiniteScroll.spec.tsx
Original file line number Diff line number Diff line change
@@ -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()
})
})
})
67 changes: 27 additions & 40 deletions webapp/src/components/InfiniteScroll/InfiniteScroll.tsx
Original file line number Diff line number Diff line change
@@ -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>
)
}
55 changes: 55 additions & 0 deletions webapp/src/components/ListPage/ListPage.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
55 changes: 55 additions & 0 deletions webapp/src/components/ListsPage/ListsPage.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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} />
Expand Down

0 comments on commit 151e6e9

Please sign in to comment.