Logs: Added support to disable and re-enable the popover menu (#98254)

* PopoverMenu: add disable menu item

* PopoverMenu: disable and reenable with shift

* Style confirmation dialog

* Update unit tests and add tracking

* PopoverMenu: i18n

* Translations

* Update tracking property

* Use alt+select to re-enable

* Update betterer
This commit is contained in:
Matias Chomicki 2025-01-07 15:17:06 +00:00 committed by GitHub
parent df80bdeb14
commit 44e65c5bc9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 205 additions and 40 deletions

View File

@ -4335,11 +4335,6 @@ exports[`better eslint`] = {
[0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "5"],
[0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "6"]
],
"public/app/features/explore/Logs/PopoverMenu.tsx:5381": [
[0, 0, 0, "No untranslated strings in text props. Wrap text with <Trans /> or use t()", "0"],
[0, 0, 0, "No untranslated strings in text props. Wrap text with <Trans /> or use t()", "1"],
[0, 0, 0, "No untranslated strings in text props. Wrap text with <Trans /> or use t()", "2"]
],
"public/app/features/explore/MetaInfoText.tsx:5381": [
[0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "0"]
],

View File

@ -8,7 +8,7 @@ import { PopoverMenu } from './PopoverMenu';
const row = createLogRow();
test('Does not render if the filter functions are not defined', () => {
render(<PopoverMenu selection="test" x={0} y={0} row={row} close={() => {}} />);
render(<PopoverMenu selection="test" x={0} y={0} row={row} close={() => {}} onDisable={() => {}} />);
expect(screen.queryByText('Copy selection')).not.toBeInTheDocument();
});
@ -16,7 +16,15 @@ test('Does not render if the filter functions are not defined', () => {
test('Renders copy and line contains filter', async () => {
const onClickFilterString = jest.fn();
render(
<PopoverMenu selection="test" x={0} y={0} row={row} close={() => {}} onClickFilterString={onClickFilterString} />
<PopoverMenu
selection="test"
x={0}
y={0}
row={row}
close={() => {}}
onDisable={() => {}}
onClickFilterString={onClickFilterString}
/>
);
expect(screen.getByText('Copy selection')).toBeInTheDocument();
@ -37,6 +45,7 @@ test('Renders copy and line does not contain filter', async () => {
y={0}
row={row}
close={() => {}}
onDisable={() => {}}
onClickFilterOutString={onClickFilterOutString}
/>
);
@ -58,6 +67,7 @@ test('Renders copy, line contains filter, and line does not contain filter', ()
y={0}
row={row}
close={() => {}}
onDisable={() => {}}
onClickFilterString={() => {}}
onClickFilterOutString={() => {}}
/>
@ -77,6 +87,7 @@ test('Can be dismissed with escape', async () => {
y={0}
row={row}
close={close}
onDisable={() => {}}
onClickFilterString={() => {}}
onClickFilterOutString={() => {}}
/>
@ -87,3 +98,24 @@ test('Can be dismissed with escape', async () => {
await userEvent.keyboard('{Escape}');
expect(close).toHaveBeenCalledTimes(1);
});
test('Can be disabled', async () => {
const onDisable = jest.fn();
render(
<PopoverMenu
selection="test"
x={0}
y={0}
row={row}
close={() => {}}
onDisable={onDisable}
onClickFilterString={() => {}}
onClickFilterOutString={() => {}}
/>
);
expect(onDisable).not.toHaveBeenCalled();
expect(screen.getByText('Disable menu')).toBeInTheDocument();
await userEvent.click(screen.getByText('Disable menu'));
expect(onDisable).toHaveBeenCalledTimes(1);
});

View File

@ -1,9 +1,10 @@
import { css } from '@emotion/css';
import { useEffect, useRef } from 'react';
import { useCallback, useEffect, useRef } from 'react';
import { GrafanaTheme2, LogRowModel } from '@grafana/data';
import { reportInteraction } from '@grafana/runtime';
import { Menu, useStyles2 } from '@grafana/ui';
import { t } from 'app/core/internationalization';
import { copyText } from '../../logs/utils';
@ -13,6 +14,7 @@ interface PopoverMenuProps {
y: number;
onClickFilterString?: (value: string, refId?: string) => void;
onClickFilterOutString?: (value: string, refId?: string) => void;
onDisable: () => void;
row: LogRowModel;
close: () => void;
}
@ -25,6 +27,7 @@ export const PopoverMenu = ({
selection,
row,
close,
...props
}: PopoverMenuProps) => {
const containerRef = useRef<HTMLDivElement | null>(null);
const styles = useStyles2(getStyles);
@ -42,6 +45,11 @@ export const PopoverMenu = ({
};
}, [close]);
const onDisable = useCallback(() => {
track('popover_menu_disabled', selection.length, row.datasourceType);
props.onDisable();
}, [props, row.datasourceType, selection.length]);
const supported = onClickFilterString || onClickFilterOutString;
if (!supported) {
@ -49,38 +57,42 @@ export const PopoverMenu = ({
}
return (
<div className={styles.menu} style={{ top: y, left: x }}>
<Menu ref={containerRef}>
<Menu.Item
label="Copy selection"
onClick={() => {
copyText(selection, containerRef);
close();
track('copy', selection.length, row.datasourceType);
}}
/>
{onClickFilterString && (
<>
<div className={styles.menu} style={{ top: y, left: x }}>
<Menu ref={containerRef}>
<Menu.Item
label="Add as line contains filter"
label={t('logs.popover-menu.copy', 'Copy selection')}
onClick={() => {
onClickFilterString(selection, row.dataFrame.refId);
copyText(selection, containerRef);
close();
track('line_contains', selection.length, row.datasourceType);
track('copy', selection.length, row.datasourceType);
}}
/>
)}
{onClickFilterOutString && (
<Menu.Item
label="Add as line does not contain filter"
onClick={() => {
onClickFilterOutString(selection, row.dataFrame.refId);
close();
track('line_does_not_contain', selection.length, row.datasourceType);
}}
/>
)}
</Menu>
</div>
{onClickFilterString && (
<Menu.Item
label={t('logs.popover-menu.line-contains', 'Add as line contains filter')}
onClick={() => {
onClickFilterString(selection, row.dataFrame.refId);
close();
track('line_contains', selection.length, row.datasourceType);
}}
/>
)}
{onClickFilterOutString && (
<Menu.Item
label={t('logs.popover-menu.line-contains-not', 'Add as line does not contain filter')}
onClick={() => {
onClickFilterOutString(selection, row.dataFrame.refId);
close();
track('line_does_not_contain', selection.length, row.datasourceType);
}}
/>
)}
<Menu.Divider />
<Menu.Item label={t('logs.popover-menu.disable-menu', 'Disable menu')} onClick={onDisable} />
</Menu>
</div>
</>
);
};

View File

@ -3,9 +3,18 @@ import userEvent from '@testing-library/user-event';
import { LogRowModel, LogsDedupStrategy, LogsSortOrder } from '@grafana/data';
import { disablePopoverMenu, enablePopoverMenu, isPopoverMenuDisabled } from '../utils';
import { LogRows, Props } from './LogRows';
import { createLogRow } from './__mocks__/logRow';
jest.mock('../utils', () => ({
...jest.requireActual('../utils'),
isPopoverMenuDisabled: jest.fn(),
disablePopoverMenu: jest.fn(),
enablePopoverMenu: jest.fn(),
}));
jest.mock('@grafana/runtime', () => ({
...jest.requireActual('@grafana/runtime'),
config: {
@ -155,6 +164,9 @@ describe('Popover menu', () => {
);
}
let orgGetSelection: () => Selection | null;
beforeEach(() => {
jest.mocked(isPopoverMenuDisabled).mockReturnValue(false);
});
beforeAll(() => {
orgGetSelection = document.getSelection;
jest.spyOn(document, 'getSelection').mockReturnValue({
@ -177,6 +189,27 @@ describe('Popover menu', () => {
expect(screen.getByText('Add as line contains filter')).toBeInTheDocument();
expect(screen.getByText('Add as line does not contain filter')).toBeInTheDocument();
});
it('Can be disabled', async () => {
setup();
await userEvent.click(screen.getByText('log message 1'));
await userEvent.click(screen.getByText('Disable menu'));
await userEvent.click(screen.getByText('Confirm'));
expect(disablePopoverMenu).toHaveBeenCalledTimes(1);
});
it('Does not appear when disabled', async () => {
jest.mocked(isPopoverMenuDisabled).mockReturnValue(true);
setup();
await userEvent.click(screen.getByText('log message 1'));
expect(screen.queryByText('Copy selection')).not.toBeInTheDocument();
});
it('Can be re-enabled', async () => {
jest.mocked(isPopoverMenuDisabled).mockReturnValue(true);
const user = userEvent.setup();
setup();
await user.keyboard('[AltLeft>]'); // Press Alt (without releasing it)
await user.click(screen.getByText('log message 1'));
expect(enablePopoverMenu).toHaveBeenCalledTimes(1);
});
it('Does not appear when the props are not defined', async () => {
setup({
onClickFilterOutString: undefined,

View File

@ -14,11 +14,12 @@ import {
} from '@grafana/data';
import { config } from '@grafana/runtime';
import { DataQuery } from '@grafana/schema';
import { PopoverContent, useTheme2 } from '@grafana/ui';
import { ConfirmModal, Icon, PopoverContent, useTheme2 } from '@grafana/ui';
import { t, Trans } from 'app/core/internationalization';
import { PopoverMenu } from '../../explore/Logs/PopoverMenu';
import { UniqueKeyMaker } from '../UniqueKeyMaker';
import { sortLogRows, targetIsElement } from '../utils';
import { disablePopoverMenu, enablePopoverMenu, isPopoverMenuDisabled, sortLogRows, targetIsElement } from '../utils';
//Components
import { LogRow } from './LogRow';
@ -112,6 +113,7 @@ export const LogRows = memo(
selectedRow: null,
popoverMenuCoordinates: { x: 0, y: 0 },
});
const [showDisablePopoverOptions, setShowDisablePopoverOptions] = useState(false);
const logRowsRef = useRef<HTMLDivElement>(null);
const theme = useTheme2();
const styles = getLogRowStyles(theme);
@ -121,7 +123,6 @@ export const LogRows = memo(
[dedupedRows]
);
const showDuplicates = dedupStrategy !== LogsDedupStrategy.none && dedupCount > 0;
// Staged rendering
const orderedRows = useMemo(
() => (logsSortOrder ? sortLogRows(dedupedRows, logsSortOrder) : dedupedRows),
[dedupedRows, logsSortOrder]
@ -130,7 +131,6 @@ export const LogRows = memo(
const getRows = useMemo(() => () => orderedRows, [orderedRows]);
const handleDeselectionRef = useRef<((e: Event) => void) | null>(null);
const keyMaker = new UniqueKeyMaker();
// eslint-disable-next-line react-hooks/exhaustive-deps
useEffect(() => {
return () => {
@ -169,7 +169,7 @@ export const LogRows = memo(
);
const popoverMenuSupported = useCallback(() => {
if (!config.featureToggles.logRowsPopoverMenu) {
if (!config.featureToggles.logRowsPopoverMenu || isPopoverMenuDisabled()) {
return false;
}
return Boolean(onClickFilterOutString || onClickFilterString);
@ -209,6 +209,9 @@ export const LogRows = memo(
if (!selection) {
return false;
}
if (e.altKey) {
enablePopoverMenu();
}
if (popoverMenuSupported() === false) {
// This signals onRowClick inside LogRow to skip the event because the user is selecting text
return selection ? true : false;
@ -236,6 +239,19 @@ export const LogRows = memo(
[handleDeselection, popoverMenuSupported]
);
const onDisablePopoverMenu = useCallback(() => {
setShowDisablePopoverOptions(true);
}, []);
const onDisableCancel = useCallback(() => {
setShowDisablePopoverOptions(false);
}, []);
const onDisableConfirm = useCallback(() => {
disablePopoverMenu();
setShowDisablePopoverOptions(false);
}, []);
return (
<div className={styles.logRows} ref={logRowsRef}>
{popoverState.selection && popoverState.selectedRow && (
@ -246,6 +262,29 @@ export const LogRows = memo(
{...popoverState.popoverMenuCoordinates}
onClickFilterString={onClickFilterString}
onClickFilterOutString={onClickFilterOutString}
onDisable={onDisablePopoverMenu}
/>
)}
{showDisablePopoverOptions && (
<ConfirmModal
isOpen
title={t('logs.log-rows.disable-popover.title', 'Disable menu')}
body={
<>
<Trans i18nKey="logs.log-rows.disable-popover.message">
You are about to disable the logs filter menu. To re-enable it, select text in a log line while
holding the alt key.
</Trans>
<div className={styles.shortcut}>
<Icon name="keyboard" />
<Trans i18nKey="logs.log-rows.disable-popover-message.shortcut">alt+select to enable again</Trans>
</div>
</>
}
confirmText={t('logs.log-rows.disable-popover.confirm', 'Confirm')}
icon="exclamation-triangle"
onConfirm={onDisableConfirm}
onDismiss={onDisableCancel}
/>
)}
<table className={cx(styles.logsRowsTable, props.overflowingContent ? '' : styles.logsRowsTableContain)}>

View File

@ -72,6 +72,15 @@ export const getLogRowStyles = memoizeOne((theme: GrafanaTheme2) => {
logRows: css({
position: 'relative',
}),
shortcut: css({
display: 'inline-flex',
alignItems: 'center',
gap: theme.spacing(1),
color: theme.colors.text.secondary,
opacity: 0.7,
fontSize: theme.typography.bodySmall.fontSize,
marginTop: theme.spacing(1),
}),
logsRowsTable: css({
label: 'logs-rows',
fontFamily: theme.typography.fontFamilyMonospace,

View File

@ -373,3 +373,16 @@ function getDataSourceLabelType(labelType: string, datasourceType: string) {
return null;
}
}
const POPOVER_STORAGE_KEY = 'logs.popover.disabled';
export function disablePopoverMenu() {
localStorage.setItem(POPOVER_STORAGE_KEY, 'true');
}
export function enablePopoverMenu() {
localStorage.removeItem(POPOVER_STORAGE_KEY);
}
export function isPopoverMenuDisabled() {
return Boolean(localStorage.getItem(POPOVER_STORAGE_KEY));
}

View File

@ -1781,6 +1781,22 @@
"log-row-message": {
"ellipsis": "… ",
"more": "more"
},
"log-rows": {
"disable-popover": {
"confirm": "Confirm",
"message": "You are about to disable the logs filter menu. To re-enable it, select text in a log line while holding the alt key.",
"title": "Disable menu"
},
"disable-popover-message": {
"shortcut": "alt+select to enable again"
}
},
"popover-menu": {
"copy": "Copy selection",
"disable-menu": "Disable menu",
"line-contains": "Add as line contains filter",
"line-contains-not": "Add as line does not contain filter"
}
},
"migrate-to-cloud": {

View File

@ -1781,6 +1781,22 @@
"log-row-message": {
"ellipsis": "… ",
"more": "mőřę"
},
"log-rows": {
"disable-popover": {
"confirm": "Cőʼnƒįřm",
"message": "Ÿőū äřę äþőūŧ ŧő đįşäþľę ŧĥę ľőģş ƒįľŧęř męʼnū. Ŧő řę-ęʼnäþľę įŧ, şęľęčŧ ŧęχŧ įʼn ä ľőģ ľįʼnę ŵĥįľę ĥőľđįʼnģ ŧĥę äľŧ ĸęy.",
"title": "Đįşäþľę męʼnū"
},
"disable-popover-message": {
"shortcut": "äľŧ+şęľęčŧ ŧő ęʼnäþľę äģäįʼn"
}
},
"popover-menu": {
"copy": "Cőpy şęľęčŧįőʼn",
"disable-menu": "Đįşäþľę męʼnū",
"line-contains": "Åđđ äş ľįʼnę čőʼnŧäįʼnş ƒįľŧęř",
"line-contains-not": "Åđđ äş ľįʼnę đőęş ʼnőŧ čőʼnŧäįʼn ƒįľŧęř"
}
},
"migrate-to-cloud": {