[Security Solution][Endpoint][Guided onboarding] New extension point with the endpoint UI #142395
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -6,13 +6,124 @@ | |||||
| */ | ||||||
|
|
||||||
| import React, { memo } from 'react'; | ||||||
| import { | ||||||
| EuiText, | ||||||
| EuiIcon, | ||||||
| EuiFlexGroup, | ||||||
| EuiFlexItem, | ||||||
| EuiLink, | ||||||
| EuiPanel, | ||||||
| EuiSpacer, | ||||||
| useEuiTheme, | ||||||
| } from '@elastic/eui'; | ||||||
| import styled from 'styled-components'; | ||||||
| import { css } from '@emotion/css'; | ||||||
| import { FormattedMessage } from '@kbn/i18n-react'; | ||||||
| import { useKibana } from '../../../../../common/lib/kibana'; | ||||||
|
|
||||||
| const CenteredEuiFlexItem = styled(EuiFlexItem)` | ||||||
| align-items: center; | ||||||
| `; | ||||||
|
|
||||||
| /** | ||||||
| * A component to be displayed in multi step onboarding process. | ||||||
| */ | ||||||
| export const EndpointPolicyCreateMultiStepExtension = memo(() => { | ||||||
|
There was a problem hiding this comment. Can you move the final component at the end so we see first all the defined consts and finally the component that uses those? There was a problem hiding this comment. Also, for functions that returns a component or JSX code I think it would be better to define those inside the component so all are rendered on component render and not on module import level. What do you think? There was a problem hiding this comment.
Personally I like it better when the higher level components/functions are on the top of the file, so when one opens it, they can see the high level logic first, and if they want to see the details, they can go down in the file. But I see that your suggestion is the general style in our codebase, so of course! It's better to have an aligned code style. There was a problem hiding this comment.
Great idea! Usually I'd keep small components out of the main component, so the functional components are not re-created on every re-render, but in this case with a static component, I agree that favouring a faster module load is probably better. |
||||||
| const { docLinks } = useKibana().services; | ||||||
| const { size } = useEuiTheme().euiTheme; | ||||||
|
|
||||||
| const title = ( | ||||||
| <EuiText> | ||||||
| <h3> | ||||||
| <FormattedMessage | ||||||
| id="xpack.securitySolution.endpoint.policy.multiStepOnboarding.title" | ||||||
| defaultMessage="We'll save your integration with our recommended defaults." | ||||||
| /> | ||||||
| </h3> | ||||||
| </EuiText> | ||||||
| ); | ||||||
|
|
||||||
| const logoSize = `calc(2 * ${size.xxxxl})`; | ||||||
| const securityLogo = ( | ||||||
| <EuiIcon | ||||||
| type="logoSecurity" | ||||||
| css={css` | ||||||
| width: ${logoSize}; | ||||||
| height: ${logoSize}; | ||||||
| `} | ||||||
| /> | ||||||
| ); | ||||||
|
|
||||||
| const features = ( | ||||||
| <EuiFlexGroup alignItems="center" gutterSize="s" responsive={false}> | ||||||
| <EuiFlexItem grow={false}> | ||||||
| <EuiIcon type="check" /> | ||||||
| </EuiFlexItem> | ||||||
|
|
||||||
| <EuiFlexItem> | ||||||
| <EuiText size="m"> | ||||||
| <p> | ||||||
| <FormattedMessage | ||||||
| id="xpack.securitySolution.endpoint.policy.multiStepOnboarding.feature" | ||||||
| defaultMessage="Windows, Mac, and Linux event collection" | ||||||
|
There was a problem hiding this comment. Nit: "macOS" (capitalized as such) is the proper name of the operating system, and Apple usually uses "Mac" to refer to a piece of hardware
Suggested change
|
||||||
| /> | ||||||
| </p> | ||||||
| </EuiText> | ||||||
| </EuiFlexItem> | ||||||
| </EuiFlexGroup> | ||||||
| ); | ||||||
|
|
||||||
| const details = ( | ||||||
| <EuiText size="m" color="subdued"> | ||||||
| <p> | ||||||
| <FormattedMessage | ||||||
| id="xpack.securitySolution.endpoint.policy.multiStepOnboarding.details" | ||||||
| defaultMessage="You can change this later by editing the Endpoint Security integration agent policy. | ||||||
|
There was a problem hiding this comment. This is Janeen. Edit to @kellyemurphy 's suggestion because we updated the name to
Suggested change
|
||||||
| Read more about Endpoint security configuration in our {docsPage}." | ||||||
|
There was a problem hiding this comment. We can shorten this entire sentence to just "Learn more" that links to that doc page and a jumpout icon. |
||||||
| values={{ | ||||||
| docsPage: ( | ||||||
| <EuiLink | ||||||
| href={docLinks.links.securitySolution.configureEndpointIntegrationPolicy} | ||||||
| target="_blank" | ||||||
| external | ||||||
| > | ||||||
| <FormattedMessage | ||||||
| id="xpack.securitySolution.endpoint.policy.multiStepOnboarding.docsPage" | ||||||
| defaultMessage="documentation" | ||||||
| /> | ||||||
| </EuiLink> | ||||||
| ), | ||||||
| }} | ||||||
| /> | ||||||
| </p> | ||||||
| </EuiText> | ||||||
| ); | ||||||
|
|
||||||
| const marginSize = `calc(${size.m} + ${size.l})`; | ||||||
|
|
||||||
| return ( | ||||||
| <EuiPanel hasShadow={false} paddingSize="l"> | ||||||
| <EuiSpacer size="xl" /> | ||||||
|
|
||||||
| {title} | ||||||
|
|
||||||
| <EuiFlexGroup | ||||||
| css={css` | ||||||
| margin-bottom: ${marginSize}; | ||||||
| margin-top: ${marginSize}; | ||||||
|
There was a problem hiding this comment. Should be There was a problem hiding this comment. Cool, thanks! It also makes things easier, that's for sure. |
||||||
| `} | ||||||
| > | ||||||
| <CenteredEuiFlexItem grow={false}>{securityLogo}</CenteredEuiFlexItem> | ||||||
|
There was a problem hiding this comment. I don't think you need this additional center alignment. You can just use the There was a problem hiding this comment. For the given design, you are right! The center alignment helps with the mobile view. |
||||||
|
|
||||||
| <EuiFlexItem> | ||||||
| <div>{features}</div> | ||||||
| </EuiFlexItem> | ||||||
| </EuiFlexGroup> | ||||||
|
|
||||||
| {details} | ||||||
|
|
||||||
| <EuiSpacer size="xl" /> | ||||||
| </EuiPanel> | ||||||
| ); | ||||||
| }); | ||||||
| EndpointPolicyCreateMultiStepExtension.displayName = 'EndpointPolicyCreateMultiStepExtension'; | ||||||
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.
I would not remove these props as could be used in the future (or might be used for other integrations) when we add any kind of integration modification action.
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.
As we discussed, I'd leave them removed to avoid unneeded complexity, and put them back when they are really needed.