From 29b0f531fa7a51f1fab65778c37c8bfa4fdde600 Mon Sep 17 00:00:00 2001 From: adcoelho Date: Mon, 9 Sep 2024 20:33:53 +0900 Subject: [PATCH 1/4] ui changes --- .../case_form_fields/custom_fields.tsx | 2 +- .../case_form_fields/translations.ts | 4 ++-- .../cases/public/components/links/index.tsx | 21 +++++++++++++------ 3 files changed, 18 insertions(+), 9 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.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} From 39138ba9eb5118dd7195b8db4fc9e25dcc783bbf Mon Sep 17 00:00:00 2001 From: adcoelho Date: Mon, 9 Sep 2024 21:02:07 +0900 Subject: [PATCH 2/4] Fix translations. --- x-pack/plugins/translations/translations/fr-FR.json | 1 - x-pack/plugins/translations/translations/ja-JP.json | 1 - x-pack/plugins/translations/translations/zh-CN.json | 1 - 3 files changed, 3 deletions(-) diff --git a/x-pack/plugins/translations/translations/fr-FR.json b/x-pack/plugins/translations/translations/fr-FR.json index 2fb444ad60020a..52d0492820fbd4 100644 --- a/x-pack/plugins/translations/translations/fr-FR.json +++ b/x-pack/plugins/translations/translations/fr-FR.json @@ -13138,7 +13138,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 b9c6d9ab1758c6..daa907baa8e7a5 100644 --- a/x-pack/plugins/translations/translations/ja-JP.json +++ b/x-pack/plugins/translations/translations/ja-JP.json @@ -13129,7 +13129,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 a573ea4868a105..9d30524ae57482 100644 --- a/x-pack/plugins/translations/translations/zh-CN.json +++ b/x-pack/plugins/translations/translations/zh-CN.json @@ -13151,7 +13151,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": "没有可用类别", From cc8a7931784654822ed6a32d19321830d27087ba Mon Sep 17 00:00:00 2001 From: adcoelho Date: Tue, 10 Sep 2024 10:21:05 +0900 Subject: [PATCH 3/4] fix tests --- .../public/components/links/index.test.tsx | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) 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 4d962197951a90..a2a6f5e3faad1f 100644 --- a/x-pack/plugins/cases/public/components/links/index.test.tsx +++ b/x-pack/plugins/cases/public/components/links/index.test.tsx @@ -32,23 +32,23 @@ describe('Configuration button', () => { wrapper = mount(, { wrappingComponent: TestProviders }); }); - test('it renders without the tooltip', () => { + it('renders without the tooltip', () => { expect(wrapper.find('[data-test-subj="configure-case-button"]').first().exists()).toBe(true); expect(wrapper.find('[data-test-subj="configure-case-tooltip"]').first().exists()).toBe(false); }); - test('it pass the correct props to the button', () => { + it('passes 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', + iconType: 'gear', isDisabled: false, 'aria-label': 'My label', children: 'My label', }); }); - test('it renders the tooltip', () => { + it('renders the tooltip', () => { const msgTooltip = {'My message tooltip'}; const newWrapper = mount( @@ -70,7 +70,7 @@ describe('Configuration button', () => { expect(wrapper.find('[data-test-subj="configure-case-button"]').first().exists()).toBe(true); }); - test('it shows the tooltip when hovering the button', () => { + 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 }); @@ -118,43 +118,45 @@ 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', () => { + it('calls navigateToCaseViewClick on click', async () => { render(); - userEvent.click(screen.getByText('test detail name')); + userEvent.click(await screen.findByText('test detail name')); expect(navigateToCaseView).toHaveBeenCalledWith({ detailName: props.detailName, }); }); - 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'); }); }); From b1e0d11471c3ca353f021b01519e07c5d4eb59f1 Mon Sep 17 00:00:00 2001 From: adcoelho Date: Wed, 11 Sep 2024 21:53:40 +0900 Subject: [PATCH 4/4] Update test --- .../public/components/links/index.test.tsx | 110 +++++++----------- 1 file changed, 40 insertions(+), 70 deletions(-) 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 54f5e5d4ecc32e..3427255b9de9b4 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>, - }); - }); + it('renders without the tooltip', async () => { + render( + + + + ); - it('renders without the tooltip', () => { - expect(wrapper.find('[data-test-subj="configure-case-button"]').first().exists()).toBe(true); + const configureButton = await screen.findByTestId('configure-case-button'); - expect(wrapper.find('[data-test-subj="configure-case-tooltip"]').first().exists()).toBe(false); + expect(configureButton).toBeEnabled(); + expect(configureButton).toHaveAttribute('href', '/app/security/cases/configure'); + expect(configureButton).toHaveAttribute('aria-label', 'My label'); }); - it('passes the correct props to the button', () => { - expect(wrapper.find('[data-test-subj="configure-case-button"]').first().props()).toMatchObject({ - href: `/app/security/cases/configure`, - iconType: 'gear', - isDisabled: false, - 'aria-label': 'My label', - children: 'My label', - }); - }); - - 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, + }); - 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(); - - newWrapper.update(); - expect(newWrapper.find('.euiToolTipPopover').last().text()).toBe( - `${titleTooltip}${msgTooltip}` - ); + expect(await screen.findByTestId('configure-case-tooltip')).toBeInTheDocument(); + expect(await screen.findByText('My title')).toBeInTheDocument(); + expect(await screen.findByText('My message tooltip')).toBeInTheDocument(); - // Clearing all mocks will also reset fake timers. - jest.clearAllMocks(); + jest.useRealTimers(); }); }); @@ -147,11 +108,20 @@ describe('CaseDetailsLink', () => { }); 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({ + advanceTimers: jest.advanceTimersByTime, + pointerEventsCheck: 0, + }); + render(); - userEvent.click(await screen.findByText('test detail name')); + await user.click(await screen.findByText('test detail name')); expect(navigateToCaseView).toHaveBeenCalledWith({ detailName: props.detailName, }); + + jest.useRealTimers(); }); it('sets the href correctly', async () => {