From 92cc10f98312a2c5ff15a6e86c981b1f3eeb93d0 Mon Sep 17 00:00:00 2001 From: Matias Chomicki Date: Thu, 20 Mar 2025 21:14:02 +0000 Subject: [PATCH] Logs: Prevent automatic scrolling on refresh after changing scroll position (#102463) --- .../logs/components/InfiniteScroll.test.tsx | 29 +++++++++++------ .../logs/components/InfiniteScroll.tsx | 3 +- public/app/plugins/panel/logs/LogsPanel.tsx | 31 +++++++++++++++++-- 3 files changed, 50 insertions(+), 13 deletions(-) diff --git a/public/app/features/logs/components/InfiniteScroll.test.tsx b/public/app/features/logs/components/InfiniteScroll.test.tsx index c6b92afaef5..9b9792c2849 100644 --- a/public/app/features/logs/components/InfiniteScroll.test.tsx +++ b/public/app/features/logs/components/InfiniteScroll.test.tsx @@ -62,11 +62,15 @@ function setup( ) { const { element, events } = getMockElement(startPosition); - function scrollTo(position: number) { + function scrollTo(position: number, timeStamp?: number) { element.scrollTop = position; act(() => { - events['scroll'](new Event('scroll')); + const event = new Event('scroll'); + if (timeStamp) { + jest.spyOn(event, 'timeStamp', 'get').mockReturnValue(timeStamp); + } + events['scroll'](event); }); // When scrolling top, we wait for the user to reach the top, and then for a new scrolling event @@ -153,7 +157,8 @@ describe('InfiniteScroll', () => { expect(await screen.findByTestId('contents')).toBeInTheDocument(); - scrollTo(endPosition); + scrollTo(endPosition - 1, 1); + scrollTo(endPosition, 600); expect(loadMoreMock).toHaveBeenCalled(); expect(await screen.findByTestId('Spinner')).toBeInTheDocument(); @@ -177,7 +182,8 @@ describe('InfiniteScroll', () => { expect(await screen.findByTestId('contents')).toBeInTheDocument(); - wheel(deltaY); + wheel(deltaY, 1); + wheel(deltaY, 600); expect(loadMoreMock).toHaveBeenCalled(); expect(await screen.findByTestId('Spinner')).toBeInTheDocument(); @@ -192,7 +198,8 @@ describe('InfiniteScroll', () => { element.clientHeight = 40; element.scrollHeight = element.clientHeight; - scrollTo(40); + scrollTo(39, 1); + scrollTo(40, 600); expect(loadMoreMock).not.toHaveBeenCalled(); expect(screen.queryByTestId('Spinner')).not.toBeInTheDocument(); @@ -207,7 +214,8 @@ describe('InfiniteScroll', () => { expect(await screen.findByTestId('contents')).toBeInTheDocument(); - scrollTo(endPosition); + scrollTo(endPosition - 1, 1); + scrollTo(endPosition, 600); expect(loadMoreMock).toHaveBeenCalledWith({ from: rows[rows.length - 1].timeEpochMs, @@ -224,7 +232,8 @@ describe('InfiniteScroll', () => { expect(await screen.findByTestId('contents')).toBeInTheDocument(); - scrollTo(endPosition); + scrollTo(endPosition - 1, 1); + scrollTo(endPosition, 600); expect(loadMoreMock).toHaveBeenCalledWith({ from: absoluteRange.from, @@ -246,7 +255,8 @@ describe('InfiniteScroll', () => { expect(await screen.findByTestId('contents')).toBeInTheDocument(); - scrollTo(endPosition); + scrollTo(endPosition - 1, 1); + scrollTo(endPosition, 600); expect(loadMoreMock).not.toHaveBeenCalled(); expect(screen.queryByTestId('Spinner')).not.toBeInTheDocument(); @@ -269,7 +279,8 @@ describe('InfiniteScroll', () => { expect(await screen.findByTestId('contents')).toBeInTheDocument(); - scrollTo(endPosition); + scrollTo(endPosition - 1, 1); + scrollTo(endPosition, 600); expect(loadMoreMock).not.toHaveBeenCalled(); expect(screen.queryByTestId('Spinner')).not.toBeInTheDocument(); diff --git a/public/app/features/logs/components/InfiniteScroll.tsx b/public/app/features/logs/components/InfiniteScroll.tsx index 36d748d2db1..5ebe5516332 100644 --- a/public/app/features/logs/components/InfiniteScroll.tsx +++ b/public/app/features/logs/components/InfiniteScroll.tsx @@ -98,6 +98,7 @@ export const InfiniteScroll = ({ } else if (scrollDirection === ScrollDirection.Bottom) { scrollBottom(); } + lastEvent.current = null; } function scrollTop() { @@ -245,7 +246,7 @@ export function shouldLoadMore( return ScrollDirection.NoScroll; } - if (lastEvent && shouldIgnoreChainOfEvents(event, lastEvent, countRef)) { + if (!lastEvent || shouldIgnoreChainOfEvents(event, lastEvent, countRef)) { return ScrollDirection.NoScroll; } diff --git a/public/app/plugins/panel/logs/LogsPanel.tsx b/public/app/plugins/panel/logs/LogsPanel.tsx index a335e830aca..088f4fd2b14 100644 --- a/public/app/plugins/panel/logs/LogsPanel.tsx +++ b/public/app/plugins/panel/logs/LogsPanel.tsx @@ -143,7 +143,7 @@ export const LogsPanel = ({ const [panelData, setPanelData] = useState(data); const dataSourcesMap = useDatasourcesFromTargets(panelData.request?.targets); // Prevents the scroll position to change when new data from infinite scrolling is received - const keepScrollPositionRef = useRef(false); + const keepScrollPositionRef = useRef(null); let closeCallback = useRef<() => void>(); const { eventBus, onAddAdHocFilter } = usePanelContext(); @@ -290,7 +290,8 @@ export const LogsPanel = ({ useLayoutEffect(() => { if (!logsContainerRef.current || !scrollElement || keepScrollPositionRef.current) { - keepScrollPositionRef.current = false; + keepScrollPositionRef.current = + keepScrollPositionRef.current === 'infinite-scroll' ? null : keepScrollPositionRef.current; return; } /** @@ -370,6 +371,30 @@ export const LogsPanel = ({ } }, [options.displayedFields]); + // Respect the scroll position when refreshing the panel + useEffect(() => { + function handleScroll() { + if (!scrollElement) { + return; + } + // Signal to keep the user scroll position + keepScrollPositionRef.current = 'user'; + const atTheBottom = scrollElement.scrollHeight - scrollElement.scrollTop - scrollElement.clientHeight === 0; + // Except when the user resets the scroll to the original position depending on the sort direction + if (scrollElement.scrollTop === 0 && !isAscending) { + keepScrollPositionRef.current = null; + } else if (atTheBottom && isAscending) { + keepScrollPositionRef.current = null; + } + } + scrollElement?.addEventListener('scroll', handleScroll); + scrollElement?.addEventListener('wheel', handleScroll); + return () => { + scrollElement?.removeEventListener('scroll', handleScroll); + scrollElement?.removeEventListener('wheel', handleScroll); + }; + }, [isAscending, scrollElement]); + const loadMoreLogs = useCallback( async (scrollRange: AbsoluteTimeRange) => { if (!data.request || !config.featureToggles.logsInfiniteScrolling || loadingRef.current) { @@ -391,7 +416,7 @@ export const LogsPanel = ({ loadingRef.current = false; } - keepScrollPositionRef.current = true; + keepScrollPositionRef.current = 'infinite-scroll'; setPanelData({ ...panelData, series: newSeries,