Skip to content

Commit

Permalink
[ResponseOps][Cases] Design Review changes #1 (elastic#192356)
Browse files Browse the repository at this point in the history
Connected to elastic#188187

## Summary

- Changed the cases `Settings` button and icon
- Changed the the `Additional fields` title to `Custom fields` for
consistency

<img width="1728" alt="Screenshot 2024-09-09 at 20 15 39"
src="https://github.com/user-attachments/assets/1fb1232a-f958-4d4d-8694-f85cc8872237">
<img width="1443" alt="Screenshot 2024-09-09 at 20 34 27"
src="https://github.com/user-attachments/assets/0fbdae02-65a6-4128-adc7-39f51cc2d5e6">
<img width="1370" alt="Screenshot 2024-09-09 at 20 34 57"
src="https://github.com/user-attachments/assets/c216407a-ac13-4579-8007-531c79d52de7">
  • Loading branch information
adcoelho authored Sep 12, 2024
1 parent e01dc14 commit 6f7bc21
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 94 deletions.
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 @@ -13103,7 +13103,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 @@ -13094,7 +13094,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 @@ -13116,7 +13116,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

0 comments on commit 6f7bc21

Please sign in to comment.