From aace94438e211f7502bf9fedb27169a9ca236b65 Mon Sep 17 00:00:00 2001 From: Matias Chomicki Date: Tue, 19 Nov 2024 17:12:41 +0000 Subject: [PATCH] Logs: Allow scroll to reach the bottom of the log list before loading more (#96668) * Infinite scroll: use timestamps to improve scrolling experience * Add unit tests --- .../logs/components/InfiniteScroll.test.tsx | 50 ++++++++++++++- .../logs/components/InfiniteScroll.tsx | 63 +++++++++++++++++-- 2 files changed, 105 insertions(+), 8 deletions(-) diff --git a/public/app/features/logs/components/InfiniteScroll.test.tsx b/public/app/features/logs/components/InfiniteScroll.test.tsx index e5e7854dd72..666932be9d9 100644 --- a/public/app/features/logs/components/InfiniteScroll.test.tsx +++ b/public/app/features/logs/components/InfiniteScroll.test.tsx @@ -74,11 +74,14 @@ function setup( wheel(-1); } } - function wheel(deltaY: number) { + function wheel(deltaY: number, timeStamp?: number) { element.scrollTop += deltaY; act(() => { const event = new WheelEvent('wheel', { deltaY }); + if (timeStamp) { + jest.spyOn(event, 'timeStamp', 'get').mockReturnValue(timeStamp); + } events['wheel'](event); }); } @@ -164,7 +167,7 @@ describe('InfiniteScroll', () => { test.each([ ['up', -5, 0], - ['down', 5, 100], + ['down', 5, 60], ])( 'Requests more logs when moving the mousewheel %s', async (_: string, deltaY: number, startPosition: number) => { @@ -273,6 +276,49 @@ describe('InfiniteScroll', () => { } ); }); + + describe('Chain of events', () => { + test('Ingnores chains of events', async () => { + const loadMoreMock = jest.fn(); + const { wheel } = setup(loadMoreMock, 57, rows, order); + + expect(await screen.findByTestId('contents')).toBeInTheDocument(); + + const timeStamps = [1, 2, 3, 4]; + timeStamps.forEach((timeStamp) => { + wheel(1, timeStamp); + }); + + expect(loadMoreMock).not.toHaveBeenCalled(); + }); + + test('Detects when chain of events ends', async () => { + const loadMoreMock = jest.fn(); + const { wheel } = setup(loadMoreMock, 57, rows, order); + + expect(await screen.findByTestId('contents')).toBeInTheDocument(); + + const timeStamps = [1, 2, 3, 600, 1]; + timeStamps.forEach((timeStamp) => { + wheel(1, timeStamp); + }); + + expect(loadMoreMock).toHaveBeenCalledTimes(1); + }); + + test('Detects when the user wants to scroll', async () => { + const loadMoreMock = jest.fn(); + const { wheel } = setup(loadMoreMock, 57, rows, order); + + expect(await screen.findByTestId('contents')).toBeInTheDocument(); + + for (let i = 0; i <= 25; i++) { + wheel(1, 399 * i + 399); + } + + expect(loadMoreMock).toHaveBeenCalledTimes(1); + }); + }); } ); diff --git a/public/app/features/logs/components/InfiniteScroll.tsx b/public/app/features/logs/components/InfiniteScroll.tsx index 66ff34e4452..d8cbeb918f3 100644 --- a/public/app/features/logs/components/InfiniteScroll.tsx +++ b/public/app/features/logs/components/InfiniteScroll.tsx @@ -1,5 +1,5 @@ import { css } from '@emotion/css'; -import { ReactNode, useCallback, useEffect, useRef, useState } from 'react'; +import { ReactNode, MutableRefObject, useCallback, useEffect, useRef, useState } from 'react'; import { AbsoluteTimeRange, CoreApp, LogRowModel, TimeRange } from '@grafana/data'; import { convertRawToRange, isRelativeTime, isRelativeTimeRange } from '@grafana/data/src/datetime/rangeutil'; @@ -41,6 +41,8 @@ export const InfiniteScroll = ({ const [lowerLoading, setLowerLoading] = useState(false); const rowsRef = useRef(rows); const lastScroll = useRef(scrollElement?.scrollTop || 0); + const lastEvent = useRef(null); + const countRef = useRef(0); // Reset messages when range/order/rows change useEffect(() => { @@ -84,12 +86,14 @@ export const InfiniteScroll = ({ if (!scrollElement || !loadMoreLogs || !rows.length || loading || !config.featureToggles.logsInfiniteScrolling) { return; } - event.stopImmediatePropagation(); - const scrollDirection = shouldLoadMore(event, scrollElement, lastScroll.current); + const scrollDirection = shouldLoadMore(event, lastEvent.current, countRef, scrollElement, lastScroll.current); + lastEvent.current = event; lastScroll.current = scrollElement.scrollTop; if (scrollDirection === ScrollDirection.NoScroll) { return; - } else if (scrollDirection === ScrollDirection.Top && topScrollEnabled) { + } + event.stopImmediatePropagation(); + if (scrollDirection === ScrollDirection.Top && topScrollEnabled) { scrollTop(); } else if (scrollDirection === ScrollDirection.Bottom) { scrollBottom(); @@ -215,7 +219,13 @@ enum ScrollDirection { Bottom = 1, NoScroll = 0, } -function shouldLoadMore(event: Event | WheelEvent, element: HTMLDivElement, lastScroll: number): ScrollDirection { +function shouldLoadMore( + event: Event | WheelEvent, + lastEvent: Event | WheelEvent | null, + countRef: MutableRefObject, + element: HTMLDivElement, + lastScroll: number +): ScrollDirection { // Disable behavior if there is no scroll if (element.scrollHeight <= element.clientHeight) { return ScrollDirection.NoScroll; @@ -224,13 +234,54 @@ function shouldLoadMore(event: Event | WheelEvent, element: HTMLDivElement, last if (delta === 0) { return ScrollDirection.NoScroll; } + const scrollDirection = delta < 0 ? ScrollDirection.Top : ScrollDirection.Bottom; const diff = scrollDirection === ScrollDirection.Top ? element.scrollTop : element.scrollHeight - element.scrollTop - element.clientHeight; - return diff <= 1 ? scrollDirection : ScrollDirection.NoScroll; + if (diff > 1) { + return ScrollDirection.NoScroll; + } + + if (lastEvent && shouldIgnoreChainOfEvents(event, lastEvent, countRef)) { + return ScrollDirection.NoScroll; + } + + return scrollDirection; +} + +function shouldIgnoreChainOfEvents( + event: Event | WheelEvent, + lastEvent: Event | WheelEvent, + countRef: MutableRefObject +) { + const deltaTime = event.timeStamp - lastEvent.timeStamp; + // Not a chain of events + if (deltaTime > 500) { + countRef.current = 0; + return false; + } + countRef.current++; + // Likely trackpad + if (deltaTime < 100) { + // User likely to want more results + if (countRef.current >= 180) { + countRef.current = 0; + return false; + } + return true; + } + // Likely mouse wheel + if (deltaTime < 400) { + // User likely to want more results + if (countRef.current >= 25) { + countRef.current = 0; + return false; + } + } + return true; } function getVisibleRange(rows: LogRowModel[]) {