github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-04-24 09:48:58 -04:00
Code Issues Projects Releases Packages Wiki Activity

[Security Solution] Eliminates a redundant external link icon (#94194) (#94388)

Browse source 
## [Security Solution] Eliminates a redundant external link icon

- Fixes an issue where [a redundant external link icon](https://github.com/elastic/kibana/issues/89084) was rendered next to port numbers

Per the [EuiLink documentation](https://elastic.github.io/eui/#/navigation/link), it's no longer necessary to render our own icon, because `EuiLink` will automatically display one when `target="_blank"` is passed as a prop to the link.

- Updates the existing link icon unit test such that it asserts a specific icon count to catch any regressions

### Before

<img width="1673" alt="before" src="https://user-images.githubusercontent.com/4459398/110530119-4cd0ac00-80d7-11eb-9d54-5d6656491e69.png">

### After

<img width="1677" alt="after" src="https://user-images.githubusercontent.com/4459398/110530165-5c4ff500-80d7-11eb-99a3-68741fab9218.png">

### Desk testing

Desk tested in:

- Chrome `89.0.4389.82`
- Firefox `86.0`
- Safari `14.0.3`
This commit is contained in:
Andrew Goldstein 2021-03-10 19:31:29 -07:00 • committed by GitHub
parent 3e3b61b148
commit 623f43cb8c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 34 additions and 158 deletions
Download patch file Download diff file Expand all files Collapse all files

76
x-pack/plugins/security_solution/public/common/components/external_link_icon/index.test.tsx
View file

@ -1,76 +0,0 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { mount } from 'enzyme';
import React from 'react';
import { TestProviders } from '../../mock';
import { ExternalLinkIcon } from '.';
describe('Duration', () => {
test('it renders expected icon type when the leftMargin prop is not specified', () => {
const wrapper = mount(
<TestProviders>
<ExternalLinkIcon />
</TestProviders>
);
expect(wrapper.find('[data-test-subj="external-link-icon"]').first().props().type).toEqual(
'popout'
);
});
test('it renders expected icon type when the leftMargin prop is true', () => {
const wrapper = mount(
<TestProviders>
<ExternalLinkIcon leftMargin={true} />
</TestProviders>
);
expect(wrapper.find('[data-test-subj="external-link-icon"]').first().props().type).toEqual(
'popout'
);
});
test('it applies a margin-left style when the leftMargin prop is true', () => {
const wrapper = mount(
<TestProviders>
<ExternalLinkIcon leftMargin={true} />
</TestProviders>
);
expect(wrapper.find('[data-test-subj="external-link-icon"]').first()).toHaveStyleRule(
'margin-left',
'5px'
);
});
test('it does NOT apply a margin-left style when the leftMargin prop is false', () => {
const wrapper = mount(
<TestProviders>
<ExternalLinkIcon leftMargin={false} />
</TestProviders>
);
expect(wrapper.find('[data-test-subj="external-link-icon"]').first()).not.toHaveStyleRule(
'margin-left'
);
});
test('it renders expected icon type when the leftMargin prop is false', () => {
const wrapper = mount(
<TestProviders>
<ExternalLinkIcon leftMargin={true} />
</TestProviders>
);
expect(wrapper.find('[data-test-subj="external-link-icon"]').first().props().type).toEqual(
'popout'
);
});
});

48
x-pack/plugins/security_solution/public/common/components/external_link_icon/index.tsx
View file

@ -1,48 +0,0 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { EuiIcon } from '@elastic/eui';
import React from 'react';
import styled from 'styled-components';
const LinkIcon = styled(EuiIcon)`
position: relative;
top: -2px;
`;
LinkIcon.displayName = 'LinkIcon';
const LinkIconWithMargin = styled(LinkIcon)`
margin-left: 5px;
`;
LinkIconWithMargin.displayName = 'LinkIconWithMargin';
const color = 'subdued';
const iconSize = 's';
const iconType = 'popout';
/**
* Renders an icon that indicates following the hyperlink will navigate to
* content external to the app
*/
export const ExternalLinkIcon = React.memo<{
leftMargin?: boolean;
}>(({ leftMargin = true }) =>
leftMargin ? (
<LinkIconWithMargin
color={color}
data-test-subj="external-link-icon"
size={iconSize}
type={iconType}
/>
) : (
<LinkIcon color={color} data-test-subj="external-link-icon" size={iconSize} type={iconType} />
)
);
ExternalLinkIcon.displayName = 'ExternalLinkIcon';

16
x-pack/plugins/security_solution/public/common/components/links/index.test.tsx
View file

@ -5,7 +5,7 @@
* 2.0.
*/
import { mount, shallow, ShallowWrapper } from 'enzyme';
import { mount, shallow, ReactWrapper, ShallowWrapper } from 'enzyme';
import React from 'react';
import { removeExternalLinkText } from '../../../../common/test_utils';
import { mountWithIntl } from '@kbn/test/jest';
@ -121,11 +121,11 @@ describe('Custom Links', () => {
describe('External Link', () => {
const mockLink = 'https://www.virustotal.com/gui/search/';
const mockLinkName = 'Link';
let wrapper: ShallowWrapper;
let wrapper: ReactWrapper | ShallowWrapper;
describe('render', () => {
beforeAll(() => {
wrapper = shallow(
wrapper = mount(
<ExternalLink url={mockLink} idx={0} allItemsLimit={5} overflowIndexStart={5}>
{mockLinkName}
</ExternalLink>
@ -137,11 +137,13 @@ describe('Custom Links', () => {
});
test('it renders ExternalLinkIcon', () => {
expect(wrapper.find('[data-test-subj="externalLinkIcon"]').exists()).toBeTruthy();
expect(wrapper.find('span [data-euiicon-type="popout"]').length).toBe(1);
});
test('it renders correct url', () => {
expect(wrapper.find('[data-test-subj="externalLink"]').prop('href')).toEqual(mockLink);
expect(wrapper.find('[data-test-subj="externalLink"]').first().prop('href')).toEqual(
mockLink
);
});
test('it renders comma if id is given', () => {
@ -435,14 +437,14 @@ describe('Custom Links', () => {
test('it renders correct number of external icons by default', () => {
const wrapper = mountWithIntl(<ReputationLink domain={'192.0.2.0'} />);
expect(wrapper.find('[data-test-subj="externalLinkIcon"]')).toHaveLength(5);
expect(wrapper.find('span [data-euiicon-type="popout"]')).toHaveLength(5);
});
test('it renders correct number of external icons', () => {
const wrapper = mountWithIntl(
<ReputationLink domain={'192.0.2.0'} overflowIndexStart={1} />
);
expect(wrapper.find('[data-test-subj="externalLinkIcon"]')).toHaveLength(1);
expect(wrapper.find('span [data-euiicon-type="popout"]')).toHaveLength(1);
});
});
});

37
x-pack/plugins/security_solution/public/common/components/links/index.tsx
View file

@ -39,7 +39,6 @@ import {
} from '../../../../common/search_strategy/security_solution/network';
import { useUiSetting$, useKibana } from '../../lib/kibana';
import { isUrlInvalid } from '../../utils/validators';
import { ExternalLinkIcon } from '../external_link_icon';
import * as i18n from './translations';
import { SecurityPageName } from '../../../app/types';
@ -54,6 +53,13 @@ export const LinkAnchor: React.FC<EuiLinkProps> = ({ children, ...props }) => (
<EuiLink {...props}>{children}</EuiLink>
);
export const PortContainer = styled.div`
& svg {
position: relative;
top: -1px;
}
`;
// Internal Links
const HostDetailsLinkComponent: React.FC<{
children?: React.ReactNode;
@ -112,11 +118,12 @@ export const ExternalLink = React.memo<{
const inAllowlist = allowedUrlSchemes.some((scheme) => url.indexOf(scheme) === 0);
return url && inAllowlist && !isUrlInvalid(url) && children ? (
<EuiToolTip content={url} position="top" data-test-subj="externalLinkTooltip">
<EuiLink href={url} target="_blank" rel="noopener" data-test-subj="externalLink">
{children}
<ExternalLinkIcon data-test-subj="externalLinkIcon" />
<>
<EuiLink href={url} target="_blank" rel="noopener" data-test-subj="externalLink">
{children}
</EuiLink>
{!isNil(idx) && idx < lastIndexToShow && <Comma data-test-subj="externalLinkComma" />}
</EuiLink>
</>
</EuiToolTip>
) : null;
}
@ -229,15 +236,17 @@ export const PortOrServiceNameLink = React.memo<{
children?: React.ReactNode;
portOrServiceName: number | string;
}>(({ children, portOrServiceName }) => (
<EuiLink
data-test-subj="port-or-service-name-link"
href={`https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml?search=${encodeURIComponent(
String(portOrServiceName)
)}`}
target="_blank"
>
{children ? children : portOrServiceName}
</EuiLink>
<PortContainer>
<EuiLink
data-test-subj="port-or-service-name-link"
href={`https://www.iana.org/assignments/service-names-port-numbers/service-names-port-numbers.xhtml?search=${encodeURIComponent(
String(portOrServiceName)
)}`}
target="_blank"
>
{children ? children : portOrServiceName}
</EuiLink>
</PortContainer>
));
PortOrServiceNameLink.displayName = 'PortOrServiceNameLink';

1
x-pack/plugins/security_solution/public/network/components/port/__snapshots__/index.test.tsx.snap generated
View file

@ -11,6 +11,5 @@ exports[`Port renders correctly against snapshot 1`] = `
<PortOrServiceNameLink
portOrServiceName="443"
/>
<ExternalLinkIcon />
</DefaultDraggable>
`;

4
x-pack/plugins/security_solution/public/network/components/port/index.test.tsx
View file

@ -60,13 +60,13 @@ describe('Port', () => {
);
});
test('it renders an external link', () => {
test('it renders only one external link icon', () => {
const wrapper = mount(
<TestProviders>
<Port contextId="test" eventId="abcd" fieldName="destination.port" value="443" />
</TestProviders>
);
expect(wrapper.find('[data-test-subj="external-link-icon"]').first().exists()).toBe(true);
expect(wrapper.find('span [data-euiicon-type="popout"]').length).toBe(1);
});
});

2
x-pack/plugins/security_solution/public/network/components/port/index.tsx
View file

@ -9,7 +9,6 @@ import React from 'react';
import { DefaultDraggable } from '../../../common/components/draggables';
import { getEmptyValue } from '../../../common/components/empty_value';
import { ExternalLinkIcon } from '../../../common/components/external_link_icon';
import { PortOrServiceNameLink } from '../../../common/components/links';
export const CLIENT_PORT_FIELD_NAME = 'client.port';
@ -40,7 +39,6 @@ export const Port = React.memo<{
value={value}
>
<PortOrServiceNameLink portOrServiceName={value || getEmptyValue()} />
<ExternalLinkIcon />
</DefaultDraggable>
));

2
x-pack/plugins/security_solution/public/timelines/components/certificate_fingerprint/index.tsx
View file

@ -10,7 +10,6 @@ import React from 'react';
import styled from 'styled-components';
import { DraggableBadge } from '../../../common/components/draggables';
import { ExternalLinkIcon } from '../../../common/components/external_link_icon';
import { CertificateFingerprintLink } from '../../../common/components/links';
import * as i18n from './translations';
@ -61,7 +60,6 @@ export const CertificateFingerprint = React.memo<{
{certificateType === 'client' ? i18n.CLIENT_CERT : i18n.SERVER_CERT}
</FingerprintLabel>
<CertificateFingerprintLink certificateFingerprint={value || ''} />
<ExternalLinkIcon />
</DraggableBadge>
);
});

2
x-pack/plugins/security_solution/public/timelines/components/ja3_fingerprint/index.tsx
View file

@ -9,7 +9,6 @@ import React from 'react';
import styled from 'styled-components';
import { DraggableBadge } from '../../../common/components/draggables';
import { ExternalLinkIcon } from '../../../common/components/external_link_icon';
import { Ja3FingerprintLink } from '../../../common/components/links';
import * as i18n from './translations';
@ -45,7 +44,6 @@ export const Ja3Fingerprint = React.memo<{
{i18n.JA3_FINGERPRINT_LABEL}
</Ja3FingerprintLabel>
<Ja3FingerprintLink data-test-subj="ja3-hash-link" ja3Fingerprint={value || ''} />
<ExternalLinkIcon />
</DraggableBadge>
));

2
x-pack/plugins/security_solution/public/timelines/components/row_renderers_browser/catalog/index.tsx
View file

@ -7,7 +7,6 @@
import { EuiLink } from '@elastic/eui';
import React from 'react';
import { ExternalLinkIcon } from '../../../../common/components/external_link_icon';
import { RowRendererId } from '../../../../../common/types/timeline';
import {
@ -37,7 +36,6 @@ const Link = ({ children, url }: { children: React.ReactNode; url: string }) =>
data-test-subj="externalLink"
>
{children}
<ExternalLinkIcon data-test-subj="externalLinkIcon" />
</EuiLink>
);

2
x-pack/plugins/security_solution/public/timelines/components/timeline/body/renderers/suricata/suricata_refs.tsx
View file

@ -9,7 +9,6 @@ import { EuiFlexGroup, EuiFlexItem, EuiLink } from '@elastic/eui';
import React from 'react';
import styled from 'styled-components';
import { ExternalLinkIcon } from '../../../../../../common/components/external_link_icon';
import { getLinksFromSignature } from './suricata_links';
const LinkEuiFlexItem = styled(EuiFlexItem)`
@ -27,7 +26,6 @@ export const SuricataRefs = React.memo<{ signatureId: number }>(({ signatureId }
<EuiLink href={link} color="subdued" target="_blank">
{link}
</EuiLink>
<ExternalLinkIcon />
</LinkEuiFlexItem>
))}
</EuiFlexGroup>