[RAC][Analyzer] Make analyzer full screen mode compatible with EuiDataGrid's #110723

kqualters-elastic · 2021-08-31T21:34:55Z

Summary

This pr enables the analyzer to work with EuiDataGrid full screen mode, by making use of EuiMutationObserver, a query selector to see when the data grid is actually expanded, and some artificial click events to open/close EuiDataGrid's full screen mode as appropriate when a user is using resolver.

Checklist

Delete any items that are not applicable to this PR.

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 does not create any new axe failures (run axe in browser: FF, Chrome)

elasticmachine · 2021-08-31T21:34:57Z

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

XavierM · 2021-09-01T01:25:37Z

x-pack/plugins/security_solution/public/plugin.tsx

@@ -147,6 +147,9 @@ export class Plugin implements IPlugin<PluginSetup, PluginStart, SetupPlugins, S
        const [coreStart, startPlugins] = await core.getStartServices();
        const subPlugins = await this.startSubPlugins(this.storage, coreStart, startPlugins);
        const { renderApp } = await this.lazyApplicationDependencies();
+        if (startPlugins.timelines && this._store) {


we do not need that anymore because of this commit da68c5d

XavierM

is this still working as expected in timeline fly-out?

kqualters-elastic · 2021-09-01T01:51:14Z

is this still working as expected in timeline fly-out?

yes

kibanamachine · 2021-09-01T15:35:53Z

💛 Build succeeded, but was flaky

continuous-integration/kibana-ci/pull-request
Commit: 8daf073
Storybooks Preview
Flaky suites:
- xpack-kibana-ciGroup5

Test Failures

Kibana Pipeline / general / X-Pack Alerting API Integration Tests.x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/find·ts.alerting api integration security and spaces enabled Alerts alerts find "after each" hook for "should handle find alert request with fields appropriately"

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

[00:00:00]       │
[00:00:00]         └-: alerting api integration security and spaces enabled
[00:00:00]           └-> "before all" hook in "alerting api integration security and spaces enabled"
[00:03:59]           └-: Alerts
[00:03:59]             └-> "before all" hook in "Alerts"
[00:05:31]             └-: alerts
[00:05:31]               └-> "before all" hook in "alerts"
[00:05:31]               └-> "before all" hook in "alerts"
[00:05:31]                 │ debg creating space
[00:05:32]                 │ debg created space
[00:05:32]                 │ debg creating space
[00:05:33]                 │ debg created space
[00:05:33]                 │ debg creating space
[00:05:34]                 │ debg created space
[00:05:34]                 │ debg creating user no_kibana_privileges
[00:05:34]                 │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [no_kibana_privileges]
[00:05:34]                 │ debg created user no_kibana_privileges
[00:05:34]                 │ debg creating role no_kibana_privileges
[00:05:34]                 │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [no_kibana_privileges]
[00:05:34]                 │ debg creating user superuser
[00:05:34]                 │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [superuser]
[00:05:34]                 │ debg created user superuser
[00:05:34]                 │ debg creating user global_read
[00:05:34]                 │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [global_read]
[00:05:34]                 │ debg created user global_read
[00:05:34]                 │ debg creating role global_read_role
[00:05:34]                 │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [global_read_role]
[00:05:34]                 │ debg creating user space_1_all
[00:05:34]                 │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [space_1_all]
[00:05:34]                 │ debg created user space_1_all
[00:05:34]                 │ debg creating role space_1_all_role
[00:05:34]                 │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [space_1_all_role]
[00:05:34]                 │ debg creating user space_1_all_with_restricted_fixture
[00:05:35]                 │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [space_1_all_with_restricted_fixture]
[00:05:35]                 │ debg created user space_1_all_with_restricted_fixture
[00:05:35]                 │ debg creating role space_1_all_with_restricted_fixture_role
[00:05:35]                 │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [space_1_all_with_restricted_fixture_role]
[00:05:35]                 │ debg creating user space_1_all_alerts_none_actions
[00:05:35]                 │ info [o.e.x.s.a.u.TransportPutUserAction] [node-01] added user [space_1_all_alerts_none_actions]
[00:05:35]                 │ debg created user space_1_all_alerts_none_actions
[00:05:35]                 │ debg creating role space_1_all_alerts_none_actions_role
[00:05:35]                 │ info [o.e.x.s.a.r.TransportPutRoleAction] [node-01] added role [space_1_all_alerts_none_actions_role]
[00:05:35]               └-: find
[00:05:35]                 └-> "before all" hook in "find"

Stack Trace

Error: expected 204 "No Content", got 409 "Conflict"
    at Test._assertStatus (/dev/shm/workspace/parallel/16/kibana/node_modules/supertest/lib/test.js:268:12)
    at Test._assertFunction (/dev/shm/workspace/parallel/16/kibana/node_modules/supertest/lib/test.js:283:11)
    at Test.assert (/dev/shm/workspace/parallel/16/kibana/node_modules/supertest/lib/test.js:173:18)
    at assert (/dev/shm/workspace/parallel/16/kibana/node_modules/supertest/lib/test.js:131:12)
    at /dev/shm/workspace/parallel/16/kibana/node_modules/supertest/lib/test.js:128:5
    at Test.Request.callback (/dev/shm/workspace/parallel/16/kibana/node_modules/supertest/node_modules/superagent/lib/node/index.js:718:3)
    at /dev/shm/workspace/parallel/16/kibana/node_modules/supertest/node_modules/superagent/lib/node/index.js:906:18
    at IncomingMessage.<anonymous> (/dev/shm/workspace/parallel/16/kibana/node_modules/supertest/node_modules/superagent/lib/node/parsers/json.js:19:7)
    at endReadableNT (internal/streams/readable.js:1317:12)
    at processTicksAndRejections (internal/process/task_queues.js:82:21)

Metrics [docs]

Async chunks

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

id	before	after	diff
`securitySolution`	6.5MB	6.5MB	+4.6KB
`timelines`	420.4KB	421.2KB	+810.0B
total			+5.4KB

History

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

XavierM · 2021-09-01T18:40:09Z

x-pack/plugins/security_solution/public/common/containers/use_full_screen/index.tsx

+  const onMutation = useCallback(() => {
+    const isExpanded = document.querySelector('.euiDataGrid--fullScreen') !== null;
+    setDataGridExpanded(isExpanded);
+  }, [setDataGridExpanded]);


i think you should be able to remove this setter as a dependency because it is coming from useState

XavierM · 2021-09-01T18:41:50Z

x-pack/plugins/timelines/public/components/t_grid/body/index.tsx

@@ -734,7 +734,6 @@ export const BodyComponent = React.memo<StatefulBodyProps>(
    );

    const height = useDataGridHeightHack(pageSize, data.length);
-


nit -> we can avoid a change file here

stephmilovic · 2021-09-01T18:43:50Z

x-pack/plugins/security_solution/public/common/containers/use_full_screen/index.tsx

@@ -29,6 +29,25 @@ export const resetScroll = () => {
  }, 0);
 };

+export const setDataGridFullScreen = (isExpanded: boolean) => {
+  const clickEvent = new KeyboardEvent('click', {


can you please put a // TODO with a link to the Eui issue so we remember to come back and refactor once a method is available. @chandlerprall could you please help us find an eui issue?

elastic/eui#5126

XavierM · 2021-09-01T18:47:09Z

x-pack/plugins/timelines/public/components/t_grid/integrated/index.tsx

+                gutterSize="none"
+              >
+                <ScrollableFlexItem grow={1}>
+                  {nonDeletedEvents.length === 0 && loading === false ? (


Why not still using the same logic than before with totalCountMinusDeleted

sorry bad merge will fix

XavierM · 2021-09-01T18:48:09Z

x-pack/plugins/timelines/public/components/t_grid/integrated/index.tsx

-                    }
-                  />
-                )}
-                {totalCountMinusDeleted > 0 && (


we did that to avoid the EuiDatagrid to render with no data because we were getting a weird UX with the EuiDatagrid when we did not have data

XavierM · 2021-09-01T18:56:32Z

x-pack/plugins/timelines/public/components/t_grid/integrated/index.tsx

-                )}
-            </UpdatedFlexGroup>
-
-            {!graphEventId && graphOverlay == null && (


you do not want this condition anymore???

XavierM · 2021-09-01T19:14:39Z

x-pack/plugins/security_solution/public/common/containers/use_full_screen/index.tsx

+    cancelable: true,
+  });
+  const dataGridIsFullScreen = document.querySelector('.euiDataGrid--fullScreen') !== null;
+  const dataGridFullScreenButtonNew = document.querySelector(


my hacky sense love it, however my gray hair are telling me to not go for it and just ask EUI to create a callback on the full screen action. I really appreciate the ingenuity here.

I am sorry if I disappoint you but a safer bet it is to go with @stephmilovic 's PR to just disable the full screen on the analyzer for now.

@paulewing are you ok with that?

kqualters-elastic · 2021-09-01T20:11:17Z

closing this for an infinitely simpler way of doing the same thing 😆 😬

kqualters-elastic added 3 commits August 31, 2021 11:23

Make analyzer work with EuiDataGrid full screen

4ac37aa

Set EuiDataGrid height via mutation observer instead of 50ms setTimeout

fc0a932

Use previous hack over MutationObserver

fc19c16

kqualters-elastic requested review from a team as code owners August 31, 2021 21:34

Fix failing test

df8154d

XavierM reviewed Sep 1, 2021

View reviewed changes

kqualters-elastic added 3 commits August 31, 2021 21:55

Remove duplicated code w/Xavier, fix test

278b4fe

Don't open EuiDataGrid when opening fullscreen popover timeline

bf54b4f

Merge branch 'master' into analyzer-full-screen

8daf073

XavierM reviewed Sep 1, 2021

View reviewed changes

stephmilovic reviewed Sep 1, 2021

View reviewed changes

XavierM reviewed Sep 1, 2021

View reviewed changes

chandlerprall mentioned this pull request Sep 1, 2021

[EuiDataGrid] allow application to control fullscreen state elastic/eui#5126

Closed

kqualters-elastic closed this Sep 1, 2021

kqualters-elastic deleted the analyzer-full-screen branch September 1, 2021 20:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RAC][Analyzer] Make analyzer full screen mode compatible with EuiDataGrid's #110723

[RAC][Analyzer] Make analyzer full screen mode compatible with EuiDataGrid's #110723

kqualters-elastic commented Aug 31, 2021

elasticmachine commented Aug 31, 2021

XavierM Sep 1, 2021

XavierM left a comment

kqualters-elastic commented Sep 1, 2021 •

edited

Loading

kibanamachine commented Sep 1, 2021

Standard Out

Stack Trace

XavierM Sep 1, 2021 •

edited

Loading

XavierM Sep 1, 2021

stephmilovic Sep 1, 2021

stephmilovic Sep 1, 2021

chandlerprall Sep 1, 2021

XavierM Sep 1, 2021

kqualters-elastic Sep 1, 2021

XavierM Sep 1, 2021

XavierM Sep 1, 2021

XavierM Sep 1, 2021 •

edited

Loading

XavierM Sep 1, 2021

kqualters-elastic commented Sep 1, 2021

		@@ -734,7 +734,6 @@ export const BodyComponent = React.memo<StatefulBodyProps>(
		);

		const height = useDataGridHeightHack(pageSize, data.length);

[RAC][Analyzer] Make analyzer full screen mode compatible with EuiDataGrid's #110723

[RAC][Analyzer] Make analyzer full screen mode compatible with EuiDataGrid's #110723

Conversation

kqualters-elastic commented Aug 31, 2021

Summary

Checklist

elasticmachine commented Aug 31, 2021

Choose a reason for hiding this comment

XavierM left a comment

Choose a reason for hiding this comment

kqualters-elastic commented Sep 1, 2021 • edited Loading

kibanamachine commented Sep 1, 2021

💛 Build succeeded, but was flaky

Standard Out

Stack Trace

Metrics [docs]

Async chunks

History

XavierM Sep 1, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XavierM Sep 1, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kqualters-elastic commented Sep 1, 2021

kqualters-elastic commented Sep 1, 2021 •

edited

Loading

XavierM Sep 1, 2021 •

edited

Loading

XavierM Sep 1, 2021 •

edited

Loading