From 2d370f398356723565b56e7ec0d3137d81d1a67b Mon Sep 17 00:00:00 2001 From: Haris Rozajac <58232930+harisrozajac@users.noreply.github.com> Date: Tue, 11 Jun 2024 11:15:36 -0600 Subject: [PATCH] Explore: Logs pinning in content outline (#88316) * wip: working version * add delete buttons, put pinned logs on top, * Use already available onPinLine prop * cleanup * Fix alignment of pinned log * Limit to 3 pinned log lines * Format tooltip message * Lower to font size and adjust padding so pinned log title is fully visible * Add internationalization support in Explore Logs * Update json for i18n * Test remove button * Open content outline after pinning a log * Remove console.log statements * Minor changes * Conflict stuff --- .betterer.results | 4 +- .../ContentOutline/ContentOutline.test.tsx | 17 ++++- .../explore/ContentOutline/ContentOutline.tsx | 19 ++++-- .../ContentOutline/ContentOutlineContext.tsx | 45 +++++++++++-- .../ContentOutline/ContentOutlineItem.tsx | 7 ++ .../ContentOutlineItemButton.tsx | 30 +++++++-- public/app/features/explore/Explore.tsx | 3 + public/app/features/explore/Logs/Logs.tsx | 65 ++++++++++++++++++- .../features/explore/Logs/LogsContainer.tsx | 3 + .../app/features/logs/components/LogRow.tsx | 4 +- .../logs/components/LogRowMenuCell.tsx | 7 +- .../logs/components/LogRowMessage.tsx | 4 ++ .../app/features/logs/components/LogRows.tsx | 5 +- public/app/plugins/panel/logs/panelcfg.cue | 18 ++--- public/locales/en-US/grafana.json | 6 ++ public/locales/pseudo-LOCALE/grafana.json | 6 ++ 16 files changed, 207 insertions(+), 36 deletions(-) diff --git a/.betterer.results b/.betterer.results index 9d4fbfb1720..fda1df4c2d5 100644 --- a/.betterer.results +++ b/.betterer.results @@ -3891,9 +3891,7 @@ exports[`better eslint`] = { ], "public/app/features/explore/Logs/Logs.tsx:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "1"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "2"], - [0, 0, 0, "No untranslated strings. Wrap text with ", "3"] + [0, 0, 0, "No untranslated strings. Wrap text with ", "1"] ], "public/app/features/explore/Logs/LogsFeedback.tsx:5381": [ [0, 0, 0, "No untranslated strings. Wrap text with ", "0"] diff --git a/public/app/features/explore/ContentOutline/ContentOutline.test.tsx b/public/app/features/explore/ContentOutline/ContentOutline.test.tsx index d28093e331a..b79c823859c 100644 --- a/public/app/features/explore/ContentOutline/ContentOutline.test.tsx +++ b/public/app/features/explore/ContentOutline/ContentOutline.test.tsx @@ -11,6 +11,8 @@ jest.mock('./ContentOutlineContext', () => ({ const scrollIntoViewMock = jest.fn(); const scrollerMock = document.createElement('div'); +const unregisterMock = jest.fn(); + const setup = (mergeSingleChild = false) => { HTMLElement.prototype.scrollIntoView = scrollIntoViewMock; @@ -50,6 +52,7 @@ const setup = (mergeSingleChild = false) => { title: 'Item 2-1', ref: document.createElement('div'), level: 'child', + onRemove: () => unregisterMock('item-2-1'), }, { id: 'item-2-2', @@ -62,7 +65,7 @@ const setup = (mergeSingleChild = false) => { }, ], register: jest.fn(), - unregister: jest.fn(), + unregister: unregisterMock, }); return render(); @@ -143,4 +146,16 @@ describe('', () => { await userEvent.click(button); expect(button.getAttribute('aria-controls')).toBe(sectionContent.id); }); + + it('deletes item on delete button click', async () => { + setup(); + const expandSectionChevrons = screen.getAllByRole('button', { name: 'Content outline item collapse button' }); + // chevron for the second item + const button = expandSectionChevrons[1]; + await userEvent.click(button); + const deleteButtons = screen.getAllByTestId('content-outline-item-delete-button'); + await userEvent.click(deleteButtons[0]); + + expect(unregisterMock).toHaveBeenCalledWith('item-2-1'); + }); }); diff --git a/public/app/features/explore/ContentOutline/ContentOutline.tsx b/public/app/features/explore/ContentOutline/ContentOutline.tsx index bcf3bb147da..05d5adda510 100644 --- a/public/app/features/explore/ContentOutline/ContentOutline.tsx +++ b/public/app/features/explore/ContentOutline/ContentOutline.tsx @@ -47,9 +47,11 @@ export function ContentOutline({ scroller, panelId }: { scroller: HTMLElement | (item) => item.children && !(item.mergeSingleChild && item.children?.length === 1) && item.children.length > 0 ); + const outlineItemsHaveDeleteButton = outlineItems.some((item) => item.children?.some((child) => child.onRemove)); + const [sectionsExpanded, setSectionsExpanded] = useState(() => { return outlineItems.reduce((acc: { [key: string]: boolean }, item) => { - acc[item.id] = false; + acc[item.id] = !!item.expanded; return acc; }, {}); }); @@ -58,6 +60,10 @@ export function ContentOutline({ scroller, panelId }: { scroller: HTMLElement | let scrollValue = 0; let el: HTMLElement | null | undefined = ref; + if (!el) { + return; + } + do { scrollValue += el?.offsetTop || 0; el = el?.offsetParent instanceof HTMLElement ? el.offsetParent : undefined; @@ -158,7 +164,7 @@ export function ContentOutline({ scroller, panelId }: { scroller: HTMLElement | title={contentOutlineExpanded ? item.title : undefined} contentOutlineExpanded={contentOutlineExpanded} className={cx(styles.buttonStyles, { - [styles.justifyCenter]: !contentOutlineExpanded, + [styles.justifyCenter]: !contentOutlineExpanded && !outlineItemsHaveDeleteButton, [styles.sectionHighlighter]: isChildActive(item, activeSectionChildId) && !contentOutlineExpanded, })} indentStyle={cx({ @@ -196,7 +202,7 @@ export function ContentOutline({ scroller, panelId }: { scroller: HTMLElement | contentOutlineExpanded={contentOutlineExpanded} icon={contentOutlineExpanded ? undefined : item.icon} className={cx(styles.buttonStyles, { - [styles.justifyCenter]: !contentOutlineExpanded, + [styles.justifyCenter]: !contentOutlineExpanded && !outlineItemsHaveDeleteButton, [styles.sectionHighlighter]: isChildActive(item, activeSectionChildId) && !contentOutlineExpanded, })} @@ -211,6 +217,7 @@ export function ContentOutline({ scroller, panelId }: { scroller: HTMLElement | isActive={shouldBeActive(child, activeSectionId, activeSectionChildId, sectionsExpanded)} extraHighlight={child.highlight} color={child.color} + onRemove={child.onRemove ? () => child.onRemove?.(child.id) : undefined} /> ))} @@ -257,10 +264,10 @@ const getStyles = (theme: GrafanaTheme2, expanded: boolean) => { marginRight: expanded ? theme.spacing(0.5) : undefined, }), indentRoot: css({ - paddingLeft: theme.spacing(4), + paddingLeft: theme.spacing(3), }), indentChild: css({ - paddingLeft: expanded ? theme.spacing(7) : theme.spacing(4), + paddingLeft: expanded ? theme.spacing(5) : theme.spacing(2.75), }), itemWrapper: css({ display: 'flex', @@ -275,7 +282,7 @@ const getStyles = (theme: GrafanaTheme2, expanded: boolean) => { borderRight: `1px solid ${theme.colors.border.medium}`, content: '""', height: '100%', - left: 48, + left: theme.spacing(4.75), position: 'absolute', transform: 'translateX(50%)', }, diff --git a/public/app/features/explore/ContentOutline/ContentOutlineContext.tsx b/public/app/features/explore/ContentOutline/ContentOutlineContext.tsx index dc579cc8404..44b2a989a60 100644 --- a/public/app/features/explore/ContentOutline/ContentOutlineContext.tsx +++ b/public/app/features/explore/ContentOutline/ContentOutlineContext.tsx @@ -18,6 +18,7 @@ export interface ContentOutlineContextProps { unregister: (id: string) => void; unregisterAllChildren: (parentId: string, childType: ITEM_TYPES) => void; updateOutlineItems: (newItems: ContentOutlineItemContextProps[]) => void; + updateItem: (id: string, properties: Partial>) => void; } interface ContentOutlineContextProviderProps { @@ -141,8 +142,11 @@ export function ContentOutlineContextProvider({ children, refreshDependencies }: ref = parent.ref; } - const childrenUpdated = [...(parent.children || []), { ...outlineItem, id, ref }]; - childrenUpdated.sort(sortElementsByDocumentPosition); + let childrenUpdated = [{ ...outlineItem, id, ref }, ...(parent.children || [])]; + + if (!outlineItem.childOnTop) { + childrenUpdated = sortItems(childrenUpdated); + } newItems[parentIndex] = { ...parent, @@ -175,6 +179,20 @@ export function ContentOutlineContextProvider({ children, refreshDependencies }: setOutlineItems(newItems); }, []); + const updateItem = useCallback((id: string, properties: Partial>) => { + setOutlineItems((prevItems) => + prevItems.map((item) => { + if (item.id === id) { + return { + ...item, + ...properties, + }; + } + return item; + }) + ); + }, []); + const unregisterAllChildren = useCallback((parentId: string, childType: ITEM_TYPES) => { setOutlineItems((prevItems) => prevItems.map((item) => { @@ -190,7 +208,8 @@ export function ContentOutlineContextProvider({ children, refreshDependencies }: setOutlineItems((prevItems) => { const newItems = [...prevItems]; for (const item of newItems) { - item.children?.sort(sortElementsByDocumentPosition); + const sortedItems = sortItems(item.children || []); + item.children = sortedItems; } return newItems; }); @@ -198,14 +217,14 @@ export function ContentOutlineContextProvider({ children, refreshDependencies }: return ( {children} ); } -export function sortElementsByDocumentPosition(a: ContentOutlineItemContextProps, b: ContentOutlineItemContextProps) { +function sortElementsByDocumentPosition(a: ContentOutlineItemContextProps, b: ContentOutlineItemContextProps) { if (a.ref && b.ref) { const diff = a.ref.compareDocumentPosition(b.ref); if (diff === Node.DOCUMENT_POSITION_PRECEDING) { @@ -217,6 +236,22 @@ export function sortElementsByDocumentPosition(a: ContentOutlineItemContextProps return 0; } +function sortItems(outlineItems: ContentOutlineItemContextProps[]): ContentOutlineItemContextProps[] { + const [skipSort, sortable] = outlineItems.reduce< + [ContentOutlineItemContextProps[], ContentOutlineItemContextProps[]] + >( + (acc, item) => { + item.childOnTop ? acc[0].push(item) : acc[1].push(item); + return acc; + }, + [[], []] + ); + + sortable.sort(sortElementsByDocumentPosition); + + return [...skipSort, ...sortable]; +} + export function useContentOutlineContext() { return useContext(ContentOutlineContext); } diff --git a/public/app/features/explore/ContentOutline/ContentOutlineItem.tsx b/public/app/features/explore/ContentOutline/ContentOutlineItem.tsx index 4b0abe9981e..6db6252ed2a 100644 --- a/public/app/features/explore/ContentOutline/ContentOutlineItem.tsx +++ b/public/app/features/explore/ContentOutline/ContentOutlineItem.tsx @@ -35,6 +35,13 @@ export interface ContentOutlineItemBaseProps { * Client can additionally mark filter actions as highlighted */ highlight?: boolean; + onRemove?: (id: string) => void; + /** + * Child that will always be on top of the list + * e.g. pinned log in Logs section + */ + childOnTop?: boolean; + expanded?: boolean; } interface ContentOutlineItemProps extends ContentOutlineItemBaseProps { diff --git a/public/app/features/explore/ContentOutline/ContentOutlineItemButton.tsx b/public/app/features/explore/ContentOutline/ContentOutlineItemButton.tsx index 8afd4c2ea17..4b45e2efa79 100644 --- a/public/app/features/explore/ContentOutline/ContentOutlineItemButton.tsx +++ b/public/app/features/explore/ContentOutline/ContentOutlineItemButton.tsx @@ -2,7 +2,7 @@ import { cx, css } from '@emotion/css'; import React, { ButtonHTMLAttributes, useEffect, useRef, useState } from 'react'; import { IconName, isIconName, GrafanaTheme2 } from '@grafana/data'; -import { Icon, Tooltip, useTheme2 } from '@grafana/ui'; +import { Button, Icon, Tooltip, useTheme2 } from '@grafana/ui'; import { TooltipPlacement } from '@grafana/ui/src/components/Tooltip'; type CommonProps = { @@ -20,6 +20,7 @@ type CommonProps = { sectionId?: string; toggleCollapsed?: () => void; color?: string; + onRemove?: () => void; }; export type ContentOutlineItemButtonProps = CommonProps & ButtonHTMLAttributes; @@ -39,6 +40,7 @@ export function ContentOutlineItemButton({ sectionId, toggleCollapsed, color, + onRemove, ...rest }: ContentOutlineItemButtonProps) { const theme = useTheme2(); @@ -83,6 +85,15 @@ export function ContentOutlineItemButton({ )} + {onRemove && ( + @@ -964,7 +1023,7 @@ class UnthemedLogs extends PureComponent {
{scanText}
diff --git a/public/app/features/explore/Logs/LogsContainer.tsx b/public/app/features/explore/Logs/LogsContainer.tsx index 528daafa093..aef5c4792e8 100644 --- a/public/app/features/explore/Logs/LogsContainer.tsx +++ b/public/app/features/explore/Logs/LogsContainer.tsx @@ -60,6 +60,7 @@ interface LogsContainerProps extends PropsFromRedux { isFilterLabelActive: (key: string, value: string, refId?: string) => Promise; onClickFilterString: (value: string, refId?: string) => void; onClickFilterOutString: (value: string, refId?: string) => void; + onPinLineCallback?: () => void; } type DataSourceInstance = @@ -282,6 +283,7 @@ class LogsContainer extends PureComponent diff --git a/public/app/features/logs/components/LogRow.tsx b/public/app/features/logs/components/LogRow.tsx index 163b53f2bec..fe161e3111e 100644 --- a/public/app/features/logs/components/LogRow.tsx +++ b/public/app/features/logs/components/LogRow.tsx @@ -15,7 +15,7 @@ import { } from '@grafana/data'; import { reportInteraction } from '@grafana/runtime'; import { DataQuery, TimeZone } from '@grafana/schema'; -import { withTheme2, Themeable2, Icon, Tooltip } from '@grafana/ui'; +import { withTheme2, Themeable2, Icon, Tooltip, PopoverContent } from '@grafana/ui'; import { checkLogsError, escapeUnescapedString } from '../utils'; @@ -60,6 +60,7 @@ interface Props extends Themeable2 { isFilterLabelActive?: (key: string, value: string, refId?: string) => Promise; onPinLine?: (row: LogRowModel) => void; onUnpinLine?: (row: LogRowModel) => void; + pinLineButtonTooltipTitle?: PopoverContent; pinned?: boolean; containerRendered?: boolean; handleTextSelection?: (e: MouseEvent, row: LogRowModel) => boolean; @@ -305,6 +306,7 @@ class UnThemedLogRow extends PureComponent { styles={styles} onPinLine={this.props.onPinLine} onUnpinLine={this.props.onUnpinLine} + pinLineButtonTooltipTitle={this.props.pinLineButtonTooltipTitle} pinned={this.props.pinned} mouseIsOver={this.state.mouseIsOver} onBlur={this.onMouseLeave} diff --git a/public/app/features/logs/components/LogRowMenuCell.tsx b/public/app/features/logs/components/LogRowMenuCell.tsx index 9313a8e8256..756e19813f7 100644 --- a/public/app/features/logs/components/LogRowMenuCell.tsx +++ b/public/app/features/logs/components/LogRowMenuCell.tsx @@ -2,7 +2,7 @@ import React, { FocusEvent, SyntheticEvent, useCallback } from 'react'; import { LogRowContextOptions, LogRowModel, getDefaultTimeRange, locationUtil, urlUtil } from '@grafana/data'; import { DataQuery } from '@grafana/schema'; -import { ClipboardButton, IconButton } from '@grafana/ui'; +import { ClipboardButton, IconButton, PopoverContent } from '@grafana/ui'; import { getConfig } from 'app/core/config'; import { LogRowStyles } from './getLogRowStyles'; @@ -20,10 +20,12 @@ interface Props { onPermalinkClick?: (row: LogRowModel) => Promise; onPinLine?: (row: LogRowModel) => void; onUnpinLine?: (row: LogRowModel) => void; + pinLineButtonTooltipTitle?: PopoverContent; pinned?: boolean; styles: LogRowStyles; mouseIsOver: boolean; onBlur: () => void; + onPinToContentOutlineClick?: (row: LogRowModel, onOpenContext: (row: LogRowModel) => void) => void; } export const LogRowMenuCell = React.memo( @@ -33,6 +35,7 @@ export const LogRowMenuCell = React.memo( onPermalinkClick, onPinLine, onUnpinLine, + pinLineButtonTooltipTitle, pinned, row, showContextToggle, @@ -145,7 +148,7 @@ export const LogRowMenuCell = React.memo( size="md" name="gf-pin" onClick={() => onPinLine && onPinLine(row)} - tooltip="Pin line" + tooltip={pinLineButtonTooltipTitle ?? 'Pin line'} tooltipPlacement="top" aria-label="Pin line" tabIndex={0} diff --git a/public/app/features/logs/components/LogRowMessage.tsx b/public/app/features/logs/components/LogRowMessage.tsx index 841923d3f17..5d99484c1b2 100644 --- a/public/app/features/logs/components/LogRowMessage.tsx +++ b/public/app/features/logs/components/LogRowMessage.tsx @@ -3,6 +3,7 @@ import Highlighter from 'react-highlight-words'; import { CoreApp, findHighlightChunksInText, LogRowContextOptions, LogRowModel } from '@grafana/data'; import { DataQuery } from '@grafana/schema'; +import { PopoverContent } from '@grafana/ui'; import { LogMessageAnsi } from './LogMessageAnsi'; import { LogRowMenuCell } from './LogRowMenuCell'; @@ -25,6 +26,7 @@ interface Props { onPermalinkClick?: (row: LogRowModel) => Promise; onPinLine?: (row: LogRowModel) => void; onUnpinLine?: (row: LogRowModel) => void; + pinLineButtonTooltipTitle?: PopoverContent; pinned?: boolean; styles: LogRowStyles; mouseIsOver: boolean; @@ -88,6 +90,7 @@ export const LogRowMessage = React.memo((props: Props) => { onPermalinkClick, onUnpinLine, onPinLine, + pinLineButtonTooltipTitle, pinned, mouseIsOver, onBlur, @@ -124,6 +127,7 @@ export const LogRowMessage = React.memo((props: Props) => { onPermalinkClick={onPermalinkClick} onPinLine={onPinLine} onUnpinLine={onUnpinLine} + pinLineButtonTooltipTitle={pinLineButtonTooltipTitle} pinned={pinned} styles={styles} mouseIsOver={mouseIsOver} diff --git a/public/app/features/logs/components/LogRows.tsx b/public/app/features/logs/components/LogRows.tsx index 400266137f4..41100a4c634 100644 --- a/public/app/features/logs/components/LogRows.tsx +++ b/public/app/features/logs/components/LogRows.tsx @@ -15,7 +15,7 @@ import { } from '@grafana/data'; import { config } from '@grafana/runtime'; import { DataQuery } from '@grafana/schema'; -import { withTheme2, Themeable2 } from '@grafana/ui'; +import { withTheme2, Themeable2, PopoverContent } from '@grafana/ui'; import { PopoverMenu } from '../../explore/Logs/PopoverMenu'; import { UniqueKeyMaker } from '../UniqueKeyMaker'; @@ -50,6 +50,7 @@ export interface Props extends Themeable2 { onClickHideField?: (key: string) => void; onPinLine?: (row: LogRowModel) => void; onUnpinLine?: (row: LogRowModel) => void; + pinLineButtonTooltipTitle?: PopoverContent; onLogRowHover?: (row?: LogRowModel) => void; onOpenContext?: (row: LogRowModel, onClose: () => void) => void; getRowContextQuery?: ( @@ -238,6 +239,7 @@ class UnThemedLogRows extends PureComponent { permalinkedRowId={this.props.permalinkedRowId} onPinLine={this.props.onPinLine} onUnpinLine={this.props.onUnpinLine} + pinLineButtonTooltipTitle={this.props.pinLineButtonTooltipTitle} pinned={this.props.pinnedRowId === row.uid} isFilterLabelActive={this.props.isFilterLabelActive} handleTextSelection={this.popoverMenuSupported() ? this.handleSelection : undefined} @@ -260,6 +262,7 @@ class UnThemedLogRows extends PureComponent { permalinkedRowId={this.props.permalinkedRowId} onPinLine={this.props.onPinLine} onUnpinLine={this.props.onUnpinLine} + pinLineButtonTooltipTitle={this.props.pinLineButtonTooltipTitle} pinned={this.props.pinnedRowId === row.uid} isFilterLabelActive={this.props.isFilterLabelActive} handleTextSelection={this.popoverMenuSupported() ? this.handleSelection : undefined} diff --git a/public/app/plugins/panel/logs/panelcfg.cue b/public/app/plugins/panel/logs/panelcfg.cue index 481fb99efe6..239de796ceb 100644 --- a/public/app/plugins/panel/logs/panelcfg.cue +++ b/public/app/plugins/panel/logs/panelcfg.cue @@ -26,15 +26,15 @@ composableKinds: PanelCfg: { version: [0, 0] schema: { Options: { - showLabels: bool - showCommonLabels: bool - showTime: bool - showLogContextToggle: bool - wrapLogMessage: bool - prettifyLogMessage: bool - enableLogDetails: bool - sortOrder: common.LogsSortOrder - dedupStrategy: common.LogsDedupStrategy + showLabels: bool + showCommonLabels: bool + showTime: bool + showLogContextToggle: bool + wrapLogMessage: bool + prettifyLogMessage: bool + enableLogDetails: bool + sortOrder: common.LogsSortOrder + dedupStrategy: common.LogsDedupStrategy // TODO: figure out how to define callbacks onClickFilterLabel?: _ onClickFilterOutLabel?: _ diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index dcacf1d1098..66793ded596 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -497,6 +497,12 @@ "title": "Add query to Query Library", "visibility": "Visibility" }, + "logs": { + "maximum-pinned-logs": "Maximum of {{PINNED_LOGS_LIMIT}} pinned logs reached. Unpin a log to add another.", + "no-logs-found": "No logs found.", + "scan-for-older-logs": "Scan for older logs", + "stop-scan": "Stop scan" + }, "rich-history": { "close-tooltip": "Close query history", "datasource-a-z": "Data source A-Z", diff --git a/public/locales/pseudo-LOCALE/grafana.json b/public/locales/pseudo-LOCALE/grafana.json index 2bb814bc727..f0455ab7fb1 100644 --- a/public/locales/pseudo-LOCALE/grafana.json +++ b/public/locales/pseudo-LOCALE/grafana.json @@ -497,6 +497,12 @@ "title": "Åđđ qūęřy ŧő Qūęřy Ŀįþřäřy", "visibility": "Vįşįþįľįŧy" }, + "logs": { + "maximum-pinned-logs": "Mäχįmūm őƒ {{PINNED_LOGS_LIMIT}} pįʼnʼnęđ ľőģş řęäčĥęđ. Ůʼnpįʼn ä ľőģ ŧő äđđ äʼnőŧĥęř.", + "no-logs-found": "Ńő ľőģş ƒőūʼnđ.", + "scan-for-older-logs": "Ŝčäʼn ƒőř őľđęř ľőģş", + "stop-scan": "Ŝŧőp şčäʼn" + }, "rich-history": { "close-tooltip": "Cľőşę qūęřy ĥįşŧőřy", "datasource-a-z": "Đäŧä şőūřčę Å-Ż",