[Security Solution] Updates the text in the Actions log and Status command to match UX #135935

kevinlog · 2022-07-07T17:21:59Z

Summary

This PR updates the text in the Action log tray and status command to match the mocks and use a uniform text. Also readjusted some spacing for the help command after feedback from UX.

Action log:

Status command:

Readjusted help command spacing:

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

…ion log layout

elasticmachine · 2022-07-07T18:29:01Z

Pinging @elastic/security-onboarding-and-lifecycle-mgt (Team:Onboarding and Lifecycle Mgt)

kevinlog · 2022-07-07T20:57:12Z

@elasticmachine merge upstream

kevinlog · 2022-07-08T11:08:01Z

@elasticmachine merge upstream

dasansol92

This is looking great! 🔥 Left just minor suggestions

dasansol92 · 2022-07-08T12:00:06Z

x-pack/plugins/security_solution/public/common/components/endpoint/agent_status_text.tsx

+ */
+
+import { i18n } from '@kbn/i18n';
+import { HostStatus } from '../../../../common/endpoint/types';


Suggested change

import { HostStatus } from '../../../../common/endpoint/types';

import type { HostStatus } from '../../../../common/endpoint/types';

dasansol92 · 2022-07-08T12:01:55Z

x-pack/plugins/security_solution/public/common/components/endpoint/agent_status_text.tsx

+import { i18n } from '@kbn/i18n';
+import { HostStatus } from '../../../../common/endpoint/types';
+
+export const agentStatusText = (hostStatus: HostStatus) => {


Since there is no JSX code here this file can be a .ts extension.

Name suggestion: getAgentStatusText

dasansol92 · 2022-07-08T12:03:40Z

.../plugins/security_solution/public/management/components/console/components/command_usage.tsx

@@ -129,7 +130,7 @@ export const CommandUsage = memo<CommandUsageProps>(({ commandDef }) => {
  }, [commandDef.args, hasArgs]);

  const parametersDescriptionList = (title: string, parameters: CommandDetails) => {
-    const description = parameters.map((item) => (
+    const description = parameters.map((item, index) => (


index is not needed here

dasansol92 · 2022-07-08T12:06:02Z

.../plugins/security_solution/public/management/components/endpoint_responder/status_action.tsx

-            />
-          </EuiText>
-          <EuiText>
+  const statusDescriptionList = () => {


I would suggest you move this inside a useCallback and change the name by something like getStatusDescriptionList

dasansol92 · 2022-07-08T12:07:38Z

.../plugins/security_solution/public/management/components/endpoint_responder/status_action.tsx

+      }`;
+    };
+
+    const statusData = [


This has also a dynamic result, move it in a useMemo will avoid unnecessary code execution during renders.

@dasansol92 if I wrap the containing function into the useCallBack() and the deps array contains the dynamic data, won't this be taken care of? If I put this as useMemo() we'd have a hook in a hook.

Ohh! You are totally right! Github diff hided me the useCallback. No need for useMemo here then. Apologies for the noise.

dasansol92 · 2022-07-08T12:10:31Z

...lution/public/management/components/endpoint_response_actions_list/response_actions_list.tsx

@@ -164,38 +168,39 @@ export const ResponseActionsList = memo<
          : undefined;

        const command = getCommand(_command);
-        const descriptionListLeft = [
+        const dataList = [


This and outputList have a dynamic result, you can use useMemo for those.

same for these, they're already wrapped in a useCallBack() with a deps array, would it still be necessary to wrap these in useMemo() ?

…ion-log-text

x-pack/plugins/security_solution/public/management/components/console/console.tsx

ashokaditya · 2022-07-08T13:18:11Z

...lution/public/management/components/endpoint_response_actions_list/response_actions_list.tsx

  }
  dt {
    font-weight: ${(props) => props.theme.eui.euiFontWeightSemiBold};
+    margin-top: ${(props) => props.theme.eui.euiSizeS} !important;


This line is redundant. line 63 already covers both dt and dd

@ashokaditya see this commit: 2bd50b7#diff-56e3d3d36e2892512e4de0a61b7e79f3a5855544e3ec6adb480c85fec616fde2R60

I refactored this a bit to remove the need for !important

kevinlog · 2022-07-08T16:32:33Z

@elasticmachine merge upstream

dasansol92

LGTM, thanks for the changes!

…ion-log-text

…nlog/kibana into task/status-action-log-text

…-fix'

x-pack/plugins/security_solution/public/common/components/endpoint/agent_status.tsx

paul-tavares · 2022-07-11T12:45:37Z

x-pack/plugins/security_solution/public/common/components/endpoint/agent_status_text.ts

+import { i18n } from '@kbn/i18n';
+import type { HostStatus } from '../../../../common/endpoint/types';
+
+export const getAgentStatusText = (hostStatus: HostStatus) => {


Ok. I see what you did here... When I saw getAgentStatusText() I though you were using the Fleet helper function.

Since this is under common/components/, can you make it a component? as in:

<EndpointAgentStatusText hostStatus="" />

My original intention was just to have this generate the text for the status which can be used either in plain text reporting or in a containing component like it is here: https://github.com/elastic/kibana/pull/135935/files#diff-dcba59229f5aeb7b5e1cc30e8b37d4b848460e6e240da640eac479b392e28939R21

Thought - are you OK if I export this text from the above file so that we can either import the text or the entire AgentStatus component based on the need?

yeah, that's fine - nothing that should hold this up. It just feels more like a "service" or "utility" than a component, so I was just highlighting that it probably should not be under component/

paul-tavares · 2022-07-11T13:02:59Z

.../plugins/security_solution/public/management/components/endpoint_responder/status_action.tsx

@@ -115,6 +115,114 @@ export const EndpointStatusActionResult = memo<
    }
  }, [fetchedPendingActionsSummary, setStore]);

+  const getStatusDescriptionList = useCallback(() => {


just FYI: This would be more efficient if it was a useMemo() that would only be updated if the dependencies changed. The way it is now, its called every time there is a re-render of the component

paul-tavares · 2022-07-11T13:13:48Z

.../plugins/security_solution/public/management/components/endpoint_responder/status_action.tsx

+    const agentStatus = () => {
+      let isolateStatus = '';
+
+      if (pendingIsolationActions.pendingIsolate > 0) {


Let's not hold up on this PR, but this logic here for actions no longer makes sense now that we introduced more actions. Not sure that isolateStatus should be set to isolating or releasing when there are other actions also pending.

(FYI: I also found that our component that normally show the isolation status/pending actions is not showing all pending actions in the tooltip - opening issue now)

paul-tavares

Checked out PR and looked over status command. Looks good 👍

kibana-ci · 2022-07-11T13:47:56Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	3115	3114	-1

Async chunks

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

id	before	after	diff
`securitySolution`	5.2MB	5.2MB	-465.0B

History

💚 Build #56363 succeeded 6930af9
💔 Build #56274 failed 788df06
💔 Build #56238 failed 84a9842
💔 Build #56104 failed 013cdd0
💔 Build #56085 failed 9b26a65
💔 Build #56061 failed 8244a73

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

…mmand to match UX (elastic#135935)

refs elastic/pull/135935

kevinlog added 4 commits July 7, 2022 12:04

[Security Solution] Update text and layout for status command and act…

0a2773a

…ion log layout

update action log tray

ee6c089

spacing adjustments

681e121

fix check

8244a73

kevinlog changed the title ~~Task/status action log text~~ [Security Solution] Updates the text in the Actions log and Status command to match UX Jul 7, 2022

kevinlog marked this pull request as ready for review July 7, 2022 18:28

kevinlog requested a review from a team as a code owner July 7, 2022 18:28

kevinlog requested review from dasansol92 and parkiino July 7, 2022 18:28

kevinlog added the Team:Defend Workflows “EDR Workflows” sub-team of Security Solution label Jul 7, 2022

kevinlog added v8.4.0 release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting labels Jul 7, 2022

kevinlog requested a review from ashokaditya July 7, 2022 18:29

adjust spacing for help and errors

9b26a65

Merge branch 'main' into task/status-action-log-text

013cdd0

Merge branch 'main' into task/status-action-log-text

84a9842

dasansol92 reviewed Jul 8, 2022

View reviewed changes

Merge branch 'main' of github.com:elastic/kibana into task/status-act…

788df06

…ion-log-text

ashokaditya reviewed Jul 8, 2022

View reviewed changes

pr comments, snapshot test

2bd50b7

kevinlog requested a review from a team as a code owner July 8, 2022 13:57

ashokaditya approved these changes Jul 8, 2022

View reviewed changes

janmonschke approved these changes Jul 8, 2022

View reviewed changes

Merge branch 'main' into task/status-action-log-text

6930af9

dasansol92 approved these changes Jul 11, 2022

View reviewed changes

kevinlog added 2 commits July 11, 2022 07:40

Merge branch 'main' of github.com:elastic/kibana into task/status-act…

c39f25b

…ion-log-text

Merge branch 'task/status-action-log-text' of https://github.com/kevi…

2fef0af

…nlog/kibana into task/status-action-log-text

kevinlog enabled auto-merge (squash) July 11, 2022 11:40

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

107effc

…-fix'

paul-tavares reviewed Jul 11, 2022

View reviewed changes

x-pack/plugins/security_solution/public/common/components/endpoint/agent_status.tsx Show resolved Hide resolved

paul-tavares reviewed Jul 11, 2022

View reviewed changes

paul-tavares approved these changes Jul 11, 2022

View reviewed changes

kevinlog merged commit c7f5a20 into elastic:main Jul 11, 2022

justinkambic pushed a commit to justinkambic/kibana that referenced this pull request Jul 13, 2022

[Security Solution] Updates the text in the Actions log and Status co…

06613f6

…mmand to match UX (elastic#135935)

ashokaditya added a commit to ashokaditya/kibana that referenced this pull request Mar 7, 2023

Align last seen item

68356ac

refs elastic/pull/135935

ashokaditya added a commit to ashokaditya/kibana that referenced this pull request Mar 7, 2023

Align last seen item

ec6cad2

refs elastic/pull/135935

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution] Updates the text in the Actions log and Status command to match UX #135935

[Security Solution] Updates the text in the Actions log and Status command to match UX #135935

kevinlog commented Jul 7, 2022 •

edited

Loading

elasticmachine commented Jul 7, 2022

kevinlog commented Jul 7, 2022

kevinlog commented Jul 8, 2022

dasansol92 left a comment

dasansol92 Jul 8, 2022

dasansol92 Jul 8, 2022

dasansol92 Jul 8, 2022

dasansol92 Jul 8, 2022

dasansol92 Jul 8, 2022

dasansol92 Jul 8, 2022

kevinlog Jul 8, 2022

dasansol92 Jul 11, 2022

dasansol92 Jul 8, 2022

kevinlog Jul 8, 2022

ashokaditya Jul 8, 2022

kevinlog Jul 8, 2022

kevinlog commented Jul 8, 2022

dasansol92 left a comment

paul-tavares Jul 11, 2022

kevinlog Jul 11, 2022

paul-tavares Jul 11, 2022

paul-tavares Jul 11, 2022

paul-tavares Jul 11, 2022

paul-tavares left a comment

kibana-ci commented Jul 11, 2022

	import { HostStatus } from '../../../../common/endpoint/types';
	import type { HostStatus } from '../../../../common/endpoint/types';

[Security Solution] Updates the text in the Actions log and Status command to match UX #135935

[Security Solution] Updates the text in the Actions log and Status command to match UX #135935

Conversation

kevinlog commented Jul 7, 2022 • edited Loading

Summary

Checklist

elasticmachine commented Jul 7, 2022

kevinlog commented Jul 7, 2022

kevinlog commented Jul 8, 2022

dasansol92 left a 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

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

kevinlog commented Jul 8, 2022

dasansol92 left a 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

paul-tavares left a comment

Choose a reason for hiding this comment

kibana-ci commented Jul 11, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

kevinlog commented Jul 7, 2022 •

edited

Loading