From 6f7bc21c18bfc353db43efa0eb2d7ded302cf610 Mon Sep 17 00:00:00 2001 From: Antonio 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 Screenshot 2024-09-09 at 20 15 39 Screenshot 2024-09-09 at 20 34 27 Screenshot 2024-09-09 at 20 34 57 --- .../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 f2b39b352a9642..da20cd4fce3975 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 = ({ -

{i18n.ADDITIONAL_FIELDS}

+

{i18n.CUSTOM_FIELDS}

 {customFieldsComponents} 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 b8359958025b32..b7a83c19251198 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 931cb6c06dbd74..365502f5c02c16 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(, { - wrappingComponent: TestProviders as ComponentType>, - }); - }); - - 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( + + + + ); - 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 = {'My message tooltip'}; - - const newWrapper = mount( - , - { - wrappingComponent: TestProviders as ComponentType>, - } - ); - - 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( - {msgTooltip}} - />, - { - wrappingComponent: TestProviders as ComponentType>, - } + render( + + {'My message tooltip'}} + /> + ); - 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(); - 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({'children'}); 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(); 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(); - 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(); - 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(); 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 cc6e29da8008cc..f1e8ca5cdb4afd 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 Promise | void; } -export const LinkButton: React.FC | PropsForAnchor> = - // TODO: Fix this manually. Issue #123375 - // eslint-disable-next-line react/display-name - ({ children, ...props }) => {children}; +type LinkButtonProps = React.FC< + (PropsForButton | PropsForAnchor) & { isEmpty?: boolean } +>; + +export const LinkButton: LinkButtonProps = ({ children, isEmpty, ...props }) => + isEmpty ? ( + {children} + ) : ( + {children} + ); +LinkButton.displayName = 'LinkButton'; // TODO: Fix this manually. Issue #123375 // eslint-disable-next-line react/display-name @@ -62,6 +69,7 @@ const CaseDetailsLinkComponent: React.FC = ({ ); }; + export const CaseDetailsLink = React.memo(CaseDetailsLinkComponent); CaseDetailsLink.displayName = 'CaseDetailsLink'; @@ -95,9 +103,10 @@ const ConfigureCaseButtonComponent: React.FC = ({ {label} diff --git a/x-pack/plugins/translations/translations/fr-FR.json b/x-pack/plugins/translations/translations/fr-FR.json index 5e08ebf2f10f29..cdd6e24ee08a58 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 76b479846efa4c..9da3c786cc1dc8 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 a17ef39b025e3e..34f5bd0874e488 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": "没有可用类别",