[Security Solution] TopN chart styling issue (#109007)

* fix topN style

* add unit tests for topN

* add unit tests

* review
This commit is contained in:
Angela Chuang 2021-08-19 17:35:28 +01:00 committed by GitHub
parent 5957d313e0
commit 9d4c062e64
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 361 additions and 37 deletions

View file

@ -0,0 +1,166 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { EuiButtonEmpty, EuiContextMenuItem } from '@elastic/eui';
import { mount } from 'enzyme';
import React from 'react';
import { TestProviders } from '../../../mock';
import { ShowTopNButton } from './show_top_n';
describe('show topN button', () => {
const defaultProps = {
field: 'signal.rule.name',
onClick: jest.fn(),
ownFocus: false,
showTopN: false,
timelineId: 'timeline-1',
value: ['rule_name'],
};
describe('button', () => {
test('should show EuiButtonIcon by default', () => {
const wrapper = mount(
<TestProviders>
<ShowTopNButton {...defaultProps} />
</TestProviders>
);
expect(wrapper.find('EuiButtonIcon').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="show-top-field"]').first().prop('iconType')).toEqual(
'visBarVertical'
);
});
test('should support EuiButtonEmpty', () => {
const testProps = {
...defaultProps,
Component: EuiButtonEmpty,
};
const wrapper = mount(
<TestProviders>
<ShowTopNButton {...testProps} />
</TestProviders>
);
expect(wrapper.find('EuiButtonIcon').exists()).toBeFalsy();
expect(wrapper.find('EuiButtonEmpty').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="show-top-field"]').first().prop('iconType')).toEqual(
'visBarVertical'
);
});
test('should support EuiContextMenuItem', () => {
const testProps = {
...defaultProps,
Component: EuiContextMenuItem,
};
const wrapper = mount(
<TestProviders>
<ShowTopNButton {...testProps} />
</TestProviders>
);
expect(wrapper.find('EuiButtonIcon').exists()).toBeFalsy();
expect(wrapper.find('EuiContextMenuItem').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="show-top-field"]').first().prop('icon')).toEqual(
'visBarVertical'
);
});
});
describe('tooltip', () => {
test('should show tooltip by default', () => {
const wrapper = mount(
<TestProviders>
<ShowTopNButton {...defaultProps} />
</TestProviders>
);
expect(wrapper.find('EuiToolTip').exists()).toBeTruthy();
});
test('should hide tooltip when topN is showed', () => {
const testProps = {
...defaultProps,
showTopN: true,
};
const wrapper = mount(
<TestProviders>
<ShowTopNButton {...testProps} />
</TestProviders>
);
expect(wrapper.find('EuiToolTip').exists()).toBeFalsy();
});
test('should hide tooltip by setting showTooltip to false', () => {
const testProps = {
...defaultProps,
showTooltip: false,
};
const wrapper = mount(
<TestProviders>
<ShowTopNButton {...testProps} />
</TestProviders>
);
expect(wrapper.find('EuiToolTip').exists()).toBeFalsy();
});
});
describe('popover', () => {
test('should be able to show topN without a popover', () => {
const testProps = {
...defaultProps,
enablePopOver: false,
showTopN: true,
};
const wrapper = mount(
<TestProviders>
<ShowTopNButton {...testProps} />
</TestProviders>
);
expect(wrapper.find('[data-test-subj="top-n"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="showTopNContainer"]').exists()).toBeFalsy();
});
test('should be able to show topN within a popover', () => {
const testProps = {
...defaultProps,
enablePopOver: true,
showTopN: true,
};
const wrapper = mount(
<TestProviders>
<ShowTopNButton {...testProps} />
</TestProviders>
);
expect(wrapper.find('[data-test-subj="top-n"]').exists()).toBeTruthy();
expect(wrapper.find('[data-test-subj="showTopNContainer"]').exists()).toBeTruthy();
});
});
describe('topN', () => {
test('should render with correct props', () => {
const onFilterAdded = jest.fn();
const testProps = {
...defaultProps,
enablePopOver: true,
showTopN: true,
onFilterAdded,
};
const wrapper = mount(
<TestProviders>
<ShowTopNButton {...testProps} />
</TestProviders>
);
expect(wrapper.find('[data-test-subj="top-n"]').prop('field')).toEqual(testProps.field);
expect(wrapper.find('[data-test-subj="top-n"]').prop('value')).toEqual(testProps.value);
expect(wrapper.find('[data-test-subj="top-n"]').prop('toggleTopN')).toEqual(
testProps.onClick
);
expect(wrapper.find('[data-test-subj="top-n"]').prop('timelineId')).toEqual(
testProps.timelineId
);
expect(wrapper.find('[data-test-subj="top-n"]').prop('onFilterAdded')).toEqual(
testProps.onFilterAdded
);
});
});
});

View file

@ -6,7 +6,13 @@
*/
import React, { useMemo } from 'react';
import { EuiButtonEmpty, EuiButtonIcon, EuiContextMenuItem, EuiToolTip } from '@elastic/eui';
import {
EuiButtonEmpty,
EuiPopover,
EuiButtonIcon,
EuiContextMenuItem,
EuiToolTip,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { StatefulTopN } from '../../top_n';
import { TimelineId } from '../../../../../common/types/timeline';
@ -23,8 +29,11 @@ const SHOW_TOP = (fieldName: string) =>
});
interface Props {
/** `Component` is only used with `EuiDataGrid`; the grid keeps a reference to `Component` for show / hide functionality */
/** When `Component` is used with `EuiDataGrid`; the grid keeps a reference to `Component` for show / hide functionality.
* When `Component` is used with `EuiContextMenu`, we pass EuiContextMenuItem to render the right style.
*/
Component?: typeof EuiButtonEmpty | typeof EuiButtonIcon | typeof EuiContextMenuItem;
enablePopOver?: boolean;
field: string;
onClick: () => void;
onFilterAdded?: () => void;
@ -38,6 +47,7 @@ interface Props {
export const ShowTopNButton: React.FC<Props> = React.memo(
({
Component,
enablePopOver,
field,
onClick,
onFilterAdded,
@ -58,7 +68,7 @@ export const ShowTopNButton: React.FC<Props> = React.memo(
: SourcererScopeName.default;
const { browserFields, indexPattern } = useSourcererScope(activeScope);
const button = useMemo(
const basicButton = useMemo(
() =>
Component ? (
<Component
@ -84,32 +94,59 @@ export const ShowTopNButton: React.FC<Props> = React.memo(
[Component, field, onClick]
);
const button = useMemo(
() =>
showTooltip && !showTopN ? (
<EuiToolTip
content={
<TooltipWithKeyboardShortcut
additionalScreenReaderOnlyContext={getAdditionalScreenReaderOnlyContext({
field,
value,
})}
content={SHOW_TOP(field)}
shortcut={SHOW_TOP_N_KEYBOARD_SHORTCUT}
showShortcut={ownFocus}
/>
}
>
{basicButton}
</EuiToolTip>
) : (
basicButton
),
[basicButton, field, ownFocus, showTooltip, showTopN, value]
);
const topNPannel = useMemo(
() => (
<StatefulTopN
browserFields={browserFields}
field={field}
indexPattern={indexPattern}
onFilterAdded={onFilterAdded}
timelineId={timelineId ?? undefined}
toggleTopN={onClick}
value={value}
/>
),
[browserFields, field, indexPattern, onClick, onFilterAdded, timelineId, value]
);
return showTopN ? (
<StatefulTopN
browserFields={browserFields}
field={field}
indexPattern={indexPattern}
onFilterAdded={onFilterAdded}
timelineId={timelineId ?? undefined}
toggleTopN={onClick}
value={value}
/>
) : showTooltip ? (
<EuiToolTip
content={
<TooltipWithKeyboardShortcut
additionalScreenReaderOnlyContext={getAdditionalScreenReaderOnlyContext({
field,
value,
})}
content={SHOW_TOP(field)}
shortcut={SHOW_TOP_N_KEYBOARD_SHORTCUT}
showShortcut={ownFocus}
/>
}
>
{button}
</EuiToolTip>
enablePopOver ? (
<EuiPopover
button={basicButton}
isOpen={showTopN}
closePopover={onClick}
panelClassName="withHoverActions__popover"
data-test-subj="showTopNContainer"
>
{topNPannel}
</EuiPopover>
) : (
topNPannel
)
) : (
button
);

View file

@ -0,0 +1,115 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { useRef } from 'react';
import { renderHook, act } from '@testing-library/react-hooks';
import { useHoverActionItems, UseHoverActionItemsProps } from './use_hover_action_items';
import { useDeepEqualSelector } from '../../hooks/use_selector';
import { DataProvider } from '../../../../common/types/timeline';
jest.mock('../../lib/kibana');
jest.mock('../../hooks/use_selector');
jest.mock('../../containers/sourcerer', () => ({
useSourcererScope: jest.fn().mockReturnValue({ browserFields: {} }),
}));
describe('useHoverActionItems', () => {
const defaultProps: UseHoverActionItemsProps = ({
dataProvider: [{} as DataProvider],
defaultFocusedButtonRef: null,
field: 'signal.rule.name',
handleHoverActionClicked: jest.fn(),
isObjectArray: true,
ownFocus: false,
showTopN: false,
stKeyboardEvent: undefined,
toggleColumn: jest.fn(),
toggleTopN: jest.fn(),
values: ['rule name'],
} as unknown) as UseHoverActionItemsProps;
beforeEach(() => {
(useDeepEqualSelector as jest.Mock).mockImplementation((cb) => {
return cb();
});
});
afterEach(() => {
(useDeepEqualSelector as jest.Mock).mockClear();
});
test('should return allActionItems', async () => {
await act(async () => {
const { result, waitForNextUpdate } = renderHook(() => {
const defaultFocusedButtonRef = useRef(null);
const testProps = {
...defaultProps,
defaultFocusedButtonRef,
};
return useHoverActionItems(testProps);
});
await waitForNextUpdate();
expect(result.current.allActionItems).toHaveLength(6);
expect(result.current.allActionItems[0].props['data-test-subj']).toEqual(
'hover-actions-filter-for'
);
expect(result.current.allActionItems[1].props['data-test-subj']).toEqual(
'hover-actions-filter-out'
);
expect(result.current.allActionItems[2].props['data-test-subj']).toEqual(
'hover-actions-toggle-column'
);
expect(result.current.allActionItems[3].props['data-test-subj']).toEqual(
'hover-actions-add-timeline'
);
expect(result.current.allActionItems[4].props['data-test-subj']).toEqual(
'hover-actions-show-top-n'
);
expect(result.current.allActionItems[5].props['data-test-subj']).toEqual(
'hover-actions-copy-button'
);
});
});
test('should return overflowActionItems', async () => {
await act(async () => {
const { result, waitForNextUpdate } = renderHook(() => {
const defaultFocusedButtonRef = useRef(null);
const testProps = {
...defaultProps,
defaultFocusedButtonRef,
enableOverflowButton: true,
};
return useHoverActionItems(testProps);
});
await waitForNextUpdate();
expect(result.current.overflowActionItems).toHaveLength(3);
expect(result.current.overflowActionItems[0].props['data-test-subj']).toEqual(
'hover-actions-filter-for'
);
expect(result.current.overflowActionItems[1].props['data-test-subj']).toEqual(
'hover-actions-filter-out'
);
expect(result.current.overflowActionItems[2].props['data-test-subj']).toEqual(
'more-actions-signal.rule.name'
);
expect(result.current.overflowActionItems[2].props.items[0].props['data-test-subj']).toEqual(
'hover-actions-toggle-column'
);
expect(result.current.overflowActionItems[2].props.items[1].props['data-test-subj']).toEqual(
'hover-actions-add-timeline'
);
expect(result.current.overflowActionItems[2].props.items[2].props['data-test-subj']).toEqual(
'hover-actions-show-top-n'
);
expect(result.current.overflowActionItems[2].props.items[3].props['data-test-subj']).toEqual(
'hover-actions-copy-button'
);
});
});
});

View file

@ -21,7 +21,7 @@ import { useSourcererScope } from '../../containers/sourcerer';
import { timelineSelectors } from '../../../timelines/store/timeline';
import { ShowTopNButton } from './actions/show_top_n';
interface UseHoverActionItemsProps {
export interface UseHoverActionItemsProps {
dataProvider?: DataProvider | DataProvider[];
dataType?: string;
defaultFocusedButtonRef: React.MutableRefObject<HTMLButtonElement | null>;
@ -43,7 +43,7 @@ interface UseHoverActionItemsProps {
values?: string[] | string | null;
}
interface UseHoverActionItems {
export interface UseHoverActionItems {
overflowActionItems: JSX.Element[];
allActionItems: JSX.Element[];
}
@ -116,7 +116,6 @@ export const useHoverActionItems = ({
*/
const showFilters =
values != null && (enableOverflowButton || (!showTopN && !enableOverflowButton));
const allItems = useMemo(
() =>
[
@ -243,7 +242,7 @@ export const useHoverActionItems = ({
]
) as JSX.Element[];
const overflowBtn = useMemo(
const showTopNBtn = useMemo(
() => (
<ShowTopNButton
Component={enableOverflowButton ? EuiContextMenuItem : undefined}
@ -276,7 +275,7 @@ export const useHoverActionItems = ({
onClick: onOverflowButtonClick,
showTooltip: enableOverflowButton ? false : true,
value: values,
items: showTopN ? [overflowBtn] : allItems.slice(itemsToShow),
items: showTopN ? [showTopNBtn] : allItems.slice(itemsToShow),
isOverflowPopoverOpen: !!isOverflowPopoverOpen,
}),
]
@ -293,7 +292,7 @@ export const useHoverActionItems = ({
isOverflowPopoverOpen,
itemsToShow,
onOverflowButtonClick,
overflowBtn,
showTopNBtn,
ownFocus,
showTopN,
stKeyboardEvent,
@ -301,9 +300,9 @@ export const useHoverActionItems = ({
]
);
const allActionItems = useMemo(() => (showTopN ? [overflowBtn] : allItems), [
const allActionItems = useMemo(() => (showTopN ? [showTopNBtn] : allItems), [
allItems,
overflowBtn,
showTopNBtn,
showTopN,
]);

View file

@ -140,6 +140,7 @@ export const defaultCellActions: TGridCellAction[] = [
}) && (
<ShowTopNButton
Component={Component}
enablePopOver
data-test-subj="hover-actions-show-top-n"
field={columnId}
onClick={onClick}

View file

@ -16,6 +16,7 @@ import {
EuiToolTip,
} from '@elastic/eui';
import styled from 'styled-components';
import { stopPropagationAndPreventDefault } from '../../../../common';
import { TooltipWithKeyboardShortcut } from '../../tooltip_with_keyboard_shortcut';
import { getAdditionalScreenReaderOnlyContext } from '../utils';
@ -34,6 +35,10 @@ export interface OverflowButtonProps extends HoverActionComponentProps {
isOverflowPopoverOpen: boolean;
}
const StyledEuiContextMenuPanel = styled(EuiContextMenuPanel)`
visibility: inherit;
`;
const OverflowButton: React.FC<OverflowButtonProps> = React.memo(
({
closePopOver,
@ -91,9 +96,10 @@ const OverflowButton: React.FC<OverflowButtonProps> = React.memo(
isOpen={isOverflowPopoverOpen}
closePopover={closePopOver}
panelPaddingSize="none"
panelClassName="withHoverActions__popover"
anchorPosition="downLeft"
>
<EuiContextMenuPanel items={items} />
<StyledEuiContextMenuPanel items={items} />
</EuiPopover>
),
[