LogContext: Make centered row unsticky on click (#70832)

* add pinning in log context

* add tests

* move icon names

* fix type of `pinned`
This commit is contained in:
Sven Grossmann 2023-06-28 15:22:54 +02:00 committed by GitHub
parent 4969a78f83
commit 1206cf3dfa
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 190 additions and 9 deletions

View File

@ -154,6 +154,8 @@ export const availableIconsIndex = {
'list-ol': true, 'list-ol': true,
lock: true, lock: true,
'map-marker': true, 'map-marker': true,
'map-marker-plus': true,
'map-marker-minus': true,
message: true, message: true,
minus: true, minus: true,
'minus-circle': true, 'minus-circle': true,

View File

@ -42,6 +42,9 @@ interface Props extends Themeable2 {
styles: LogRowStyles; styles: LogRowStyles;
permalinkedRowId?: string; permalinkedRowId?: string;
scrollIntoView?: (element: HTMLElement) => void; scrollIntoView?: (element: HTMLElement) => void;
onPinLine?: (row: LogRowModel) => void;
onUnpinLine?: (row: LogRowModel) => void;
pinned?: boolean;
} }
interface State { interface State {
@ -236,6 +239,9 @@ class UnThemedLogRow extends PureComponent<Props, State> {
onPermalinkClick={this.props.onPermalinkClick} onPermalinkClick={this.props.onPermalinkClick}
app={app} app={app}
styles={styles} styles={styles}
onPinLine={this.props.onPinLine}
onUnpinLine={this.props.onUnpinLine}
pinned={this.props.pinned}
/> />
)} )}
</tr> </tr>

View File

@ -63,12 +63,12 @@ describe('LogRowMessage', () => {
}); });
describe('with permalinking', () => { describe('with permalinking', () => {
it('should show permalinking button when no `onPermalinkClick` is defined', () => { it('should show permalinking button when `onPermalinkClick` is defined', () => {
setup({ onPermalinkClick: jest.fn() }); setup({ onPermalinkClick: jest.fn() });
expect(screen.queryByLabelText('Copy shortlink')).toBeInTheDocument(); expect(screen.queryByLabelText('Copy shortlink')).toBeInTheDocument();
}); });
it('should not show permalinking button when `onPermalinkClick` is defined', () => { it('should not show permalinking button when `onPermalinkClick` is not defined', () => {
setup(); setup();
expect(screen.queryByLabelText('Copy shortlink')).not.toBeInTheDocument(); expect(screen.queryByLabelText('Copy shortlink')).not.toBeInTheDocument();
}); });
@ -83,4 +83,61 @@ describe('LogRowMessage', () => {
expect(permalinkClick).toHaveBeenCalledWith(props.row); expect(permalinkClick).toHaveBeenCalledWith(props.row);
}); });
}); });
describe('with pinning', () => {
describe('for `onPinLine`', () => {
it('should show pinning button when `onPinLine` is defined', () => {
setup({ onPinLine: jest.fn() });
expect(screen.queryByLabelText('Pin line')).toBeInTheDocument();
});
it('should not show pinning button when `onPinLine` and `pinned` is defined', () => {
setup({ onPinLine: jest.fn(), pinned: true });
expect(screen.queryByLabelText('Pin line')).not.toBeInTheDocument();
});
it('should not show pinning button when `onPinLine` is not defined', () => {
setup();
expect(screen.queryByLabelText('Pin line')).not.toBeInTheDocument();
});
it('should call `onPinLine` on click', async () => {
const onPinLine = jest.fn();
setup({ onPinLine });
const button = screen.getByLabelText('Pin line');
await userEvent.click(button);
expect(onPinLine).toHaveBeenCalledTimes(1);
});
});
describe('for `onUnpinLine`', () => {
it('should not show pinning button when `onUnpinLine` is defined', () => {
setup({ onUnpinLine: jest.fn() });
expect(screen.queryByLabelText('Unpin line')).not.toBeInTheDocument();
});
it('should show 2 pinning buttons when `onUnpinLine` and `pinned` is defined', () => {
// we show 2 because we now have an "always visible" menu, and a "hover" menu
setup({ onUnpinLine: jest.fn(), pinned: true });
expect(screen.queryAllByLabelText('Unpin line').length).toBe(2);
});
it('should not show pinning button when `onUnpinLine` is not defined', () => {
setup();
expect(screen.queryByLabelText('Unpin line')).not.toBeInTheDocument();
});
it('should call `onUnpinLine` on click', async () => {
const onUnpinLine = jest.fn();
setup({ onUnpinLine, pinned: true });
const button = screen.getAllByLabelText('Unpin line')[0];
await userEvent.click(button);
expect(onUnpinLine).toHaveBeenCalledTimes(1);
});
});
});
}); });

View File

@ -19,6 +19,9 @@ interface Props {
showContextToggle?: (row?: LogRowModel) => boolean; showContextToggle?: (row?: LogRowModel) => boolean;
onOpenContext: (row: LogRowModel) => void; onOpenContext: (row: LogRowModel) => void;
onPermalinkClick?: (row: LogRowModel) => Promise<void>; onPermalinkClick?: (row: LogRowModel) => Promise<void>;
onPinLine?: (row: LogRowModel) => void;
onUnpinLine?: (row: LogRowModel) => void;
pinned?: boolean;
styles: LogRowStyles; styles: LogRowStyles;
} }
@ -77,7 +80,17 @@ export class LogRowMessage extends PureComponent<Props> {
}; };
render() { render() {
const { row, wrapLogMessage, prettifyLogMessage, showContextToggle, styles, onPermalinkClick } = this.props; const {
row,
wrapLogMessage,
prettifyLogMessage,
showContextToggle,
styles,
onPermalinkClick,
onUnpinLine,
onPinLine,
pinned,
} = this.props;
const { hasAnsi, raw } = row; const { hasAnsi, raw } = row;
const restructuredEntry = restructureLog(raw, prettifyLogMessage); const restructuredEntry = restructureLog(raw, prettifyLogMessage);
const shouldShowContextToggle = showContextToggle ? showContextToggle(row) : false; const shouldShowContextToggle = showContextToggle ? showContextToggle(row) : false;
@ -101,7 +114,20 @@ export class LogRowMessage extends PureComponent<Props> {
</div> </div>
</td> </td>
<td className={cx('log-row-menu-cell', styles.logRowMenuCell)}> <td className={cx('log-row-menu-cell', styles.logRowMenuCell)}>
<span className={cx('log-row-menu', styles.rowMenu)} onClick={this.onLogRowClick}> {pinned && (
<span className={cx('log-row-menu', 'log-row-menu-visible', styles.rowMenu)} onClick={this.onLogRowClick}>
<IconButton
className={styles.unPinButton}
size="md"
name="map-marker-minus"
onClick={() => onUnpinLine && onUnpinLine(row)}
tooltip="Unpin line"
tooltipPlacement="top"
aria-label="Unpin line"
/>
</span>
)}
<span className={cx('log-row-menu', styles.rowMenu, styles.hidden)} onClick={this.onLogRowClick}>
{shouldShowContextToggle && ( {shouldShowContextToggle && (
<IconButton <IconButton
size="md" size="md"
@ -122,6 +148,28 @@ export class LogRowMessage extends PureComponent<Props> {
tooltip="Copy to clipboard" tooltip="Copy to clipboard"
tooltipPlacement="top" tooltipPlacement="top"
/> />
{pinned && onUnpinLine && (
<IconButton
className={styles.unPinButton}
size="md"
name="map-marker-minus"
onClick={() => onUnpinLine && onUnpinLine(row)}
tooltip="Unpin line"
tooltipPlacement="top"
aria-label="Unpin line"
/>
)}
{!pinned && onPinLine && (
<IconButton
className={styles.unPinButton}
size="md"
name="map-marker-plus"
onClick={() => onPinLine && onPinLine(row)}
tooltip="Pin line"
tooltipPlacement="top"
aria-label="Pin line"
/>
)}
{onPermalinkClick && row.uid && ( {onPermalinkClick && row.uid && (
<IconButton <IconButton
tooltip="Copy shortlink" tooltip="Copy shortlink"

View File

@ -1,5 +1,5 @@
import memoizeOne from 'memoize-one'; import memoizeOne from 'memoize-one';
import React, { PureComponent } from 'react'; import React, { ComponentProps, PureComponent } from 'react';
import { import {
TimeZone, TimeZone,
@ -42,11 +42,14 @@ export interface Props extends Themeable2 {
getFieldLinks?: (field: Field, rowIndex: number, dataFrame: DataFrame) => Array<LinkModel<Field>>; getFieldLinks?: (field: Field, rowIndex: number, dataFrame: DataFrame) => Array<LinkModel<Field>>;
onClickShowField?: (key: string) => void; onClickShowField?: (key: string) => void;
onClickHideField?: (key: string) => void; onClickHideField?: (key: string) => void;
onPinLine?: (row: LogRowModel) => void;
onUnpinLine?: (row: LogRowModel) => void;
onLogRowHover?: (row?: LogRowModel) => void; onLogRowHover?: (row?: LogRowModel) => void;
onOpenContext?: (row: LogRowModel, onClose: () => void) => void; onOpenContext?: (row: LogRowModel, onClose: () => void) => void;
onPermalinkClick?: (row: LogRowModel) => Promise<void>; onPermalinkClick?: (row: LogRowModel) => Promise<void>;
permalinkedRowId?: string; permalinkedRowId?: string;
scrollIntoView?: (element: HTMLElement) => void; scrollIntoView?: (element: HTMLElement) => void;
pinnedRowId?: string;
} }
interface State { interface State {
@ -142,7 +145,7 @@ class UnThemedLogRows extends PureComponent<Props, State> {
// React profiler becomes unusable if we pass all rows to all rows and their labels, using getter instead // React profiler becomes unusable if we pass all rows to all rows and their labels, using getter instead
const getRows = this.makeGetRows(orderedRows); const getRows = this.makeGetRows(orderedRows);
const getLogRowProperties = (row: LogRowModel) => { const getLogRowProperties = (row: LogRowModel): ComponentProps<typeof LogRow> => {
return { return {
getRows: getRows, getRows: getRows,
row: row, row: row,
@ -169,6 +172,9 @@ class UnThemedLogRows extends PureComponent<Props, State> {
onPermalinkClick: this.props.onPermalinkClick, onPermalinkClick: this.props.onPermalinkClick,
scrollIntoView: this.props.scrollIntoView, scrollIntoView: this.props.scrollIntoView,
permalinkedRowId: this.props.permalinkedRowId, permalinkedRowId: this.props.permalinkedRowId,
onPinLine: this.props.onPinLine,
onUnpinLine: this.props.onUnpinLine,
pinned: this.props.pinnedRowId === row.uid,
}; };
}; };
return ( return (

View File

@ -89,6 +89,10 @@ export const getLogRowStyles = memoizeOne((theme: GrafanaTheme2) => {
z-index: 1; z-index: 1;
} }
.log-row-menu-visible {
visibility: hidden;
}
background: ${hoverBgColor}; background: ${hoverBgColor};
} }
@ -248,7 +252,6 @@ export const getLogRowStyles = memoizeOne((theme: GrafanaTheme2) => {
box-shadow: ${theme.shadows.z3}; box-shadow: ${theme.shadows.z3};
padding: ${theme.spacing(0.5, 1, 0.5, 1)}; padding: ${theme.spacing(0.5, 1, 0.5, 1)};
z-index: 100; z-index: 100;
visibility: hidden;
gap: ${theme.spacing(0.5)}; gap: ${theme.spacing(0.5)};
& > button { & > button {
@ -302,6 +305,14 @@ export const getLogRowStyles = memoizeOne((theme: GrafanaTheme2) => {
padding-top: ${theme.spacing(0.5)}; padding-top: ${theme.spacing(0.5)};
} }
`, `,
hidden: css`
label: hidden;
visibility: hidden;
`,
unPinButton: css`
height: ${theme.spacing(3)};
line-height: ${theme.spacing(2.5)};
`,
}; };
}); });

View File

@ -334,4 +334,44 @@ describe('LogRowContextModal', () => {
await waitFor(() => expect(dispatchMock).toHaveBeenCalledWith(splitOpenSym)); await waitFor(() => expect(dispatchMock).toHaveBeenCalledWith(splitOpenSym));
}); });
it('should make the center row sticky on load', async () => {
render(
<LogRowContextModal
row={row}
open={true}
onClose={() => {}}
getRowContext={getRowContext}
timeZone={timeZone}
logsSortOrder={LogsSortOrder.Descending}
/>
);
await waitFor(() => {
const rows = screen.getByTestId('entry-row');
expect(rows).toHaveStyle('position: sticky');
});
});
it('should make the center row unsticky on unPinClick', async () => {
render(
<LogRowContextModal
row={row}
open={true}
onClose={() => {}}
getRowContext={getRowContext}
timeZone={timeZone}
logsSortOrder={LogsSortOrder.Descending}
/>
);
await waitFor(() => {
const rows = screen.getByTestId('entry-row');
expect(rows).toHaveStyle('position: sticky');
});
const unpinButtons = screen.getAllByLabelText('Unpin line')[0];
await userEvent.click(unpinButtons);
const rows = screen.getByTestId('entry-row');
expect(rows).not.toHaveStyle('position: sticky');
});
}); });

View File

@ -41,11 +41,13 @@ const getStyles = (theme: GrafanaTheme2) => {
left: 50%; left: 50%;
transform: translate(-50%, -50%); transform: translate(-50%, -50%);
`, `,
entry: css` sticky: css`
position: sticky; position: sticky;
z-index: 1; z-index: 1;
top: -1px; top: -1px;
bottom: -1px; bottom: -1px;
`,
entry: css`
& > td { & > td {
padding: ${theme.spacing(1)} 0 ${theme.spacing(1)} 0; padding: ${theme.spacing(1)} 0 ${theme.spacing(1)} 0;
} }
@ -54,6 +56,10 @@ const getStyles = (theme: GrafanaTheme2) => {
& > table { & > table {
margin-bottom: 0; margin-bottom: 0;
} }
& .log-row-menu {
margin-top: -6px;
}
`, `,
datasourceUi: css` datasourceUi: css`
padding-bottom: ${theme.spacing(1.25)}; padding-bottom: ${theme.spacing(1.25)};
@ -178,6 +184,8 @@ export const LogRowContextModal: React.FunctionComponent<LogRowContextModalProps
const theme = useTheme2(); const theme = useTheme2();
const styles = getStyles(theme); const styles = getStyles(theme);
const [sticky, setSticky] = useState(true);
// we need to keep both the "above" and "below" rows // we need to keep both the "above" and "below" rows
// in the same react-state, to be able to atomically change both // in the same react-state, to be able to atomically change both
// at the same time. // at the same time.
@ -448,7 +456,7 @@ export const LogRowContextModal: React.FunctionComponent<LogRowContextModalProps
</td> </td>
</tr> </tr>
<tr ref={preEntryElement}></tr> <tr ref={preEntryElement}></tr>
<tr ref={entryElement} className={styles.entry}> <tr ref={entryElement} className={cx(styles.entry, sticky ? styles.sticky : null)} data-testid="entry-row">
<td className={styles.noMarginBottom}> <td className={styles.noMarginBottom}>
<LogRows <LogRows
logRows={[row]} logRows={[row]}
@ -462,6 +470,9 @@ export const LogRowContextModal: React.FunctionComponent<LogRowContextModalProps
displayedFields={displayedFields} displayedFields={displayedFields}
onClickShowField={showField} onClickShowField={showField}
onClickHideField={hideField} onClickHideField={hideField}
onUnpinLine={() => setSticky(false)}
onPinLine={() => setSticky(true)}
pinnedRowId={sticky ? row.uid : undefined}
/> />
</td> </td>
</tr> </tr>