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

@@ -397,7 +397,7 @@ class UnthemedLogs extends PureComponent<Props, State> {
           )}
         </Collapse>
         <Collapse label="Logs" loading={loading} isOpen className={styleOverridesForStickyNavigation}>
-          <div className={styles.logOptions} ref={this.topLogsRef}>
+          <div className={styles.logOptions}>
             <InlineFieldRow>
               <InlineField label="Time" className={styles.horizontalInlineLabel} transparent>
                 <InlineSwitch
@@ -471,6 +471,7 @@ class UnthemedLogs extends PureComponent<Props, State> {
               </InlineField>
             </div>
           </div>
+          <div ref={this.topLogsRef} />
           <LogsMetaRow
             logRows={logRows}
             meta={logsMeta || []}

public/app/features/explore/LogsNavigation.test.tsx

@@ -1,4 +1,5 @@
-import { render, screen, fireEvent } from '@testing-library/react';
+import { render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
 import React, { ComponentProps } from 'react';
 
 import { LogsSortOrder } from '@grafana/data';
@@ -25,7 +26,7 @@ const defaultProps: LogsNavigationProps = {
   clearCache: jest.fn(),
 };
 
-const setup = (propOverrides?: object) => {
+const setup = (propOverrides?: Partial<LogsNavigationProps>) => {
   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();
+  });
 });

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({
     </Button>
   );
 
+  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 (
     <div className={styles.navContainer}>
       {oldestLogsFirst ? olderLogsButton : newerLogsButton}
@@ -171,7 +188,7 @@ function LogsNavigation({
         oldestLogsFirst={oldestLogsFirst}
         timeZone={timeZone}
         loading={loading}
-        changeTime={changeTime}
+        onClick={onPageClick}
       />
       {oldestLogsFirst ? newerLogsButton : olderLogsButton}
       <Button
@@ -189,7 +206,7 @@ function LogsNavigation({
 
 export default memo(LogsNavigation);
 
-const getStyles = (theme: GrafanaTheme2, oldestLogsFirst: boolean, loading: boolean) => {
+const getStyles = (theme: GrafanaTheme2, oldestLogsFirst: boolean) => {
   const navContainerHeight = theme.flags.topnav
     ? `calc(100vh - 2*${theme.spacing(2)} - 2*${TOP_BAR_LEVEL_HEIGHT}px)`
     : '95vh';

public/app/features/explore/LogsNavigationPages.test.tsx

@@ -1,11 +1,12 @@
 import { render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
 import React, { ComponentProps } from 'react';
 
 import { LogsNavigationPages } from './LogsNavigationPages';
 
 type LogsNavigationPagesProps = ComponentProps<typeof LogsNavigationPages>;
 
-const setup = (propOverrides?: object) => {
+const setup = (propOverrides?: Partial<LogsNavigationPagesProps>) => {
   const props: LogsNavigationPagesProps = {
     pages: [
       {
@@ -21,7 +22,7 @@ const setup = (propOverrides?: object) => {
     oldestLogsFirst: false,
     timeZone: 'local',
     loading: false,
-    changeTime: jest.fn(),
+    onClick: jest.fn(),
     ...propOverrides,
   };
 
@@ -43,4 +44,11 @@ describe('LogsNavigationPages', () => {
     expect(screen.getByText(/02:59:11 — 02:59:15/i)).toBeInTheDocument();
     expect(screen.getByText(/02:59:01 — 02:59:05/i)).toBeInTheDocument();
   });
+  it('should invoke the callback when clicked', async () => {
+    const onPageClicked = jest.fn();
+    setup({ onClick: onPageClicked });
+    expect(onPageClicked).not.toHaveBeenCalled();
+    await userEvent.click(screen.getByText(/02:59:05 — 02:59:01/i));
+    expect(onPageClicked).toHaveBeenCalled();
+  });
 });

public/app/features/explore/LogsNavigationPages.tsx

@@ -1,8 +1,7 @@
 import { css, cx } from '@emotion/css';
 import React from 'react';
 
-import { dateTimeFormat, systemDateFormats, TimeZone, AbsoluteTimeRange, GrafanaTheme2 } from '@grafana/data';
-import { reportInteraction } from '@grafana/runtime';
+import { dateTimeFormat, systemDateFormats, TimeZone, GrafanaTheme2 } from '@grafana/data';
 import { CustomScrollbar, Spinner, useTheme2, clearButtonStyles } from '@grafana/ui';
 
 import { LogsPage } from './LogsNavigation';
@@ -13,17 +12,10 @@ type Props = {
   oldestLogsFirst: boolean;
   timeZone: TimeZone;
   loading: boolean;
-  changeTime: (range: AbsoluteTimeRange) => void;
+  onClick: (page: LogsPage, pageNumber: number) => void;
 };
 
-export function LogsNavigationPages({
-  pages,
-  currentPageIndex,
-  oldestLogsFirst,
-  timeZone,
-  loading,
-  changeTime,
-}: Props) {
+export function LogsNavigationPages({ pages, currentPageIndex, oldestLogsFirst, timeZone, loading, onClick }: Props) {
   const formatTime = (time: number) => {
     return `${dateTimeFormat(time, {
       format: systemDateFormats.interval.second,
@@ -54,11 +46,7 @@ export function LogsNavigationPages({
               className={cx(clearButtonStyles(theme), styles.page)}
               key={page.queryRange.to}
               onClick={() => {
-                reportInteraction('grafana_explore_logs_pagination_clicked', {
-                  pageType: 'page',
-                  pageNumber: index + 1,
-                });
-                !loading && changeTime({ from: page.queryRange.from, to: page.queryRange.to });
+                onClick(page, index + 1);
               }}
             >
               <div className={cx(styles.line, { selectedBg: currentPageIndex === index })} />