diff --git a/public/app/features/logs/components/InfiniteScroll.test.tsx b/public/app/features/logs/components/InfiniteScroll.test.tsx index db5b9f00770..a4078ea5ca3 100644 --- a/public/app/features/logs/components/InfiniteScroll.test.tsx +++ b/public/app/features/logs/components/InfiniteScroll.test.tsx @@ -3,6 +3,7 @@ import React, { useEffect, useRef, useState } from 'react'; import { LogRowModel, dateTimeForTimeZone } from '@grafana/data'; import { convertRawToRange } from '@grafana/data/src/datetime/rangeutil'; +import { config } from '@grafana/runtime'; import { LogsSortOrder } from '@grafana/schema'; import { InfiniteScroll, Props, SCROLLING_THRESHOLD } from './InfiniteScroll'; @@ -50,6 +51,45 @@ function ScrollWithWrapper({ children, ...props }: Props) { ); } +function setup(loadMoreMock: () => void, startPosition: number, rows: LogRowModel[], order: LogsSortOrder) { + const { element, events } = getMockElement(startPosition); + + function scrollTo(position: number) { + element.scrollTop = position; + + act(() => { + events['scroll'](new Event('scroll')); + }); + } + function wheel(deltaY: number) { + act(() => { + const event = new WheelEvent('wheel', { deltaY }); + events['wheel'](event); + }); + } + + render( + +
+ + ); + + return { element, events, scrollTo, wheel }; +} + +beforeAll(() => { + config.featureToggles.logsInfiniteScrolling = true; +}); +afterAll(() => { + config.featureToggles.logsInfiniteScrolling = false; +}); + describe('InfiniteScroll', () => { test('Wraps components without adding DOM elements', async () => { const { container } = render( @@ -81,39 +121,29 @@ describe('InfiniteScroll', () => { rows = createLogRows(absoluteRange.from + 2 * SCROLLING_THRESHOLD, absoluteRange.to - 2 * SCROLLING_THRESHOLD); }); - function setup(loadMoreMock: () => void, startPosition: number) { - const { element, events } = getMockElement(startPosition); - render( - -
- - ); - return { element, events }; - } - test.each([ ['top', 10, 0], - ['bottom', 90, 100], - ])('Requests more logs when scrolling %s', async (_: string, startPosition: number, endPosition: number) => { - const loadMoreMock = jest.fn(); - const { element, events } = setup(loadMoreMock, startPosition); + ['bottom', 50, 60], + ])( + 'Requests more logs when scrolling %s', + async (direction: string, startPosition: number, endPosition: number) => { + const loadMoreMock = jest.fn(); + const { scrollTo, element } = setup(loadMoreMock, startPosition, rows, order); - expect(await screen.findByTestId('contents')).toBeInTheDocument(); - element.scrollTop = endPosition; + expect(await screen.findByTestId('contents')).toBeInTheDocument(); - act(() => { - events['scroll'](new Event('scroll')); - }); + scrollTo(endPosition); - expect(loadMoreMock).toHaveBeenCalled(); - expect(await screen.findByTestId('Spinner')).toBeInTheDocument(); - }); + expect(loadMoreMock).toHaveBeenCalled(); + expect(await screen.findByTestId('Spinner')).toBeInTheDocument(); + if (direction === 'bottom') { + // Bottom loader visibility trick + expect(element.scrollTo).toHaveBeenCalled(); + } else { + expect(element.scrollTo).not.toHaveBeenCalled(); + } + } + ); test.each([ ['up', -5, 0], @@ -122,14 +152,11 @@ describe('InfiniteScroll', () => { 'Requests more logs when moving the mousewheel %s', async (_: string, deltaY: number, startPosition: number) => { const loadMoreMock = jest.fn(); - const { events } = setup(loadMoreMock, startPosition); + const { wheel } = setup(loadMoreMock, startPosition, rows, order); expect(await screen.findByTestId('contents')).toBeInTheDocument(); - act(() => { - const event = new WheelEvent('wheel', { deltaY }); - events['wheel'](event); - }); + wheel(deltaY); expect(loadMoreMock).toHaveBeenCalled(); expect(await screen.findByTestId('Spinner')).toBeInTheDocument(); @@ -138,33 +165,28 @@ describe('InfiniteScroll', () => { test('Does not request more logs when there is no scroll', async () => { const loadMoreMock = jest.fn(); - const { element, events } = setup(loadMoreMock, 0); + const { scrollTo, element } = setup(loadMoreMock, 0, rows, order); expect(await screen.findByTestId('contents')).toBeInTheDocument(); element.clientHeight = 40; element.scrollHeight = element.clientHeight; - act(() => { - events['scroll'](new Event('scroll')); - }); + scrollTo(40); expect(loadMoreMock).not.toHaveBeenCalled(); expect(screen.queryByTestId('Spinner')).not.toBeInTheDocument(); }); test('Requests newer logs from the most recent timestamp', async () => { - const startPosition = order === LogsSortOrder.Descending ? 10 : 90; // Scroll top - const endPosition = order === LogsSortOrder.Descending ? 0 : 100; // Scroll bottom + const startPosition = order === LogsSortOrder.Descending ? 10 : 50; // Scroll top + const endPosition = order === LogsSortOrder.Descending ? 0 : 60; // Scroll bottom const loadMoreMock = jest.fn(); - const { element, events } = setup(loadMoreMock, startPosition); + const { scrollTo } = setup(loadMoreMock, startPosition, rows, order); expect(await screen.findByTestId('contents')).toBeInTheDocument(); - element.scrollTop = endPosition; - act(() => { - events['scroll'](new Event('scroll')); - }); + scrollTo(endPosition); expect(loadMoreMock).toHaveBeenCalledWith({ from: rows[rows.length - 1].timeEpochMs, @@ -173,18 +195,15 @@ describe('InfiniteScroll', () => { }); test('Requests older logs from the oldest timestamp', async () => { - const startPosition = order === LogsSortOrder.Ascending ? 10 : 90; // Scroll top - const endPosition = order === LogsSortOrder.Ascending ? 0 : 100; // Scroll bottom + const startPosition = order === LogsSortOrder.Ascending ? 10 : 50; // Scroll top + const endPosition = order === LogsSortOrder.Ascending ? 0 : 60; // Scroll bottom const loadMoreMock = jest.fn(); - const { element, events } = setup(loadMoreMock, startPosition); + const { scrollTo } = setup(loadMoreMock, startPosition, rows, order); expect(await screen.findByTestId('contents')).toBeInTheDocument(); - element.scrollTop = endPosition; - act(() => { - events['scroll'](new Event('scroll')); - }); + scrollTo(endPosition); expect(loadMoreMock).toHaveBeenCalledWith({ from: absoluteRange.from, @@ -193,39 +212,20 @@ describe('InfiniteScroll', () => { }); describe('With absolute range matching visible range', () => { - function setup(loadMoreMock: () => void, startPosition: number, rows: LogRowModel[]) { - const { element, events } = getMockElement(startPosition); - render( - -
- - ); - return { element, events }; - } - test.each([ ['top', 10, 0], - ['bottom', 90, 100], + ['bottom', 50, 60], ])( 'It does not request more when scrolling %s', async (_: string, startPosition: number, endPosition: number) => { // Visible range matches the current range const rows = createLogRows(absoluteRange.from, absoluteRange.to); const loadMoreMock = jest.fn(); - const { element, events } = setup(loadMoreMock, startPosition, rows); + const { scrollTo } = setup(loadMoreMock, startPosition, rows, order); expect(await screen.findByTestId('contents')).toBeInTheDocument(); - element.scrollTop = endPosition; - act(() => { - events['scroll'](new Event('scroll')); - }); + scrollTo(endPosition); expect(loadMoreMock).not.toHaveBeenCalled(); expect(screen.queryByTestId('Spinner')).not.toBeInTheDocument(); @@ -235,39 +235,20 @@ describe('InfiniteScroll', () => { }); describe('With relative range matching visible range', () => { - function setup(loadMoreMock: () => void, startPosition: number, rows: LogRowModel[]) { - const { element, events } = getMockElement(startPosition); - render( - -
- - ); - return { element, events }; - } - test.each([ ['top', 10, 0], - ['bottom', 90, 100], + ['bottom', 50, 60], ])( 'It does not request more when scrolling %s', async (_: string, startPosition: number, endPosition: number) => { // Visible range matches the current range const rows = createLogRows(absoluteRange.from, absoluteRange.to); const loadMoreMock = jest.fn(); - const { element, events } = setup(loadMoreMock, startPosition, rows); + const { scrollTo } = setup(loadMoreMock, startPosition, rows, order); expect(await screen.findByTestId('contents')).toBeInTheDocument(); - element.scrollTop = endPosition; - act(() => { - events['scroll'](new Event('scroll')); - }); + scrollTo(endPosition); expect(loadMoreMock).not.toHaveBeenCalled(); expect(screen.queryByTestId('Spinner')).not.toBeInTheDocument(); @@ -301,6 +282,7 @@ function getMockElement(scrollTop: number) { scrollHeight: 100, clientHeight: 40, scrollTop, + scrollTo: jest.fn(), }; return { element, events }; diff --git a/public/app/features/logs/components/InfiniteScroll.tsx b/public/app/features/logs/components/InfiniteScroll.tsx index 2a048c0f4ad..9cf97d4d3fd 100644 --- a/public/app/features/logs/components/InfiniteScroll.tsx +++ b/public/app/features/logs/components/InfiniteScroll.tsx @@ -3,7 +3,7 @@ import React, { ReactNode, useEffect, useRef, useState } from 'react'; import { AbsoluteTimeRange, LogRowModel, TimeRange } from '@grafana/data'; import { convertRawToRange, isRelativeTime, isRelativeTimeRange } from '@grafana/data/src/datetime/rangeutil'; -import { reportInteraction } from '@grafana/runtime'; +import { config, reportInteraction } from '@grafana/runtime'; import { LogsSortOrder, TimeZone } from '@grafana/schema'; import { LoadingIndicator } from './LoadingIndicator'; @@ -33,13 +33,16 @@ export const InfiniteScroll = ({ const [lowerOutOfRange, setLowerOutOfRange] = useState(false); const [upperLoading, setUpperLoading] = useState(false); const [lowerLoading, setLowerLoading] = useState(false); + const rowsRef = useRef(rows); const lastScroll = useRef(scrollElement?.scrollTop || 0); + // Reset messages when range/order/rows change useEffect(() => { setUpperOutOfRange(false); setLowerOutOfRange(false); }, [range, rows, sortOrder]); + // Reset loading messages when loading stops useEffect(() => { if (!loading) { setUpperLoading(false); @@ -47,13 +50,32 @@ export const InfiniteScroll = ({ } }, [loading]); + // Ensure bottom loader visibility + useEffect(() => { + if (lowerLoading && scrollElement) { + scrollElement.scrollTo(0, scrollElement.scrollHeight - scrollElement.clientHeight); + } + }, [lowerLoading, scrollElement]); + + // Request came back with no new past rows + useEffect(() => { + if (rows !== rowsRef.current && rows.length === rowsRef.current.length && (upperLoading || lowerLoading)) { + if (sortOrder === LogsSortOrder.Descending && lowerLoading) { + setLowerOutOfRange(true); + } else if (sortOrder === LogsSortOrder.Ascending && upperLoading) { + setUpperOutOfRange(true); + } + } + rowsRef.current = rows; + }, [lowerLoading, rows, sortOrder, upperLoading]); + useEffect(() => { if (!scrollElement || !loadMoreLogs) { return; } function handleScroll(event: Event | WheelEvent) { - if (!scrollElement || !loadMoreLogs || !rows.length || loading) { + if (!scrollElement || !loadMoreLogs || !rows.length || loading || !config.featureToggles.logsInfiniteScrolling) { return; } event.stopImmediatePropagation(); @@ -69,15 +91,12 @@ export const InfiniteScroll = ({ } function scrollTop() { - if (!canScrollTop(getVisibleRange(rows), range, timeZone, sortOrder)) { + const newRange = canScrollTop(getVisibleRange(rows), range, timeZone, sortOrder); + if (!newRange) { setUpperOutOfRange(true); return; } setUpperOutOfRange(false); - const newRange = - sortOrder === LogsSortOrder.Descending - ? getNextRange(getVisibleRange(rows), range, timeZone) - : getPrevRange(getVisibleRange(rows), range); loadMoreLogs?.(newRange); setUpperLoading(true); reportInteraction('grafana_logs_infinite_scrolling', { @@ -87,15 +106,12 @@ export const InfiniteScroll = ({ } function scrollBottom() { - if (!canScrollBottom(getVisibleRange(rows), range, timeZone, sortOrder)) { + const newRange = canScrollBottom(getVisibleRange(rows), range, timeZone, sortOrder); + if (!newRange) { setLowerOutOfRange(true); return; } setLowerOutOfRange(false); - const newRange = - sortOrder === LogsSortOrder.Descending - ? getPrevRange(getVisibleRange(rows), range) - : getNextRange(getVisibleRange(rows), range, timeZone); loadMoreLogs?.(newRange); setLowerLoading(true); reportInteraction('grafana_logs_infinite_scrolling', { @@ -160,9 +176,8 @@ function shouldLoadMore(event: Event | WheelEvent, element: HTMLDivElement, last scrollDirection === ScrollDirection.Top ? element.scrollTop : element.scrollHeight - element.scrollTop - element.clientHeight; - const coef = 1; - return diff <= coef ? scrollDirection : ScrollDirection.NoScroll; + return diff <= 1 ? scrollDirection : ScrollDirection.NoScroll; } function getVisibleRange(rows: LogRowModel[]) { @@ -195,13 +210,16 @@ function canScrollTop( currentRange: TimeRange, timeZone: TimeZone, sortOrder: LogsSortOrder -) { +): AbsoluteTimeRange | undefined { if (sortOrder === LogsSortOrder.Descending) { // When requesting new logs, update the current range if using relative time ranges. currentRange = updateCurrentRange(currentRange, timeZone); - return currentRange.to.valueOf() - visibleRange.to > SCROLLING_THRESHOLD; + const canScroll = currentRange.to.valueOf() - visibleRange.to > SCROLLING_THRESHOLD; + return canScroll ? getNextRange(visibleRange, currentRange, timeZone) : undefined; } - return Math.abs(currentRange.from.valueOf() - visibleRange.from) > SCROLLING_THRESHOLD; + + const canScroll = Math.abs(currentRange.from.valueOf() - visibleRange.from) > SCROLLING_THRESHOLD; + return canScroll ? getPrevRange(visibleRange, currentRange) : undefined; } function canScrollBottom( @@ -209,13 +227,15 @@ function canScrollBottom( currentRange: TimeRange, timeZone: TimeZone, sortOrder: LogsSortOrder -) { +): AbsoluteTimeRange | undefined { if (sortOrder === LogsSortOrder.Descending) { - return Math.abs(currentRange.from.valueOf() - visibleRange.from) > SCROLLING_THRESHOLD; + const canScroll = Math.abs(currentRange.from.valueOf() - visibleRange.from) > SCROLLING_THRESHOLD; + return canScroll ? getPrevRange(visibleRange, currentRange) : undefined; } // When requesting new logs, update the current range if using relative time ranges. currentRange = updateCurrentRange(currentRange, timeZone); - return currentRange.to.valueOf() - visibleRange.to > SCROLLING_THRESHOLD; + const canScroll = currentRange.to.valueOf() - visibleRange.to > SCROLLING_THRESHOLD; + return canScroll ? getNextRange(visibleRange, currentRange, timeZone) : undefined; } // Given a TimeRange, returns a new instance if using relative time, or else the same.