From 6f7bc21c18bfc353db43efa0eb2d7ded302cf610 Mon Sep 17 00:00:00 2001 From: Antonio <antonio.coelho@elastic.co> Date: Thu, 12 Sep 2024 18:24:53 +0900 Subject: [PATCH] [ResponseOps][Cases] Design Review changes #1 (#192356) Connected to #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"> --- .../case_form_fields/custom_fields.tsx | 2 +- .../case_form_fields/translations.ts | 4 +- .../public/components/links/index.test.tsx | 129 +++++++----------- .../cases/public/components/links/index.tsx | 21 ++- .../translations/translations/fr-FR.json | 1 - .../translations/translations/ja-JP.json | 1 - .../translations/translations/zh-CN.json | 1 - 7 files changed, 65 insertions(+), 94 deletions(-) diff --git a/x-pack/plugins/cases/public/components/case_form_fields/custom_fields.tsx b/x-pack/plugins/cases/public/components/case_form_fields/custom_fields.tsx index f2b39b352a964..da20cd4fce397 100644 --- a/x-pack/plugins/cases/public/components/case_form_fields/custom_fields.tsx +++ b/x-pack/plugins/cases/public/components/case_form_fields/custom_fields.tsx @@ -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> diff --git a/x-pack/plugins/cases/public/components/case_form_fields/translations.ts b/x-pack/plugins/cases/public/components/case_form_fields/translations.ts index b8359958025b3..b7a83c1925119 100644 --- a/x-pack/plugins/cases/public/components/case_form_fields/translations.ts +++ b/x-pack/plugins/cases/public/components/case_form_fields/translations.ts @@ -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', }); diff --git a/x-pack/plugins/cases/public/components/links/index.test.tsx b/x-pack/plugins/cases/public/components/links/index.test.tsx index 931cb6c06dbd7..365502f5c02c1 100644 --- a/x-pack/plugins/cases/public/components/links/index.test.tsx +++ b/x-pack/plugins/cases/public/components/links/index.test.tsx @@ -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 '.'; @@ -20,7 +17,6 @@ import { useCaseViewNavigation } from '../../common/navigation/hooks'; jest.mock('../../common/navigation/hooks'); describe('Configuration button', () => { - let wrapper: ReactWrapper; const props: ConfigureCaseButtonProps = { label: 'My label', msgTooltip: <></>, @@ -28,81 +24,46 @@ describe('Configuration button', () => { 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(); }); }); @@ -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({ @@ -153,7 +116,9 @@ 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, }); @@ -161,11 +126,11 @@ describe('CaseDetailsLink', () => { 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'); }); }); diff --git a/x-pack/plugins/cases/public/components/links/index.tsx b/x-pack/plugins/cases/public/components/links/index.tsx index cc6e29da8008c..f1e8ca5cdb4af 100644 --- a/x-pack/plugins/cases/public/components/links/index.tsx +++ b/x-pack/plugins/cases/public/components/links/index.tsx @@ -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'; @@ -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 @@ -62,6 +69,7 @@ const CaseDetailsLinkComponent: React.FC<CaseDetailsLinkProps> = ({ </LinkAnchor> ); }; + export const CaseDetailsLink = React.memo(CaseDetailsLinkComponent); CaseDetailsLink.displayName = 'CaseDetailsLink'; @@ -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} diff --git a/x-pack/plugins/translations/translations/fr-FR.json b/x-pack/plugins/translations/translations/fr-FR.json index 5e08ebf2f10f2..cdd6e24ee08a5 100644 --- a/x-pack/plugins/translations/translations/fr-FR.json +++ b/x-pack/plugins/translations/translations/fr-FR.json @@ -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", diff --git a/x-pack/plugins/translations/translations/ja-JP.json b/x-pack/plugins/translations/translations/ja-JP.json index 76b479846efa4..9da3c786cc1dc 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -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": "カテゴリがありません", diff --git a/x-pack/plugins/translations/translations/zh-CN.json b/x-pack/plugins/translations/translations/zh-CN.json index a17ef39b025e3..34f5bd0874e48 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -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": "没有可用类别",