From 931ae02f2642e0b09aaf449862bc315e7d98f9db Mon Sep 17 00:00:00 2001 From: Matias Chomicki Date: Tue, 11 Apr 2023 16:26:50 +0200 Subject: [PATCH] Logs Navigation: Scroll to first log when using pagination (#66214) * Logs: add reference to the start of the logs * Logs: Improve pagination by scrolling to the first log * Logs: move first log ref * Logs navigation: reset scroll on page changes * Update tests * Logs navigation: unify reference to start of logs for scrolling to top * Chore: update test title * Move scrolling reference a bit more to the top --- public/app/features/explore/Logs.tsx | 3 +- .../features/explore/LogsNavigation.test.tsx | 20 ++++++++--- .../app/features/explore/LogsNavigation.tsx | 33 ++++++++++++++----- .../explore/LogsNavigationPages.test.tsx | 12 +++++-- .../features/explore/LogsNavigationPages.tsx | 20 +++-------- 5 files changed, 56 insertions(+), 32 deletions(-) diff --git a/public/app/features/explore/Logs.tsx b/public/app/features/explore/Logs.tsx index 263595de654..63964d11b06 100644 --- a/public/app/features/explore/Logs.tsx +++ b/public/app/features/explore/Logs.tsx @@ -397,7 +397,7 @@ class UnthemedLogs extends PureComponent { )} -
+
{
+
{ +const setup = (propOverrides?: Partial) => { const props = { ...defaultProps, ...propOverrides, @@ -73,10 +74,10 @@ describe('LogsNavigation', () => { expect(screen.getByTestId('logsNavigationPages')).toBeInTheDocument(); }); - it('should correctly request older logs when flipped order', () => { + it('should correctly request older logs when flipped order', async () => { const onChangeTimeMock = jest.fn(); const { rerender } = setup({ onChangeTime: onChangeTimeMock }); - fireEvent.click(screen.getByTestId('olderLogsButton')); + await userEvent.click(screen.getByTestId('olderLogsButton')); expect(onChangeTimeMock).toHaveBeenCalledWith({ from: 1637319359000, to: 1637322959000 }); rerender( @@ -88,7 +89,16 @@ describe('LogsNavigation', () => { logsSortOrder={LogsSortOrder.Ascending} /> ); - fireEvent.click(screen.getByTestId('olderLogsButton')); + await userEvent.click(screen.getByTestId('olderLogsButton')); expect(onChangeTimeMock).toHaveBeenCalledWith({ from: 1637319338000, to: 1637322938000 }); }); + + it('should reset the scroll when pagination is clic ked', async () => { + const scrollToTopLogsMock = jest.fn(); + setup({ scrollToTopLogs: scrollToTopLogsMock }); + + expect(scrollToTopLogsMock).not.toHaveBeenCalled(); + await userEvent.click(screen.getByTestId('olderLogsButton')); + expect(scrollToTopLogsMock).toHaveBeenCalled(); + }); }); diff --git a/public/app/features/explore/LogsNavigation.tsx b/public/app/features/explore/LogsNavigation.tsx index ba3193c7594..250ac8c6e89 100644 --- a/public/app/features/explore/LogsNavigation.tsx +++ b/public/app/features/explore/LogsNavigation.tsx @@ -1,6 +1,6 @@ import { css } from '@emotion/css'; import { isEqual } from 'lodash'; -import React, { memo, useEffect, useRef, useState } from 'react'; +import React, { memo, useCallback, useEffect, useRef, useState } from 'react'; import { AbsoluteTimeRange, GrafanaTheme2, LogsSortOrder, TimeZone } from '@grafana/data'; import { reportInteraction } from '@grafana/runtime'; @@ -54,7 +54,7 @@ function LogsNavigation({ const onFirstPage = oldestLogsFirst ? currentPageIndex === pages.length - 1 : currentPageIndex === 0; const onLastPage = oldestLogsFirst ? currentPageIndex === 0 : currentPageIndex === pages.length - 1; const theme = useTheme2(); - const styles = getStyles(theme, oldestLogsFirst, loading); + const styles = getStyles(theme, oldestLogsFirst); // Main effect to set pages and index useEffect(() => { @@ -91,10 +91,13 @@ function LogsNavigation({ // eslint-disable-next-line react-hooks/exhaustive-deps }, []); - const changeTime = ({ from, to }: AbsoluteTimeRange) => { - expectedRangeRef.current = { from, to }; - onChangeTime({ from, to }); - }; + const changeTime = useCallback( + ({ from, to }: AbsoluteTimeRange) => { + expectedRangeRef.current = { from, to }; + onChangeTime({ from, to }); + }, + [onChangeTime] + ); const sortPages = (a: LogsPage, b: LogsPage, logsSortOrder?: LogsSortOrder | null) => { if (logsSortOrder === LogsSortOrder.Ascending) { @@ -123,6 +126,7 @@ function LogsNavigation({ //If we are on the last page, create new range changeTime({ from: visibleRange.from - rangeSpanRef.current, to: visibleRange.from }); } + scrollToTopLogs(); }} disabled={loading} > @@ -150,6 +154,7 @@ function LogsNavigation({ to: pages[currentPageIndex + indexChange].queryRange.to, }); } + scrollToTopLogs(); //If we are on the first page, button is disabled and we do nothing }} disabled={loading || onFirstPage} @@ -162,6 +167,18 @@ function LogsNavigation({ ); + const onPageClick = useCallback( + (page: LogsPage, pageNumber: number) => { + reportInteraction('grafana_explore_logs_pagination_clicked', { + pageType: 'page', + pageNumber, + }); + !loading && changeTime({ from: page.queryRange.from, to: page.queryRange.to }); + scrollToTopLogs(); + }, + [changeTime, loading, scrollToTopLogs] + ); + return (
{oldestLogsFirst ? olderLogsButton : newerLogsButton} @@ -171,7 +188,7 @@ function LogsNavigation({ oldestLogsFirst={oldestLogsFirst} timeZone={timeZone} loading={loading} - changeTime={changeTime} + onClick={onPageClick} /> {oldestLogsFirst ? newerLogsButton : olderLogsButton}