New Logs Panel: follow up improvements and fixes (#105767)

* LogList: add version with no scroll * LogList: fix support for permalinked and pinned logs * LogLineDetails: improve resizable * LogList: improve height adjustment when details are open * LogList: let the people select text * Revert "LogList: add version with no scroll" This reverts commit f26cdce696. * LogList: update test * New Logs Panel: rename permalinkedRowId to permalinkedLogId * LogListContext: update mock * ControlledLogRows: implement custom scrollIntoView * Logs Panel: fix re-render regression * LogLine: tweak hover and expanded colors
2025-05-22 21:12:06 +03:00
parent 67d28465de
commit a676fd620a
9 changed files with 141 additions and 22 deletions
@@ -1063,7 +1063,6 @@ const UnthemedLogs: React.FunctionComponent<Props> = (props: Props) => {
                  onOpenContext={onOpenContext}
                  onPermalinkClick={onPermalinkClick}
                  permalinkedRowId={panelState?.logs?.id}
-                  scrollIntoView={scrollIntoView}
                  isFilterLabelActive={props.isFilterLabelActive}
                  onClickFilterString={props.onClickFilterString}
                  onClickFilterOutString={props.onClickFilterOutString}
@@ -1179,6 +1178,7 @@ const UnthemedLogs: React.FunctionComponent<Props> = (props: Props) => {
                  onPermalinkClick={onPermalinkClick}
                  onPinLine={onPinToContentOutlineClick}
                  onUnpinLine={onPinToContentOutlineClick}
+                  permalinkedLogId={panelState?.logs?.id}
                  pinLineButtonTooltipTitle={pinLineButtonTooltipTitle}
                  pinnedLogs={pinnedLogs}
                  showControls
@@ -1,5 +1,5 @@
 import { css } from '@emotion/css';
-import { useEffect, useMemo, useRef, forwardRef, useImperativeHandle } from 'react';
+import { useEffect, useMemo, useRef, forwardRef, useImperativeHandle, useCallback } from 'react';

 import {
  AbsoluteTimeRange,
@@ -96,7 +96,17 @@ export const ControlledLogRows = forwardRef<HTMLDivElement | null, ControlledLog
 ControlledLogRows.displayName = 'ControlledLogRows';

 const LogRowsComponent = forwardRef<HTMLDivElement | null, LogRowsComponentProps>(
-  ({ loading, loadMoreLogs, deduplicatedRows = [], range, ...rest }: LogRowsComponentProps, ref) => {
+  (
+    {
+      loading,
+      loadMoreLogs,
+      deduplicatedRows = [],
+      range,
+      scrollIntoView: scrollIntoViewProp,
+      ...rest
+    }: LogRowsComponentProps,
+    ref
+  ) => {
    const {
      app,
      dedupStrategy,
@@ -135,6 +145,22 @@ const LogRowsComponent = forwardRef<HTMLDivElement | null, LogRowsComponentProps
      return config.featureToggles.logsInfiniteScrolling ? styles.scrollableLogRows : styles.logRows;
    }, [ref]);

+    const scrollIntoView = useCallback(
+      (element: HTMLElement) => {
+        if (scrollIntoViewProp) {
+          scrollIntoViewProp(element);
+          return;
+        }
+        if (scrollElementRef.current) {
+          scrollElementRef.current.scroll({
+            behavior: 'smooth',
+            top: scrollElementRef.current.scrollTop + element.getBoundingClientRect().top - window.innerHeight / 2,
+          });
+        }
+      },
+      [scrollIntoViewProp]
+    );
+
    return (
      <div className={styles.logRowsContainer}>
        <LogListControls eventBus={eventBus} />
@@ -158,6 +184,7 @@ const LogRowsComponent = forwardRef<HTMLDivElement | null, LogRowsComponentProps
              logsSortOrder={sortOrder}
              scrollElement={scrollElementRef.current}
              prettifyLogMessage={Boolean(prettifyJSON)}
+              scrollIntoView={scrollIntoView}
              showLabels={Boolean(showUniqueLabels)}
              showTime={showTime}
              wrapLogMessage={wrapLogMessage}
@@ -10,7 +10,7 @@ import { LOG_LINE_BODY_FIELD_NAME } from '../LogDetailsBody';
 import { LogMessageAnsi } from '../LogMessageAnsi';

 import { LogLineMenu } from './LogLineMenu';
-import { useLogIsPinned, useLogListContext } from './LogListContext';
+import { useLogIsPermalinked, useLogIsPinned, useLogListContext } from './LogListContext';
 import { LogListModel } from './processing';
 import {
  FIELD_GAP_MULTIPLIER,
@@ -51,6 +51,7 @@ export const LogLine = ({
  );
  const logLineRef = useRef<HTMLDivElement | null>(null);
  const pinned = useLogIsPinned(log);
+  const permalinked = useLogIsPermalinked(log);

  useEffect(() => {
    if (!onOverflow || !logLineRef.current) {
@@ -92,7 +93,7 @@ export const LogLine = ({
  return (
    <div style={style}>
      <div
-        className={`${styles.logLine} ${variant ?? ''} ${pinned ? styles.pinnedLogLine : ''} ${detailsShown ? styles.detailsDisplayed : ''}`}
+        className={`${styles.logLine} ${variant ?? ''} ${pinned ? styles.pinnedLogLine : ''} ${permalinked ? styles.permalinkedLogLine : ''} ${detailsShown ? styles.detailsDisplayed : ''}`}
        ref={onOverflow ? logLineRef : undefined}
        onMouseEnter={handleMouseOver}
        onFocus={handleMouseOver}
@@ -262,7 +263,7 @@ export const getStyles = (theme: GrafanaTheme2) => {
      fontSize: theme.typography.fontSize,
      wordBreak: 'break-all',
      '&:hover': {
-        background: `hsla(0, 0%, 0%, 0.2)`,
+        background: theme.isDark ? `hsla(0, 0%, 0%, 0.3)` : `hsla(0, 0%, 0%, 0.1)`,
      },
      '&.infinite-scroll': {
        '&::before': {
@@ -311,11 +312,14 @@ export const getStyles = (theme: GrafanaTheme2) => {
      },
    }),
    detailsDisplayed: css({
-      background: `hsla(0, 0%, 0%, 0.2)`,
+      background: theme.isDark ? `hsla(0, 0%, 0%, 0.5)` : `hsla(0, 0%, 0%, 0.1)`,
    }),
    pinnedLogLine: css({
      backgroundColor: tinycolor(theme.colors.info.transparent).setAlpha(0.25).toString(),
    }),
+    permalinkedLogLine: css({
+      backgroundColor: tinycolor(theme.colors.info.transparent).setAlpha(0.25).toString(),
+    }),
    menuIcon: css({
      height: getLineHeight(),
      margin: 0,
@@ -4,7 +4,7 @@ import { useCallback, useRef } from 'react';

 import { GrafanaTheme2 } from '@grafana/data';
 import { useTranslate } from '@grafana/i18n';
-import { IconButton, useStyles2, useTheme2 } from '@grafana/ui';
+import { getDragStyles, IconButton, useStyles2, useTheme2 } from '@grafana/ui';
 import { GetFieldLinksFn } from 'app/plugins/panel/logs/types';

 import { LogDetails } from '../LogDetails';
@@ -40,6 +40,7 @@ export const LogLineDetails = ({ containerElement, getFieldLinks, logs, onResize
  const getRows = useCallback(() => logs, [logs]);
  const logRowsStyles = getLogRowStyles(useTheme2());
  const styles = useStyles2(getStyles);
+  const dragStyles = useStyles2(getDragStyles);
  const containerRef = useRef<HTMLDivElement | null>(null);
  const { t } = useTranslate();

@@ -51,7 +52,13 @@ export const LogLineDetails = ({ containerElement, getFieldLinks, logs, onResize
  }, [onResize, setDetailsWidth]);

  return (
-    <Resizable onResize={handleResize} defaultSize={{ width: detailsWidth, height: containerElement.clientHeight }}>
+    <Resizable
+      onResize={handleResize}
+      handleClasses={{ left: dragStyles.dragHandleBaseVertical }}
+      defaultSize={{ width: detailsWidth, height: containerElement.clientHeight }}
+      enable={{ left: true }}
+      minWidth={40}
+    >
      <div className={styles.container} ref={containerRef}>
        <IconButton
          name="times"
@@ -87,4 +87,24 @@ describe('LogList', () => {
    expect(screen.queryByText('Fields')).not.toBeInTheDocument();
    expect(screen.queryByText('Close log details')).not.toBeInTheDocument();
  });
+
+  test('Allows people to select text without opening log details', async () => {
+    const spy = jest.spyOn(document, 'getSelection');
+    spy.mockReturnValue({
+      toString: () => 'selected log line',
+      removeAllRanges: () => {},
+      addRange: (range: Range) => {},
+    } as Selection);
+
+    render(<LogList {...defaultProps} enableLogDetails={true} />);
+
+    await userEvent.click(screen.getByText('log message 1'));
+
+    expect(screen.queryByText('name_of_the_label')).not.toBeInTheDocument();
+    expect(screen.queryByText('value of the label')).not.toBeInTheDocument();
+    expect(screen.queryByText('Fields')).not.toBeInTheDocument();
+    expect(screen.queryByText('Close log details')).not.toBeInTheDocument();
+
+    spy.mockRestore();
+  });
 });
@@ -69,6 +69,7 @@ export interface Props {
  onPinLine?: (row: LogRowModel) => void;
  onOpenContext?: (row: LogRowModel, onClose: () => void) => void;
  onUnpinLine?: (row: LogRowModel) => void;
+  permalinkedLogId?: string;
  pinLineButtonTooltipTitle?: PopoverContent;
  pinnedLogs?: string[];
  showControls: boolean;
@@ -88,6 +89,7 @@ type LogListComponentProps = Omit<
  | 'dedupStrategy'
  | 'displayedFields'
  | 'enableLogDetails'
+  | 'permalinkedLogId'
  | 'showTime'
  | 'sortOrder'
  | 'syntaxHighlighting'
@@ -125,6 +127,7 @@ export const LogList = ({
  onPinLine,
  onOpenContext,
  onUnpinLine,
+  permalinkedLogId,
  pinLineButtonTooltipTitle,
  pinnedLogs,
  showControls,
@@ -161,6 +164,7 @@ export const LogList = ({
      onPinLine={onPinLine}
      onOpenContext={onOpenContext}
      onUnpinLine={onUnpinLine}
+      permalinkedLogId={permalinkedLogId}
      pinLineButtonTooltipTitle={pinLineButtonTooltipTitle}
      pinnedLogs={pinnedLogs}
      showControls={showControls}
@@ -205,6 +209,7 @@ const LogListComponent = ({
    dedupStrategy,
    filterLevels,
    forceEscape,
+    permalinkedLogId,
    showDetails,
    showTime,
    sortOrder,
@@ -213,7 +218,9 @@ const LogListComponent = ({
  } = useLogListContext();
  const [processedLogs, setProcessedLogs] = useState<LogListModel[]>([]);
  const [listHeight, setListHeight] = useState(
-    app === CoreApp.Explore ? window.innerHeight * 0.75 : containerElement.clientHeight
+    app === CoreApp.Explore
+      ? Math.max(window.innerHeight * 0.8, containerElement.clientHeight)
+      : containerElement.clientHeight
  );
  const theme = useTheme2();
  const listRef = useRef<VariableSizeList | null>(null);
@@ -262,7 +269,11 @@ const LogListComponent = ({

  useEffect(() => {
    const handleResize = debounce(() => {
-      setListHeight(app === CoreApp.Explore ? window.innerHeight * 0.75 : containerElement.clientHeight);
+      setListHeight(
+        app === CoreApp.Explore
+          ? Math.max(window.innerHeight * 0.8, containerElement.clientHeight)
+          : containerElement.clientHeight
+      );
    }, 50);
    window.addEventListener('resize', handleResize);
    handleResize();
@@ -292,8 +303,15 @@ const LogListComponent = ({
  );

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

  if (!containerElement || listHeight == null) {
    // Wait for container to be rendered
@@ -302,6 +320,10 @@ const LogListComponent = ({

  const handleLogLineClick = useCallback(
    (log: LogListModel) => {
+      // Let people select text
+      if (document.getSelection()?.toString()) {
+        return;
+      }
      toggleDetails(log);
    },
    [toggleDetails]
@@ -3,13 +3,20 @@ import { ReactNode } from 'react';

 import { createLogLine } from '../__mocks__/logRow';

-import { useLogListContextData, useLogListContext, useLogIsPinned, LogListContext } from './LogListContext';
+import {
+  useLogListContextData,
+  useLogListContext,
+  useLogIsPinned,
+  LogListContext,
+  useLogIsPermalinked,
+} from './LogListContext';
 import { defaultValue } from './__mocks__/LogListContext';

-const log = createLogLine({ rowId: 'yep' });
+const log = createLogLine({ rowId: 'yep', uid: 'uid' });
 const value = {
  ...defaultValue,
  pinnedLogs: ['yep'],
+  permalinkedLogId: log.uid,
 };
 const wrapper = ({ children }: { children: ReactNode }) => (
  <LogListContext.Provider value={value}>{children}</LogListContext.Provider>
@@ -33,9 +40,22 @@ test('Allows to tell if a log is pinned', () => {
  expect(result.current).toBe(true);
 });

-test('Allows to tell if a log is pinned', () => {
+test('Allows to tell if a log is not pinned', () => {
  const otherLog = createLogLine({ rowId: 'nope' });
  const { result } = renderHook(() => useLogIsPinned(otherLog), { wrapper });

  expect(result.current).toBe(false);
 });
+
+test('Allows to tell if a log is permalinked', () => {
+  const { result } = renderHook(() => useLogIsPermalinked(log), { wrapper });
+
+  expect(result.current).toBe(true);
+});
+
+test('Allows to tell if a log is not permalinked', () => {
+  const otherLog = createLogLine({ rowId: 'nope' });
+  const { result } = renderHook(() => useLogIsPermalinked(otherLog), { wrapper });
+
+  expect(result.current).toBe(false);
+});
@@ -91,11 +91,16 @@ export const useLogListContext = (): LogListContextData => {
  return useContext(LogListContext);
 };

-export const useLogIsPinned = (log: LogRowModel) => {
+export const useLogIsPinned = (log: LogListModel) => {
  const { pinnedLogs } = useContext(LogListContext);
  return pinnedLogs?.some((logId) => logId === log.rowId);
 };

+export const useLogIsPermalinked = (log: LogListModel) => {
+  const { permalinkedLogId } = useContext(LogListContext);
+  return permalinkedLogId && permalinkedLogId === log.uid;
+};
+
 export type LogListState = Pick<
  LogListContextData,
  | 'dedupStrategy'
@@ -139,6 +144,7 @@ export interface Props {
  onPinLine?: (row: LogRowModel) => void;
  onOpenContext?: (row: LogRowModel, onClose: () => void) => void;
  onUnpinLine?: (row: LogRowModel) => void;
+  permalinkedLogId?: string;
  pinLineButtonTooltipTitle?: PopoverContent;
  pinnedLogs?: string[];
  prettifyJSON?: boolean;
@@ -178,6 +184,7 @@ export const LogListContextProvider = ({
  onPinLine,
  onOpenContext,
  onUnpinLine,
+  permalinkedLogId,
  pinLineButtonTooltipTitle,
  pinnedLogs,
  prettifyJSON,
@@ -217,9 +224,6 @@ export const LogListContextProvider = ({
      syntaxHighlighting,
      wrapLogMessage,
    };
-    if (!shallowCompare(logListState.pinnedLogs ?? [], pinnedLogs ?? [])) {
-      newState.pinnedLogs = pinnedLogs;
-    }
    if (!shallowCompare(logListState, newState)) {
      setLogListState(newState);
    }
@@ -250,6 +254,12 @@ export const LogListContextProvider = ({
    }
  }, [hasUnescapedContent, logListState]);

+  useEffect(() => {
+    if (!shallowCompare(logListState.pinnedLogs ?? [], pinnedLogs ?? [])) {
+      setLogListState({ ...logListState, pinnedLogs });
+    }
+  }, [logListState, pinnedLogs]);
+
  const detailsDisplayed = useCallback(
    (log: LogListModel) => !!showDetails.find((shownLog) => shownLog.uid === log.uid),
    [showDetails]
@@ -426,6 +436,7 @@ export const LogListContextProvider = ({
        onPinLine,
        onOpenContext,
        onUnpinLine,
+        permalinkedLogId,
        pinLineButtonTooltipTitle,
        pinnedLogs: logListState.pinnedLogs,
        prettifyJSON: logListState.prettifyJSON,
@@ -1,8 +1,9 @@
 import { createContext, useContext } from 'react';

-import { CoreApp, LogRowModel, LogsDedupStrategy, LogsSortOrder } from '@grafana/data';
+import { CoreApp, LogsDedupStrategy, LogsSortOrder } from '@grafana/data';

 import { LogListContextData, Props } from '../LogListContext';
+import { LogListModel } from '../processing';

 export const LogListContext = createContext<LogListContextData>({
  app: CoreApp.Unknown,
@@ -44,11 +45,16 @@ export const useLogListContext = (): LogListContextData => {
  return useContext(LogListContext);
 };

-export const useLogIsPinned = (log: LogRowModel) => {
+export const useLogIsPinned = (log: LogListModel) => {
  const { pinnedLogs } = useContext(LogListContext);
  return pinnedLogs?.some((logId) => logId === log.rowId);
 };

+export const useLogIsPermalinked = (log: LogListModel) => {
+  const { permalinkedLogId } = useContext(LogListContext);
+  return permalinkedLogId && permalinkedLogId === log.uid;
+};
+
 export const defaultValue: LogListContextData = {
  setDedupStrategy: jest.fn(),
  setFilterLevels: jest.fn(),
@@ -114,6 +120,7 @@ export const LogListContextProvider = ({
  onPinLine = jest.fn(),
  onOpenContext = jest.fn(),
  onUnpinLine = jest.fn(),
+  permalinkedLogId,
  pinnedLogs = [],
  showTime = true,
  sortOrder = LogsSortOrder.Descending,
@@ -137,6 +144,7 @@ export const LogListContextProvider = ({
        onPinLine,
        onOpenContext,
        onUnpinLine,
+        permalinkedLogId,
        pinnedLogs,
        setDedupStrategy: jest.fn(),
        setFilterLevels: jest.fn(),