New Log Context: Scroll and resize improvements (#109143)

* LogList: keep scroll position after scrolling top * Infinite Scroll: add unit tests * Remove log * Remove log * LogList: unify height calculation * LogList: manage list height with resize observer * LogLineDetails: report app with details displayed
2025-08-05 15:03:03 +02:00
parent e7d633b002
commit 94874823ce
5 changed files with 403 additions and 41 deletions
@@ -107,11 +107,12 @@ function setup(
  return { element, events, scrollTo, wheel };
 }

+const originalState = config.featureToggles.logsInfiniteScrolling;
 beforeAll(() => {
  config.featureToggles.logsInfiniteScrolling = true;
 });
 afterAll(() => {
-  config.featureToggles.logsInfiniteScrolling = false;
+  config.featureToggles.logsInfiniteScrolling = originalState;
 });

 describe('InfiniteScroll', () => {
@@ -0,0 +1,352 @@
+import { act, render, screen } from '@testing-library/react';
+import { VariableSizeList } from 'react-window';
+
+import { createTheme, dateTimeForTimeZone, rangeUtil } from '@grafana/data';
+import { config } from '@grafana/runtime';
+import { LogsSortOrder } from '@grafana/schema';
+
+import { ScrollDirection, SCROLLING_THRESHOLD } from '../InfiniteScroll';
+import { createLogLine } from '../mocks/logRow';
+
+import { InfiniteScroll, InfiniteScrollMode, Props } from './InfiniteScroll';
+import { LogListModel } from './processing';
+import { LogLineVirtualization } from './virtualization';
+
+const defaultTz = 'browser';
+
+const absoluteRange = {
+  from: 1702578600000,
+  to: 1702578900000,
+};
+const defaultRange = rangeUtil.convertRawToRange({
+  from: dateTimeForTimeZone(defaultTz, absoluteRange.from),
+  to: dateTimeForTimeZone(defaultTz, absoluteRange.to),
+});
+
+const theme = createTheme();
+const virtualization = new LogLineVirtualization(theme, 'default');
+const defaultProps: Omit<Props, 'children' | 'scrollElement'> = {
+  loadMore: jest.fn(),
+  timeRange: defaultRange,
+  logs: [],
+  sortOrder: LogsSortOrder.Descending,
+  timeZone: 'browser',
+  displayedFields: [],
+  handleOverflow: jest.fn(),
+  infiniteScrollMode: 'interval',
+  onClick: jest.fn(),
+  setInitialScrollPosition: jest.fn(),
+  showTime: false,
+  virtualization,
+  wrapLogMessage: false,
+};
+
+function setup(
+  loadMoreMock: () => void,
+  startPosition: number,
+  logs: LogListModel[],
+  order: LogsSortOrder,
+  infiniteScrollMode: InfiniteScrollMode = 'interval'
+) {
+  const { element, events } = getMockElement(startPosition);
+
+  function scrollTo(position: number, timeStamp?: number) {
+    element.scrollTop = position;
+
+    act(() => {
+      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
+    // in the same direction before triggering a new query.
+    if (position === 0) {
+      wheel(-1);
+    }
+  }
+  function wheel(deltaY: number, timeStamp?: number) {
+    element.scrollTop += deltaY;
+    if (element.scrollTop < 0) {
+      element.scrollTop = 0;
+    }
+
+    act(() => {
+      const event = new WheelEvent('wheel', { deltaY });
+      if (timeStamp) {
+        jest.spyOn(event, 'timeStamp', 'get').mockReturnValue(timeStamp);
+      }
+      events['wheel'](event);
+    });
+  }
+
+  render(
+    <InfiniteScroll
+      {...defaultProps}
+      sortOrder={order}
+      logs={logs}
+      scrollElement={element as unknown as HTMLDivElement}
+      loadMore={loadMoreMock}
+      infiniteScrollMode={infiniteScrollMode}
+    >
+      {({ getItemKey, itemCount, onItemsRendered, Renderer }) => (
+        <VariableSizeList
+          height={100}
+          itemCount={itemCount}
+          itemSize={() => virtualization.getLineHeight()}
+          itemKey={getItemKey}
+          layout="vertical"
+          onItemsRendered={onItemsRendered}
+          style={{ overflow: 'scroll' }}
+          width="100%"
+        >
+          {Renderer}
+        </VariableSizeList>
+      )}
+    </InfiniteScroll>
+  );
+
+  return { element, events, scrollTo, wheel };
+}
+
+const originalState = config.featureToggles.logsInfiniteScrolling;
+beforeAll(() => {
+  config.featureToggles.logsInfiniteScrolling = true;
+});
+afterAll(() => {
+  config.featureToggles.logsInfiniteScrolling = originalState;
+});
+
+describe('InfiniteScroll', () => {
+  describe.each([LogsSortOrder.Descending, LogsSortOrder.Ascending])(
+    'When the sort order is descending',
+    (order: LogsSortOrder) => {
+      let logs: LogListModel[];
+      beforeEach(() => {
+        logs = createLogs(absoluteRange.from + 2 * SCROLLING_THRESHOLD, absoluteRange.to - 2 * SCROLLING_THRESHOLD);
+      });
+
+      test.each([
+        ['top', 10, 0],
+        ['bottom', 50, 60],
+      ])(
+        'Requests more logs when scrolling %s',
+        async (direction: string, startPosition: number, endPosition: number) => {
+          const loadMoreMock = jest.fn();
+          const { scrollTo } = setup(loadMoreMock, startPosition, logs, order);
+
+          expect(await screen.findByText('log line 1')).toBeInTheDocument();
+
+          scrollTo(endPosition - 1, 1);
+          scrollTo(endPosition, 600);
+
+          expect(loadMoreMock).toHaveBeenCalled();
+          expect(await screen.findByTestId('Spinner')).toBeInTheDocument();
+        }
+      );
+
+      test.each([
+        ['up', -5, 0],
+        ['down', 5, 60],
+      ])(
+        'Requests more logs when moving the mousewheel %s',
+        async (_: string, deltaY: number, startPosition: number) => {
+          const loadMoreMock = jest.fn();
+          const { wheel } = setup(loadMoreMock, startPosition, logs, order);
+
+          expect(await screen.findByText('log line 1')).toBeInTheDocument();
+
+          wheel(deltaY, 1);
+          wheel(deltaY, 600);
+
+          expect(loadMoreMock).toHaveBeenCalled();
+          expect(await screen.findByTestId('Spinner')).toBeInTheDocument();
+        }
+      );
+
+      test('Does not request more logs when there is no scroll', async () => {
+        const loadMoreMock = jest.fn();
+        const { scrollTo, element } = setup(loadMoreMock, 0, logs, order);
+
+        expect(await screen.findByText('log line 1')).toBeInTheDocument();
+        element.clientHeight = 40;
+        element.scrollHeight = element.clientHeight;
+
+        scrollTo(39, 1);
+        scrollTo(40, 600);
+
+        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 : 50; // Scroll top
+        const endPosition = order === LogsSortOrder.Descending ? 0 : 60; // Scroll bottom
+
+        const loadMoreMock = jest.fn();
+        const { scrollTo } = setup(loadMoreMock, startPosition, logs, order);
+
+        expect(await screen.findByText('log line 1')).toBeInTheDocument();
+
+        scrollTo(endPosition - 1, 1);
+        scrollTo(endPosition, 600);
+
+        expect(loadMoreMock).toHaveBeenCalledWith(
+          {
+            from: logs[logs.length - 1].timeEpochMs,
+            to: absoluteRange.to,
+          },
+          order === LogsSortOrder.Descending ? -1 : 1
+        );
+      });
+
+      test('Requests older logs from the oldest timestamp', async () => {
+        const startPosition = order === LogsSortOrder.Ascending ? 10 : 50; // Scroll top
+        const endPosition = order === LogsSortOrder.Ascending ? 0 : 60; // Scroll bottom
+
+        const loadMoreMock = jest.fn();
+        const { scrollTo } = setup(loadMoreMock, startPosition, logs, order);
+
+        expect(await screen.findByText('log line 1')).toBeInTheDocument();
+
+        scrollTo(endPosition - 1, 1);
+        scrollTo(endPosition, 600);
+
+        expect(loadMoreMock).toHaveBeenCalledWith(
+          {
+            from: absoluteRange.from,
+            to: logs[0].timeEpochMs,
+          },
+          order === LogsSortOrder.Ascending ? -1 : 1
+        );
+      });
+
+      describe('With absolute range matching visible range', () => {
+        test('It does not request more when scrolling bottom', async () => {
+          // Visible range matches the current range
+          const logs = createLogs(absoluteRange.from, absoluteRange.to);
+          const loadMoreMock = jest.fn();
+          const { scrollTo } = setup(loadMoreMock, 50, logs, order);
+
+          expect(await screen.findByText('log line 1')).toBeInTheDocument();
+
+          scrollTo(59, 1);
+          scrollTo(60, 600);
+
+          expect(loadMoreMock).not.toHaveBeenCalled();
+          expect(screen.queryByTestId('Spinner')).not.toBeInTheDocument();
+          expect(await screen.findByText('End of the selected time range.')).toBeInTheDocument();
+        });
+      });
+
+      describe('With relative range matching visible range', () => {
+        test('It does not request more when scrolling bottom', async () => {
+          // Visible range matches the current range
+          const logs = createLogs(absoluteRange.from, absoluteRange.to);
+          const loadMoreMock = jest.fn();
+          const { scrollTo } = setup(loadMoreMock, 50, logs, order);
+
+          expect(await screen.findByText('log line 1')).toBeInTheDocument();
+
+          scrollTo(59, 1);
+          scrollTo(60, 600);
+
+          expect(loadMoreMock).not.toHaveBeenCalled();
+          expect(screen.queryByTestId('Spinner')).not.toBeInTheDocument();
+          expect(await screen.findByText('End of the selected time range.')).toBeInTheDocument();
+        });
+      });
+
+      describe('Chain of events', () => {
+        test('Ingnores chains of events', async () => {
+          const loadMoreMock = jest.fn();
+          const { wheel } = setup(loadMoreMock, 57, logs, order);
+
+          expect(await screen.findByText('log line 1')).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, logs, order);
+
+          expect(await screen.findByText('log line 1')).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, logs, order);
+
+          expect(await screen.findByText('log line 1')).toBeInTheDocument();
+
+          for (let i = 0; i <= 25; i++) {
+            wheel(1, 399 * i + 399);
+          }
+
+          expect(loadMoreMock).toHaveBeenCalledTimes(1);
+        });
+      });
+
+      describe('With scroll mode unlimited', () => {
+        test('Allows infinite scroll in the top direction', async () => {
+          const loadMoreMock = jest.fn();
+          const { wheel } = setup(loadMoreMock, 0, logs, order, 'unlimited');
+
+          expect(await screen.findByText('log line 1')).toBeInTheDocument();
+
+          wheel(-5, 1);
+          wheel(-5, 600);
+
+          expect(loadMoreMock).toHaveBeenCalledWith(expect.anything(), ScrollDirection.Top);
+        });
+      });
+    }
+  );
+});
+
+function createLogs(from: number, to: number) {
+  const rows = [
+    createLogLine({ entry: 'log line 1', uid: 'log-1' }),
+    createLogLine({ entry: 'log line 2', uid: 'log-22' }),
+  ];
+  // Time field
+  rows[0].dataFrame.fields[0].values = [from, to];
+  rows[0].timeEpochMs = from;
+  rows[1].dataFrame.fields[0].values = [from, to];
+  rows[1].timeEpochMs = to;
+  return rows;
+}
+
+// JSDOM doesn't support layout, so we will mock the expected attribute values for the test cases.
+function getMockElement(scrollTop: number) {
+  const events: Record<string, (e: Event | WheelEvent) => void> = {};
+  const element = {
+    addEventListener: (event: string, callback: (e: Event | WheelEvent) => void) => {
+      events[event] = callback;
+    },
+    removeEventListener: jest.fn(),
+    stopImmediatePropagation: jest.fn(),
+    scrollHeight: 100,
+    clientHeight: 40,
+    scrollTop,
+    scrollTo: jest.fn(),
+    scroll: jest.fn(),
+  };
+
+  return { element, events };
+}
@@ -21,7 +21,7 @@ interface ChildrenProps {
  Renderer: (props: ListChildComponentProps) => ReactNode;
 }

-interface Props {
+export interface Props {
  children: (props: ChildrenProps) => ReactNode;
  displayedFields: string[];
  handleOverflow: (index: number, id: string, height?: number) => void;
@@ -30,7 +30,7 @@ interface Props {
  logs: LogListModel[];
  onClick: (e: MouseEvent<HTMLElement>, log: LogListModel) => void;
  scrollElement: HTMLDivElement | null;
-  setInitialScrollPosition: () => void;
+  setInitialScrollPosition: (log?: LogListModel) => void;
  showTime: boolean;
  sortOrder: LogsSortOrder;
  timeRange: TimeRange;
@@ -72,6 +72,7 @@ export const InfiniteScroll = ({
  const lastLogOfPage = useRef<string[]>([]);
  const styles = useStyles2(getStyles, virtualization);
  const resetStateTimeout = useRef<ReturnType<typeof setTimeout> | null>(null);
+  const scrollToLogLineRef = useRef<LogListModel | undefined>(undefined);

  useEffect(() => {
    // Logs have not changed, ignore effect
@@ -84,6 +85,9 @@ export const InfiniteScroll = ({
      setInfiniteLoaderState(
        logs.length === prevLogs.length && infiniteScrollMode === 'interval' ? 'out-of-bounds' : 'idle'
      );
+      if (scrollToLogLineRef.current) {
+        setAutoScroll(true);
+      }
    } else {
      lastLogOfPage.current = [];
      setAutoScroll(true);
@@ -98,7 +102,8 @@ export const InfiniteScroll = ({

  useEffect(() => {
    if (autoScroll) {
-      setInitialScrollPosition();
+      setInitialScrollPosition(scrollToLogLineRef.current);
+      scrollToLogLineRef.current = undefined;
      setAutoScroll(false);
    }
  }, [autoScroll, setInitialScrollPosition]);
@@ -116,6 +121,7 @@ export const InfiniteScroll = ({
      if (scrollDirection === ScrollDirection.Bottom) {
        lastLogOfPage.current.push(logs[logs.length - 1].uid);
      } else {
+        scrollToLogLineRef.current = logs[0];
        lastLogOfPage.current.push(logs[0].uid);
      }
      setInfiniteLoaderState('loading');
@@ -65,7 +65,7 @@ export const LogLineDetails = memo(({ containerElement, focusLogLine, logs, onRe
 LogLineDetails.displayName = 'LogLineDetails';

 const LogLineDetailsTabs = memo(({ focusLogLine, logs }: Pick<Props, 'focusLogLine' | 'logs'>) => {
-  const { closeDetails, noInteractions, showDetails, toggleDetails } = useLogListContext();
+  const { app, closeDetails, noInteractions, showDetails, toggleDetails } = useLogListContext();
  const [currentLog, setCurrentLog] = useState(showDetails[0]);
  const previousShowDetails = usePrevious(showDetails);
  const styles = useStyles2(getStyles, 'sidebar');
@@ -75,6 +75,7 @@ const LogLineDetailsTabs = memo(({ focusLogLine, logs }: Pick<Props, 'focusLogLi
    if (!noInteractions) {
      reportInteraction('logs_log_line_details_displayed', {
        mode: 'sidebar',
+        app,
      });
    }
    // Once
@@ -133,7 +134,7 @@ export interface InlineLogLineDetailsProps {
 }

 export const InlineLogLineDetails = memo(({ logs, log }: InlineLogLineDetailsProps) => {
-  const { noInteractions } = useLogListContext();
+  const { app, noInteractions } = useLogListContext();
  const styles = useStyles2(getStyles, 'inline');
  const scrollRef = useRef<HTMLDivElement | null>(null);

@@ -141,9 +142,10 @@ export const InlineLogLineDetails = memo(({ logs, log }: InlineLogLineDetailsPro
    if (!noInteractions) {
      reportInteraction('logs_log_line_details_displayed', {
        mode: 'inline',
+        app,
      });
    }
-  }, [noInteractions]);
+  }, [app, noInteractions]);

  const saveScroll = useCallback(() => {
    saveDetailsScrollPosition(log, scrollRef.current?.scrollTop ?? 0);
@@ -245,11 +245,7 @@ const LogListComponent = ({
    wrapLogMessage,
  } = useLogListContext();
  const [processedLogs, setProcessedLogs] = useState<LogListModel[]>([]);
-  const [listHeight, setListHeight] = useState(
-    app === CoreApp.Explore
-      ? Math.max(window.innerHeight * 0.8, containerElement.clientHeight)
-      : containerElement.clientHeight
-  );
+  const [listHeight, setListHeight] = useState(getListHeight(containerElement, app));
  const theme = useTheme2();
  const listRef = useRef<VariableSizeList | null>(null);
  const widthRef = useRef(containerElement.clientWidth);
@@ -327,28 +323,21 @@ const LogListComponent = ({
    listRef.current?.resetAfterIndex(0);
  }, [wrapLogMessage, showDetails, displayedFields, dedupStrategy]);

-  useEffect(() => {
-    const handleResize = debounce(() => {
-      setListHeight(
-        (app === CoreApp.Explore
-          ? Math.max(window.innerHeight * 0.8, containerElement.clientHeight)
-          : containerElement.clientHeight) - (searchVisible ? LOG_LIST_SEARCH_HEIGHT : 0)
-      );
-    }, 50);
-    window.addEventListener('resize', handleResize);
-    handleResize();
-    return () => {
-      window.removeEventListener('resize', handleResize);
-    };
-  }, [app, containerElement.clientHeight, searchVisible]);
+  useLayoutEffect(() => {
+    if (widthRef.current !== widthContainer.clientWidth) {
+      widthRef.current = widthContainer.clientWidth;
+      debouncedResetAfterIndex(0);
+    }
+  });

  useLayoutEffect(() => {
-    if (widthRef.current === widthContainer.clientWidth) {
-      return;
-    }
-    widthRef.current = widthContainer.clientWidth;
-    debouncedResetAfterIndex(0);
-  });
+    const handleResize = debounce(() => {
+      setListHeight(getListHeight(containerElement, app, searchVisible));
+    }, 50);
+    const observer = new ResizeObserver(() => handleResize());
+    observer.observe(containerElement);
+    return () => observer.disconnect();
+  }, [app, containerElement, searchVisible]);

  const overflowIndexRef = useRef(Infinity);
  const handleOverflow = useCallback(
@@ -365,16 +354,20 @@ const LogListComponent = ({
    [debouncedResetAfterIndex, virtualization, widthContainer]
  );

-  const handleScrollPosition = useCallback(() => {
-    if (permalinkedLogId) {
-      const index = processedLogs.findIndex((log) => log.uid === permalinkedLogId);
-      if (index >= 0) {
-        listRef.current?.scrollToItem(index, 'start');
-        return;
+  const handleScrollPosition = useCallback(
+    (log?: LogListModel) => {
+      const scrollToUID = log ? log.uid : permalinkedLogId;
+      if (scrollToUID) {
+        const index = processedLogs.findIndex((log) => log.uid === scrollToUID);
+        if (index >= 0) {
+          listRef.current?.scrollToItem(index, 'start');
+          return;
+        }
      }
-    }
-    listRef.current?.scrollToItem(initialScrollPosition === 'top' ? 0 : processedLogs.length - 1);
-  }, [initialScrollPosition, permalinkedLogId, processedLogs]);
+      listRef.current?.scrollToItem(initialScrollPosition === 'top' ? 0 : processedLogs.length - 1);
+    },
+    [initialScrollPosition, permalinkedLogId, processedLogs]
+  );

  if (!containerElement || listHeight == null) {
    // Wait for container to be rendered
@@ -552,3 +545,11 @@ function handleScrollToEvent(event: ScrollToLogsEvent, logs: LogListModel[], lis
    }
  }
 }
+
+function getListHeight(containerElement: HTMLDivElement, app: CoreApp, searchVisible = false) {
+  return (
+    (app === CoreApp.Explore
+      ? Math.max(window.innerHeight * 0.8, containerElement.clientHeight)
+      : containerElement.clientHeight) - (searchVisible ? LOG_LIST_SEARCH_HEIGHT : 0)
+  );
+}