[Security Solution][Lens] New trigger actions for chart legends and table cell actions #146779

semd · 2022-12-01T12:01:43Z

Summary

Enable XY chart legends and Table cell actions to render dynamically registered "cell value" actions, scoped for Lens embeddables only.
Security Solution actions were created to be displayed in the Lens visualizations rendered.

closes #145708

New Trigger

A new uiActions trigger CELL_VALUE_TRIGGER has been created to register these actions. It is used in both TableChart cell actions and XyCharts legend actions to create additional action options.

Table cell actions

Actions registered and added to the CELL_VALUE_TRIGGER will be appended to the cell actions of the table. The action compatibility is checked for each column.

Chart legend actions

The same for XyCharts legends. All actions registered and added to the CELL_VALUE_TRIGGER will be appended to the legend actions of the charts. The action compatibility is checked for each series accessor.

Filter for & Filter out actions:

The XY and Table charts have the "Filter for & Filter out" actions hardcoded in the components code. They manually check the value is filterable before showing the actions, but the panel items and cell actions are added explicitly for them in the components.

This logic has not changed in this PR, the actions added dynamically using CELL_VALUE_TRIGGER are just appended after the "Filter for & Filter out" options.

However, "Filter for / Filter out" actions could also be integrated into this pattern, we would only have to register these actions to the CELL_VALUE_TRIGGER with the proper order value, and then remove all hardcoded logic in the components.

Security Solution actions

The actions that we have registered from Security Solutions ("Add to timeline" and "Copy to clipboard") will be displayed only when the embeddable is rendered inside Security Solution, this is ensured via the isCompatible actions method.

Security Solution Filter In/Out bug

There was a CSS rule present in a global Security Solution context, affecting all the EuiDataGrid cell actions popovers:

kibana/x-pack/plugins/security_solution/public/common/components/page/index.tsx

Lines 124 to 130 in 475e47e

    
               // Hide EUI's 'Filter in' and 'Filter out' footer buttons - replaced with our own buttons 
        
               .euiPopoverFooter:nth-child(2) { 
        
                 .euiFlexItem:first-child, 
        
                 .euiFlexItem:nth-child(2) { 
        
                     display: none; 
        
                 } 
        
               }

The goal of this rule was to save some vertical space, by hiding the two auto-generated Filter In/Out actions (generally the first two cell actions), so we could show the horizontal Filter In/Out custom buttons added manually to the popover top header.

This CSS rule was causing a bug when rendering Table visualizations (they use EuiDataGrid as well), always hiding the two first cell actions in the popover.

After discussing this topic with the team, and considering there is an ongoing task to unify the cell actions experience in Security and centralize the implementation (see: #144943), we decided to remove the problematic CSS rule and the custom horizontal Filter buttons, so the auto-generated Filter actions are displayed, this way all tables in Security (alerts, events, and also embedded table charts) will have a unified and consistent UX.

If the cell actions popover grows too much in the future we could apply a "More actions ..." footer option strategy.

Testing

The new actions will be displayed on any embedded Lens object rendered inside Security. A good place to test that is Cases, since we can attach Lens visualizations in the comments. However, Cases explicitly disables all the trigger actions passing disableTriggers to the Lens Embeddables.

kibana/x-pack/plugins/cases/public/components/markdown_editor/plugins/lens/processor.tsx

Line 52 in b80a23e

disableTriggers

The easiest way to test different chart types and tables is to remove/comment the disableTriggers prop in the Cases processor component, and then go to a Case and attach different Lens visualizations. All actions should appear in the legends and table cell hover actions.

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)

elasticmachine · 2022-12-01T17:50:40Z

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

elasticmachine · 2022-12-01T17:50:42Z

Pinging @elastic/security-solution (Team: SecuritySolution)

…-ref HEAD~1..HEAD --fix'

semd · 2022-12-02T12:03:19Z

x-pack/plugins/security_solution/public/common/components/page/index.tsx

-  .euiDataGridRowCell .euiDataGridRowCell__expandActions .euiDataGridRowCell__actionButtonIcon {
-    display: none;
-
-    &:first-child,
-    &:nth-child(2),
-    &:nth-child(3),
-    &:last-child {
-      display: inline-flex;
-    }
-
-  }


removed in favor of visibleCellActions prop

Thanks for cleaning this up! 👍

x-pack/plugins/security_solution/public/actions/add_to_timeline/add_to_timeline_action.ts

x-pack/plugins/security_solution/public/actions/add_to_timeline/data_provider.ts

…ue_actions

…md/kibana into 145708_viz_data_value_actions

semd · 2022-12-12T12:41:10Z

Hi @ThomThomson , thanks a lot for taking a look at the PR.
About your comment

Is there a reason that the new trigger needs to be registered inside the embeddable plugin? Most of the triggers registered here have to do directly with an embeddable function. Mostly the triggers that have to do with the embeddable panel itself.

I have tried to follow the existing pattern that dashboard embeddable uses when it comes to registering the triggers. The triggers that are used in the embeddables, via dashboard or data actions, are registered in the embeddable plugin, some of which are also very related to visualizations (see SELECT_RANGE_TRIGGER, VALUE_CLICK_TRIGGER).
This new trigger works the same way but with lens embeddable, so I assumed it could be registered in the same place.

Other plugins can and should register UI Actions and triggers where appropriate. Is this trigger only used by Lens? If so, it might belong in the Lens plugin instead.
If this is expected to be a general purpose trigger used by multiple plugins, I'll happily approve.

For now, it is only used by Lens, but its purpose is not restricted to it, I tried to make it as generic as possible. It is not tied to Lens, visualization legends or table actions specifically. So, in the future, it can be used by other plugins to show actions for a rendered value.
The ValueHoverActions (package) component for instance, that will be created soon, could also use this trigger.

Having that said, I don't have a strong opinion about it, I registered the trigger in the embeddable as the "default" place, since the trigger can be potentially used by different entities, but I am not 100% convinced it is the correct place either. I am open to moving the trigger register to any other place that you think is more convenient.

ThomThomson

Thank you for the explanation @semd, as long as the implementation is intended to be generic, the embeddable plugin seems like an okay home for the time being!

LGTM - code review only.

semd · 2022-12-12T19:31:36Z

Hi @stephmilovic,
About your questions:

My first question may be dumb, related my unfamiliarity with Lens. My video below demonstrates the following behavior: I was able to use the Treemap visualization and see the actions in security/cases. However, on other visualizations like "Bar vertical stacked" that looks like it has the same type of legend, we do not have the actions in cases. Is this actually a different category of renderers? If so, is the plan to follow up and apply our actions to them all?
Next, I'm wondering where the filters get applied to? See in my video, I do the timeline/copy actions successfully but am confused by the filters. For filter in and out, either we should be hiding those on cases pages or apply to global kql, but not do nothing? Am i missing what these buttons are doing within security? Edit: this behavior already in main for the filters, we should fix it in follow up pr

Not dumb at all, I had to dig into the charts code to find that there's a disableTriggers flag prop in the embeddables, that cases set to true when rendering the embeddable in the comments:

kibana/x-pack/plugins/cases/public/components/markdown_editor/plugins/lens/processor.tsx

Line 52 in b80a23e

disableTriggers

EDIT:
The problem is that only a few visualization types check this flag when rendering the actions, like the stacked bar chart (XYChart), but some others don't check that flag and always show the actions if they are compatible. Even though the Filter actions do not work when the disableTriggers is true, the options are rendered anyway, that seems a bug.

I assume Cases set the disableTriggers to true to prevent Lens to show these filtering actions, it does not make much sense to have them on the Cases page.

I did not realize this flag existed, so right now the AddToTimeline and CopyToClipboard actions are inconsistent with the Filter actions, they should also be hidden inside Cases. I am fixing that.

The ideal scenario for us would probably be to display all the actions except for the Filter For/Out inside cases, but we would need more granularity to do so, the disableTriggers prop is not enough.

Since this PR is adding actions in a general way inside Security, we could keep all actions hidden in Cases by now, and discuss this topic with @paulewing and the Cases team.

What do you think?

…ue_actions

…-fix'

stephmilovic

Thank you for adding all the tests requested, LGTM!

michaelolo24 · 2022-12-14T18:56:43Z

...urity_solution/public/timelines/components/timeline/cell_rendering/default_cell_renderer.tsx


 const hasCellActions = (columnId?: string) => {
  return columnId && !FIELDS_WITHOUT_CELL_ACTIONS.includes(columnId);
 };

+const StyledContent = styled.div<{ $isDetails: boolean }>`


michaelolo24

Thanks for making this change! Everything seems to look and work fine for the tables. Investigations Codeowners LGTM!

…md/kibana into 145708_viz_data_value_actions

…ue_actions

dej611

Found only minor nitpicks here and there, nothing major.
Tested locally on Safari and found no regression.

dej611 · 2022-12-15T11:30:44Z

..._expressions/expression_partition_vis/public/expression_renderers/partition_vis_renderer.tsx

@@ -60,6 +84,12 @@ export const getPartitionVisRenderer: (
      unmountComponentAtNode(domNode);
    });

+    const columnCellValueActions = await getColumnCellValueActions(


Can we combine this with the palette async code below?

dej611 · 2022-12-15T12:06:46Z

src/plugins/chart_expressions/expression_xy/public/components/xy_chart.test.tsx

@@ -58,8 +58,10 @@ import { ExtendedDataLayerConfig, XYProps, AnnotationLayerConfigResult } from '.
 import { DataLayers } from './data_layers';
 import { SplitChart } from './split_chart';
 import { LegendSize } from '@kbn/visualizations-plugin/common';
+import { LayerCellValueActions } from '../types';


Suggested change

import { LayerCellValueActions } from '../types';

import type { LayerCellValueActions } from '../types';

dej611 · 2022-12-15T12:11:16Z

x-pack/plugins/lens/public/types.ts

@@ -1335,7 +1336,7 @@ export function isLensEditEvent<T extends LensEditSupportedActions>(

 export function isLensTableRowContextMenuClickEvent(
  event: ExpressionRendererEvent
-): event is BrushTriggerEvent {
+): event is LensTableRowContextMenuEvent {


dej611 · 2022-12-15T14:28:39Z

x-pack/plugins/lens/public/visualizations/datatable/expression.tsx

@@ -104,6 +129,11 @@ export const getDatatableRenderer = (dependencies: {
      }
    }

+    const columnCellValueActions = await getColumnCellValueActions(


can it be combined with the columnFilterable one?

…ue_actions

kibana-ci · 2022-12-16T11:28:57Z

💚 Build Succeeded

Buildkite Build
Commit: 8432fb8

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	3387	3395	+8

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`embeddable`	413	421	+8
`expressions`	1739	1746	+7
`lens`	598	599	+1
total			+16

Any counts in public APIs

Total count of every any typed public API. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats any for more detailed information.

id	before	after	diff
`embeddable`	6	8	+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`expressionPartitionVis`	52.4KB	52.9KB	+511.0B
`expressionXY`	118.8KB	119.2KB	+444.0B
`lens`	1.3MB	1.3MB	+1.4KB
`securitySolution`	10.1MB	12.6MB	⚠️ +2.5MB
total			⚠️ +2.5MB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`embeddable`	69.8KB	70.2KB	+406.0B
`expressionPartitionVis`	24.6KB	24.9KB	+367.0B
`expressions`	98.5KB	98.7KB	+155.0B
`expressionXY`	37.1KB	37.4KB	+342.0B
`lens`	30.5KB	30.5KB	+74.0B
`securitySolution`	50.4KB	51.0KB	+575.0B
total			+1.9KB

Unknown metric groups

API count

id	before	after	diff
`embeddable`	514	522	+8
`expressions`	2198	2205	+7
`lens`	694	695	+1
total			+16

async chunk count

id	before	after	diff
`securitySolution`	37	42	+5

ESLint disabled in files

id	before	after	diff
`osquery`	1	2	+1

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	19	21	+2
`fleet`	61	67	+6
`osquery`	109	115	+6
`securitySolution`	440	446	+6
total			+20

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	20	22	+2
`fleet`	70	76	+6
`osquery`	110	117	+7
`securitySolution`	517	523	+6
total			+21

History

💚 Build #95085 succeeded 7843c3f
💚 Build #94760 succeeded 585263c
💔 Build #94734 failed d2ba3d9
💚 Build #94643 succeeded f60c338
💚 Build #93976 succeeded 41cca66
💔 Build #93834 failed 020541f

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @semd

semd added 2 commits November 30, 2022 18:33

register security lens cellValue actions

4d41a54

add cell actions to visualizations

be16993

semd self-assigned this Dec 1, 2022

semd added 4 commits December 1, 2022 13:02

add to timeline display name

5877254

rollback dashboards table test

11bc30e

lens embeddable conflict solved

37475e5

fix types

8f1e3be

semd added the Feature:Visualizations Generic visualization features (in case no more specific feature label is available) label Dec 1, 2022

semd added 2 commits December 1, 2022 15:06

rollback LensRendererEvent type

0fee044

fix tests

39bed22

semd marked this pull request as ready for review December 1, 2022 17:50

semd requested review from a team as code owners December 1, 2022 17:50

unify security datagrid cellActions

6440f4f

semd requested review from a team as code owners December 2, 2022 11:54

botelastic bot added Feature:Embedding Embedding content via iFrame Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) labels Dec 2, 2022

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

34ebdf9

…-ref HEAD~1..HEAD --fix'

semd commented Dec 2, 2022

View reviewed changes

semd changed the title ~~[Lens] New trigger actions for chart legends and table cell actions~~ [Security Solution][Lens] New trigger actions for chart legends and table cell actions Dec 2, 2022

stephmilovic reviewed Dec 9, 2022

View reviewed changes

x-pack/plugins/security_solution/public/actions/add_to_timeline/add_to_timeline_action.ts Outdated Show resolved Hide resolved

stephmilovic reviewed Dec 9, 2022

View reviewed changes

x-pack/plugins/security_solution/public/actions/add_to_timeline/data_provider.ts Show resolved Hide resolved

stephmilovic reviewed Dec 9, 2022

View reviewed changes

x-pack/plugins/security_solution/public/actions/add_to_timeline/data_provider.ts Show resolved Hide resolved

semd added 3 commits December 12, 2022 12:11

remove comment

80d36cc

Merge remote-tracking branch 'upstream/main' into 145708_viz_data_val…

15334b2

…ue_actions

Merge branch '145708_viz_data_value_actions' of https://github.com/se…

41cca66

…md/kibana into 145708_viz_data_value_actions

ThomThomson approved these changes Dec 12, 2022

View reviewed changes

semd and others added 5 commits December 14, 2022 11:38

added interative conditionals + tests

f60c338

tests

dcff83d

Merge remote-tracking branch 'upstream/main' into 145708_viz_data_val…

d2ba3d9

…ue_actions

fix path change

f481439

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

585263c

…-fix'

stephmilovic approved these changes Dec 14, 2022

View reviewed changes

michaelolo24 reviewed Dec 14, 2022

View reviewed changes

michaelolo24 approved these changes Dec 14, 2022

View reviewed changes

semd added 3 commits December 15, 2022 16:11

encapsulate layerCellValueActions fuction

83aa17c

Merge branch '145708_viz_data_value_actions' of https://github.com/se…

f5ef5f6

…md/kibana into 145708_viz_data_value_actions

Merge remote-tracking branch 'upstream/main' into 145708_viz_data_val…

7843c3f

…ue_actions

dej611 approved these changes Dec 15, 2022

View reviewed changes

semd added 2 commits December 16, 2022 10:58

combine async calls

cd1bdbe

Merge remote-tracking branch 'upstream/main' into 145708_viz_data_val…

8432fb8

…ue_actions

semd merged commit d8318ed into elastic:main Dec 16, 2022

kibanamachine added v8.7.0 backport:skip This commit does not require backporting labels Dec 16, 2022

This was referenced Jan 12, 2023

[Security Solution] Research/POC if AddToTimelinesButton could be a package. #145664

Closed

[Security Solution][Tech debt][Epic] Redesign Security Solution hover/inline actions to make it extensible and reusable across the plugins. #144943

Closed

drewdaemon mentioned this pull request Oct 26, 2023

[Lens] create strategy for embeddable interaction customization #169983

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Lens] New trigger actions for chart legends and table cell actions #146779

[Security Solution][Lens] New trigger actions for chart legends and table cell actions #146779

semd commented Dec 1, 2022 •

edited

Loading

elasticmachine commented Dec 1, 2022

elasticmachine commented Dec 1, 2022

semd Dec 2, 2022

angorayc Dec 5, 2022

semd commented Dec 12, 2022

ThomThomson left a comment •

edited

Loading

semd commented Dec 12, 2022 •

edited

Loading

stephmilovic left a comment

michaelolo24 Dec 14, 2022

michaelolo24 left a comment

dej611 left a comment

dej611 Dec 15, 2022

dej611 Dec 15, 2022

dej611 Dec 15, 2022

dej611 Dec 15, 2022

kibana-ci commented Dec 16, 2022

API count

async chunk count

ESLint disabled in files

ESLint disabled line counts

Total ESLint disabled count

	// Hide EUI's 'Filter in' and 'Filter out' footer buttons - replaced with our own buttons
	.euiPopoverFooter:nth-child(2) {
	.euiFlexItem:first-child,
	.euiFlexItem:nth-child(2) {
	display: none;
	}
	}

	import { LayerCellValueActions } from '../types';
	import type { LayerCellValueActions } from '../types';

[Security Solution][Lens] New trigger actions for chart legends and table cell actions #146779

[Security Solution][Lens] New trigger actions for chart legends and table cell actions #146779

Conversation

semd commented Dec 1, 2022 • edited Loading

Summary

New Trigger

Table cell actions

Chart legend actions

Filter for & Filter out actions:

Security Solution actions

Security Solution Filter In/Out bug

Testing

Checklist

elasticmachine commented Dec 1, 2022

elasticmachine commented Dec 1, 2022

semd Dec 2, 2022

Choose a reason for hiding this comment

angorayc Dec 5, 2022

Choose a reason for hiding this comment

semd commented Dec 12, 2022

ThomThomson left a comment • edited Loading

Choose a reason for hiding this comment

semd commented Dec 12, 2022 • edited Loading

stephmilovic left a comment

Choose a reason for hiding this comment

michaelolo24 Dec 14, 2022

Choose a reason for hiding this comment

michaelolo24 left a comment

Choose a reason for hiding this comment

dej611 left a comment

Choose a reason for hiding this comment

dej611 Dec 15, 2022

Choose a reason for hiding this comment

dej611 Dec 15, 2022

Choose a reason for hiding this comment

dej611 Dec 15, 2022

Choose a reason for hiding this comment

dej611 Dec 15, 2022

Choose a reason for hiding this comment

kibana-ci commented Dec 16, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Any counts in public APIs

Async chunks

Page load bundle

API count

async chunk count

ESLint disabled in files

ESLint disabled line counts

Total ESLint disabled count

History

semd commented Dec 1, 2022 •

edited

Loading

ThomThomson left a comment •

edited

Loading

semd commented Dec 12, 2022 •

edited

Loading