-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Security Solution] Add CellActions (alpha version) component to ui_actions plugin #147434
Merged
machadoum
merged 15 commits into
elastic:main
from
machadoum:siem-explore-145663-ui-action
Dec 20, 2022
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
725ccfd
Add CellActions component to ui_actions plugin
machadoum 331f4d3
Fix axe error
machadoum c70cc9e
Add ui actions to storybook CI
machadoum d779139
Add Readme file to ui-action component
machadoum 1bdf7a7
Add improvements suggested during code review
machadoum 310b666
Improve CellAction popover by not rendering visible actions if extra …
machadoum 3404f32
Fix hover actions popover calling getActions when unmounted
machadoum c0811c1
Refactor components to access 'getAction'' from context instead of fr…
machadoum 8a101ec
Update CellActions config property name to be more intuitive
machadoum c98beba
Add unit test for when action order is undefined
machadoum cac804d
Fix unit tests broken when renaming config
machadoum eb50c58
Replace setHoverTimeout for debounced function
machadoum cd96c5e
Add code review suggestions
machadoum cf8373e
Add code review suggestions [2]
machadoum 226993c
Fix extra actions doesn't close when hover content is clicked
machadoum File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
/* | ||
* 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 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
const defaultConfig = require('@kbn/storybook').defaultConfig; | ||
|
||
module.exports = { | ||
...defaultConfig, | ||
stories: ['../**/*.stories.tsx'], | ||
reactOptions: { | ||
strictMode: true, | ||
}, | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
This package provides a uniform interface for displaying UI actions for a cell. | ||
For the `CellActions` component to work, it must be wrapped by `CellActionsContextProvider`. Ideally, the wrapper should stay on the top of the rendering tree. | ||
|
||
Example: | ||
```JSX | ||
<CellActionsContextProvider | ||
// call uiActions.getTriggerCompatibleActions(triggerId, data) | ||
getCompatibleActions={getCompatibleActions}> | ||
... | ||
<CellActions mode={CellActionsMode.HOVER_POPOVER} triggerId={MY_TRIGGER_ID} config={{ field: 'fieldName', value: 'fieldValue', fieldType: 'text' }}> | ||
Hover me | ||
</CellActions> | ||
</CellActionsContextProvider> | ||
|
||
``` | ||
|
||
`CellActions` component will display all compatible actions registered for the trigger id. |
34 changes: 34 additions & 0 deletions
34
src/plugins/ui_actions/public/cell_actions/components/cell_action_item.test.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
/* | ||
* 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 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import { render } from '@testing-library/react'; | ||
import React from 'react'; | ||
import { makeAction } from '../mocks/helpers'; | ||
import { CellActionExecutionContext } from './cell_actions'; | ||
import { ActionItem } from './cell_action_item'; | ||
|
||
describe('ActionItem', () => { | ||
it('renders', () => { | ||
const action = makeAction('test-action'); | ||
const actionContext = {} as CellActionExecutionContext; | ||
const { queryByTestId } = render( | ||
<ActionItem action={action} actionContext={actionContext} showTooltip={false} /> | ||
); | ||
expect(queryByTestId('actionItem-test-action')).toBeInTheDocument(); | ||
}); | ||
|
||
it('renders tooltip when showTooltip=true is received', () => { | ||
const action = makeAction('test-action'); | ||
const actionContext = {} as CellActionExecutionContext; | ||
const { container } = render( | ||
<ActionItem action={action} actionContext={actionContext} showTooltip /> | ||
); | ||
|
||
expect(container.querySelector('.euiToolTipAnchor')).not.toBeNull(); | ||
}); | ||
}); |
45 changes: 45 additions & 0 deletions
45
src/plugins/ui_actions/public/cell_actions/components/cell_action_item.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
/* | ||
* 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 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import React, { useMemo } from 'react'; | ||
|
||
import { EuiButtonIcon, EuiToolTip, IconType } from '@elastic/eui'; | ||
import type { Action } from '../../actions'; | ||
import { CellActionExecutionContext } from './cell_actions'; | ||
|
||
export const ActionItem = ({ | ||
action, | ||
actionContext, | ||
showTooltip, | ||
}: { | ||
action: Action; | ||
actionContext: CellActionExecutionContext; | ||
showTooltip: boolean; | ||
}) => { | ||
const actionProps = useMemo( | ||
() => ({ | ||
iconType: action.getIconType(actionContext) as IconType, | ||
onClick: () => action.execute(actionContext), | ||
'data-test-subj': `actionItem-${action.id}`, | ||
'aria-label': action.getDisplayName(actionContext), | ||
}), | ||
[action, actionContext] | ||
); | ||
|
||
if (!actionProps.iconType) return null; | ||
|
||
return showTooltip ? ( | ||
<EuiToolTip | ||
content={action.getDisplayNameTooltip ? action.getDisplayNameTooltip(actionContext) : ''} | ||
> | ||
<EuiButtonIcon {...actionProps} iconSize="s" /> | ||
</EuiToolTip> | ||
) : ( | ||
<EuiButtonIcon {...actionProps} iconSize="s" /> | ||
); | ||
}; |
77 changes: 77 additions & 0 deletions
77
src/plugins/ui_actions/public/cell_actions/components/cell_actions.stories.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
/* | ||
* 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 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import React from 'react'; | ||
import { ComponentStory } from '@storybook/react'; | ||
import { CellActionsContextProvider } from './cell_actions_context'; | ||
import { makeAction } from '../mocks/helpers'; | ||
import { CellActions, CellActionsMode, CellActionsProps } from './cell_actions'; | ||
|
||
const TRIGGER_ID = 'testTriggerId'; | ||
|
||
const FIELD = { name: 'name', value: '123', type: 'text' }; | ||
|
||
const getCompatibleActions = () => | ||
Promise.resolve([ | ||
makeAction('Filter in', 'plusInCircle', 2), | ||
makeAction('Filter out', 'minusInCircle', 3), | ||
makeAction('Minimize', 'minimize', 1), | ||
makeAction('Send email', 'email', 4), | ||
makeAction('Pin field', 'pin', 5), | ||
]); | ||
|
||
export default { | ||
title: 'CellAction', | ||
decorators: [ | ||
(storyFn: Function) => ( | ||
<CellActionsContextProvider | ||
// call uiActions getTriggerCompatibleActions(triggerId, data) | ||
getTriggerCompatibleActions={getCompatibleActions} | ||
> | ||
<div style={{ paddingTop: '70px' }} /> | ||
{storyFn()} | ||
</CellActionsContextProvider> | ||
), | ||
], | ||
}; | ||
|
||
const CellActionsTemplate: ComponentStory<React.FC<CellActionsProps>> = (args) => ( | ||
<CellActions {...args}>Field value</CellActions> | ||
); | ||
|
||
export const DefaultWithControls = CellActionsTemplate.bind({}); | ||
|
||
DefaultWithControls.argTypes = { | ||
mode: { | ||
options: [CellActionsMode.HOVER_POPOVER, CellActionsMode.ALWAYS_VISIBLE], | ||
defaultValue: CellActionsMode.HOVER_POPOVER, | ||
control: { | ||
type: 'radio', | ||
}, | ||
}, | ||
}; | ||
|
||
DefaultWithControls.args = { | ||
showActionTooltips: true, | ||
mode: CellActionsMode.ALWAYS_VISIBLE, | ||
triggerId: TRIGGER_ID, | ||
field: FIELD, | ||
visibleCellActions: 3, | ||
}; | ||
|
||
export const CellActionInline = ({}: {}) => ( | ||
<CellActions mode={CellActionsMode.ALWAYS_VISIBLE} triggerId={TRIGGER_ID} field={FIELD}> | ||
Field value | ||
</CellActions> | ||
); | ||
|
||
export const CellActionHoverPopup = ({}: {}) => ( | ||
<CellActions mode={CellActionsMode.HOVER_POPOVER} triggerId={TRIGGER_ID} field={FIELD}> | ||
Hover me | ||
</CellActions> | ||
); |
74 changes: 74 additions & 0 deletions
74
src/plugins/ui_actions/public/cell_actions/components/cell_actions.test.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,74 @@ | ||
/* | ||
* 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 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import { act, render } from '@testing-library/react'; | ||
import React from 'react'; | ||
import { CellActions, CellActionsMode } from './cell_actions'; | ||
import { CellActionsContextProvider } from './cell_actions_context'; | ||
|
||
const TRIGGER_ID = 'test-trigger-id'; | ||
const FIELD = { name: 'name', value: '123', type: 'text' }; | ||
|
||
describe('CellActions', () => { | ||
it('renders', async () => { | ||
const getActionsPromise = Promise.resolve([]); | ||
const getActions = () => getActionsPromise; | ||
|
||
const { queryByTestId } = render( | ||
<CellActionsContextProvider getTriggerCompatibleActions={getActions}> | ||
<CellActions mode={CellActionsMode.ALWAYS_VISIBLE} triggerId={TRIGGER_ID} field={FIELD}> | ||
Field value | ||
</CellActions> | ||
</CellActionsContextProvider> | ||
); | ||
|
||
await act(async () => { | ||
await getActionsPromise; | ||
}); | ||
|
||
expect(queryByTestId('cellActions')).toBeInTheDocument(); | ||
}); | ||
|
||
it('renders InlineActions when mode is ALWAYS_VISIBLE', async () => { | ||
const getActionsPromise = Promise.resolve([]); | ||
const getActions = () => getActionsPromise; | ||
|
||
const { queryByTestId } = render( | ||
<CellActionsContextProvider getTriggerCompatibleActions={getActions}> | ||
<CellActions mode={CellActionsMode.ALWAYS_VISIBLE} triggerId={TRIGGER_ID} field={FIELD}> | ||
Field value | ||
</CellActions> | ||
</CellActionsContextProvider> | ||
); | ||
|
||
await act(async () => { | ||
await getActionsPromise; | ||
}); | ||
|
||
expect(queryByTestId('inlineActions')).toBeInTheDocument(); | ||
}); | ||
|
||
it('renders HoverActionsPopover when mode is HOVER_POPOVER', async () => { | ||
const getActionsPromise = Promise.resolve([]); | ||
const getActions = () => getActionsPromise; | ||
|
||
const { queryByTestId } = render( | ||
<CellActionsContextProvider getTriggerCompatibleActions={getActions}> | ||
<CellActions mode={CellActionsMode.HOVER_POPOVER} triggerId={TRIGGER_ID} field={FIELD}> | ||
Field value | ||
</CellActions> | ||
</CellActionsContextProvider> | ||
); | ||
|
||
await act(async () => { | ||
await getActionsPromise; | ||
}); | ||
|
||
expect(queryByTestId('hoverActionsPopover')).toBeInTheDocument(); | ||
}); | ||
}); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it make sense to update the
Action
type to makegetIconType
returnIconType
instead ofstring | undefined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using
IconType
instead ofstring
makes sense to me. I think it is astring
becauseEuiContextMenuItemIcon
expects astring
instead ofIconType
. So updating it might lead to cascading changes that could grow big.Tagging @Dosant and @vadimkibana to collect their input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to think about this. That would mean
ui_actions
plugin service layer would have a dependency on EUI, which maybe is fine, maybe not desirable.