Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ResponseOps][Cases] Design Review changes #1 #192356

Merged
merged 6 commits into from
Sep 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ const CustomFieldsComponent: React.FC<Props> = ({
<EuiFormRow fullWidth>
<EuiFlexGroup direction="column" gutterSize="s">
<EuiText size="m">
<h3>{i18n.ADDITIONAL_FIELDS}</h3>
<h3>{i18n.CUSTOM_FIELDS}</h3>
</EuiText>
<EuiSpacer size="xs" />
<EuiFlexItem data-test-subj="caseCustomFields">{customFieldsComponents}</EuiFlexItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ import { i18n } from '@kbn/i18n';

export * from '../../common/translations';

export const ADDITIONAL_FIELDS = i18n.translate('xpack.cases.additionalFields', {
defaultMessage: 'Additional fields',
export const CUSTOM_FIELDS = i18n.translate('xpack.cases.customFields', {
defaultMessage: 'Custom fields',
});
129 changes: 47 additions & 82 deletions x-pack/plugins/cases/public/components/links/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,8 @@
*/

import React from 'react';
import type { ComponentType, ReactWrapper } from 'enzyme';
import { mount } from 'enzyme';
import { render, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { EuiText } from '@elastic/eui';

import type { ConfigureCaseButtonProps, CaseDetailsLinkProps } from '.';
import { ConfigureCaseButton, CaseDetailsLink } from '.';
Expand All @@ -20,89 +17,53 @@ import { useCaseViewNavigation } from '../../common/navigation/hooks';
jest.mock('../../common/navigation/hooks');

describe('Configuration button', () => {
let wrapper: ReactWrapper;
const props: ConfigureCaseButtonProps = {
label: 'My label',
msgTooltip: <></>,
showToolTip: false,
titleTooltip: '',
};

beforeAll(() => {
wrapper = mount(<ConfigureCaseButton {...props} />, {
wrappingComponent: TestProviders as ComponentType<React.PropsWithChildren<{}>>,
});
});

test('it renders without the tooltip', () => {
expect(wrapper.find('[data-test-subj="configure-case-button"]').first().exists()).toBe(true);
it('renders without the tooltip', async () => {
render(
<TestProviders>
<ConfigureCaseButton {...props} />
</TestProviders>
);

expect(wrapper.find('[data-test-subj="configure-case-tooltip"]').first().exists()).toBe(false);
});
const configureButton = await screen.findByTestId('configure-case-button');

test('it pass the correct props to the button', () => {
expect(wrapper.find('[data-test-subj="configure-case-button"]').first().props()).toMatchObject({
href: `/app/security/cases/configure`,
iconType: 'controlsHorizontal',
isDisabled: false,
'aria-label': 'My label',
children: 'My label',
});
expect(configureButton).toBeEnabled();
expect(configureButton).toHaveAttribute('href', '/app/security/cases/configure');
expect(configureButton).toHaveAttribute('aria-label', 'My label');
});

test('it renders the tooltip', () => {
const msgTooltip = <EuiText>{'My message tooltip'}</EuiText>;

const newWrapper = mount(
<ConfigureCaseButton
{...props}
showToolTip={true}
titleTooltip={'My tooltip title'}
msgTooltip={msgTooltip}
/>,
{
wrappingComponent: TestProviders as ComponentType<React.PropsWithChildren<{}>>,
}
);

expect(newWrapper.find('[data-test-subj="configure-case-tooltip"]').first().exists()).toBe(
true
);
it('renders the tooltip correctly when hovering the button', async () => {
jest.useFakeTimers();

expect(wrapper.find('[data-test-subj="configure-case-button"]').first().exists()).toBe(true);
});
const user = userEvent.setup({
advanceTimers: jest.advanceTimersByTime,
pointerEventsCheck: 0,
});

test('it shows the tooltip when hovering the button', () => {
// Use fake timers so we don't have to wait for the EuiToolTip timeout
jest.useFakeTimers({ legacyFakeTimers: true });

const msgTooltip = 'My message tooltip';
const titleTooltip = 'My title';

const newWrapper = mount(
<ConfigureCaseButton
{...props}
showToolTip={true}
titleTooltip={titleTooltip}
msgTooltip={<>{msgTooltip}</>}
/>,
{
wrappingComponent: TestProviders as ComponentType<React.PropsWithChildren<{}>>,
}
render(
<TestProviders>
<ConfigureCaseButton
{...props}
showToolTip={true}
titleTooltip={'My title'}
msgTooltip={<>{'My message tooltip'}</>}
/>
</TestProviders>
);

newWrapper.find('a[data-test-subj="configure-case-button"]').first().simulate('mouseOver');
await user.hover(await screen.findByTestId('configure-case-button'));

// Run the timers so the EuiTooltip will be visible
jest.runAllTimers();
expect(await screen.findByTestId('configure-case-tooltip')).toBeInTheDocument();
expect(await screen.findByText('My title')).toBeInTheDocument();
expect(await screen.findByText('My message tooltip')).toBeInTheDocument();

newWrapper.update();
expect(newWrapper.find('.euiToolTipPopover').last().text()).toBe(
`${titleTooltip}${msgTooltip}`
);

// Clearing all mocks will also reset fake timers.
jest.clearAllMocks();
jest.useRealTimers();
});
});

Expand All @@ -120,31 +81,33 @@ describe('CaseDetailsLink', () => {
useCaseViewNavigationMock.mockReturnValue({ getCaseViewUrl, navigateToCaseView });
});

test('it renders', () => {
it('renders', async () => {
render(<CaseDetailsLink {...props} />);
expect(screen.getByText('test detail name')).toBeInTheDocument();
expect(await screen.findByText('test detail name')).toBeInTheDocument();
});

test('it renders the children instead of the detail name if provided', () => {
it('renders the children instead of the detail name if provided', async () => {
render(<CaseDetailsLink {...props}>{'children'}</CaseDetailsLink>);
expect(screen.queryByText('test detail name')).toBeFalsy();
expect(screen.getByText('children')).toBeInTheDocument();
expect(await screen.findByText('children')).toBeInTheDocument();
});

test('it uses the detailName in the aria-label if the title is not provided', () => {
it('uses the detailName in the aria-label if the title is not provided', async () => {
render(<CaseDetailsLink {...props} />);
expect(
screen.getByLabelText(`click to visit case with title ${props.detailName}`)
await screen.findByLabelText(`click to visit case with title ${props.detailName}`)
).toBeInTheDocument();
});

test('it uses the title in the aria-label if provided', () => {
it('uses the title in the aria-label if provided', async () => {
render(<CaseDetailsLink {...props} title={'my title'} />);
expect(screen.getByText('test detail name')).toBeInTheDocument();
expect(screen.getByLabelText(`click to visit case with title my title`)).toBeInTheDocument();
expect(await screen.findByText('test detail name')).toBeInTheDocument();
expect(
await screen.findByLabelText(`click to visit case with title my title`)
).toBeInTheDocument();
});

test('it calls navigateToCaseViewClick on click', async () => {
it('calls navigateToCaseViewClick on click', async () => {
// Workaround for timeout via https://github.com/testing-library/user-event/issues/833#issuecomment-1171452841
jest.useFakeTimers();
const user = userEvent.setup({
Expand All @@ -153,19 +116,21 @@ describe('CaseDetailsLink', () => {
});

render(<CaseDetailsLink {...props} />);
await user.click(screen.getByText('test detail name'));

await user.click(await screen.findByText('test detail name'));

expect(navigateToCaseView).toHaveBeenCalledWith({
detailName: props.detailName,
});

jest.useRealTimers();
});

test('it set the href correctly', () => {
it('sets the href correctly', async () => {
render(<CaseDetailsLink {...props} />);
expect(getCaseViewUrl).toHaveBeenCalledWith({
detailName: props.detailName,
});
expect(screen.getByRole('link')).toHaveAttribute('href', '/cases/test');
expect(await screen.findByRole('link')).toHaveAttribute('href', '/cases/test');
});
});
21 changes: 15 additions & 6 deletions x-pack/plugins/cases/public/components/links/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import type { EuiButtonProps, EuiLinkProps, PropsForAnchor, PropsForButton } from '@elastic/eui';
import { EuiButton, EuiLink, EuiToolTip } from '@elastic/eui';
import { EuiButton, EuiLink, EuiToolTip, EuiButtonEmpty } from '@elastic/eui';
import React, { useCallback, useMemo } from 'react';
import { useCaseViewNavigation, useConfigureCasesNavigation } from '../../common/navigation';
import * as i18n from './translations';
Expand All @@ -18,10 +18,17 @@ export interface CasesNavigation<T = React.MouseEvent | MouseEvent | null, K = n
: (arg: T) => Promise<void> | void;
}

export const LinkButton: React.FC<PropsForButton<EuiButtonProps> | PropsForAnchor<EuiButtonProps>> =
// TODO: Fix this manually. Issue #123375
// eslint-disable-next-line react/display-name
({ children, ...props }) => <EuiButton {...props}>{children}</EuiButton>;
type LinkButtonProps = React.FC<
(PropsForButton<EuiButtonProps> | PropsForAnchor<EuiButtonProps>) & { isEmpty?: boolean }
>;

export const LinkButton: LinkButtonProps = ({ children, isEmpty, ...props }) =>
isEmpty ? (
<EuiButtonEmpty {...props}>{children}</EuiButtonEmpty>
) : (
<EuiButton {...props}>{children}</EuiButton>
);
LinkButton.displayName = 'LinkButton';

// TODO: Fix this manually. Issue #123375
// eslint-disable-next-line react/display-name
Expand Down Expand Up @@ -62,6 +69,7 @@ const CaseDetailsLinkComponent: React.FC<CaseDetailsLinkProps> = ({
</LinkAnchor>
);
};

export const CaseDetailsLink = React.memo(CaseDetailsLinkComponent);
CaseDetailsLink.displayName = 'CaseDetailsLink';

Expand Down Expand Up @@ -95,9 +103,10 @@ const ConfigureCaseButtonComponent: React.FC<ConfigureCaseButtonProps> = ({
<LinkButton
onClick={navigateToConfigureCasesClick}
href={getConfigureCasesUrl()}
iconType="controlsHorizontal"
iconType="gear"
isDisabled={false}
aria-label={label}
isEmpty={true}
data-test-subj="configure-case-button"
>
{label}
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/fr-FR.json
Original file line number Diff line number Diff line change
Expand Up @@ -13137,7 +13137,6 @@
"xpack.cases.actions.viewCase": "Afficher le cas",
"xpack.cases.actions.visualizationActions.addToExistingCase.displayName": "Ajouter au cas",
"xpack.cases.addConnector.title": "Ajouter un connecteur",
"xpack.cases.additionalFields": "Champs supplémentaires",
"xpack.cases.allCases.actions": "Actions",
"xpack.cases.allCases.comments": "Commentaires",
"xpack.cases.allCases.noCategoriesAvailable": "Pas de catégories disponibles",
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/ja-JP.json
Original file line number Diff line number Diff line change
Expand Up @@ -13128,7 +13128,6 @@
"xpack.cases.actions.viewCase": "ケースの表示",
"xpack.cases.actions.visualizationActions.addToExistingCase.displayName": "ケースに追加",
"xpack.cases.addConnector.title": "コネクターの追加",
"xpack.cases.additionalFields": "追加フィールド",
"xpack.cases.allCases.actions": "アクション",
"xpack.cases.allCases.comments": "コメント",
"xpack.cases.allCases.noCategoriesAvailable": "カテゴリがありません",
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/translations/translations/zh-CN.json
Original file line number Diff line number Diff line change
Expand Up @@ -13150,7 +13150,6 @@
"xpack.cases.actions.viewCase": "查看案例",
"xpack.cases.actions.visualizationActions.addToExistingCase.displayName": "添加到案例",
"xpack.cases.addConnector.title": "添加连接器",
"xpack.cases.additionalFields": "其他字段",
"xpack.cases.allCases.actions": "操作",
"xpack.cases.allCases.comments": "注释",
"xpack.cases.allCases.noCategoriesAvailable": "没有可用类别",
Expand Down