From 0b144da7b3f3b284885f50444ad6f4aa45f56d52 Mon Sep 17 00:00:00 2001 From: Paul Tavares Date: Thu, 16 Apr 2020 13:22:35 -0400 Subject: [PATCH 01/10] new hook providing generic event handler for use with react router --- .../use_navigate_by_router_event_handler.ts | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 x-pack/plugins/endpoint/public/applications/endpoint/view/hooks/use_navigate_by_router_event_handler.ts diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/hooks/use_navigate_by_router_event_handler.ts b/x-pack/plugins/endpoint/public/applications/endpoint/view/hooks/use_navigate_by_router_event_handler.ts new file mode 100644 index 00000000000000..dc33f0befaf357 --- /dev/null +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/hooks/use_navigate_by_router_event_handler.ts @@ -0,0 +1,70 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { MouseEventHandler, useCallback } from 'react'; +import { useHistory } from 'react-router-dom'; +import { LocationDescriptorObject } from 'history'; + +type EventHandlerCallback = MouseEventHandler; + +/** + * Provides an event handler that can be used with (for example) `onClick` props to prevent the + * event's default behaviour and instead navigate to to a route via the Router + * + * @param routeTo + * @param onClick + */ +export const useNavigateByRouterEventHandler = ( + routeTo: string | [string, unknown] | LocationDescriptorObject, // Cover the calling signature of `history.push()` + + /** Additional onClick callback */ + onClick?: EventHandlerCallback +): EventHandlerCallback => { + const history = useHistory(); + return useCallback( + ev => { + try { + if (onClick) { + onClick(ev); + } + } catch (error) { + ev.preventDefault(); + throw error; + } + + if (ev.defaultPrevented) { + return; + } + + if (ev.button !== 0) { + return; + } + + if ( + ev.currentTarget instanceof HTMLAnchorElement && + ev.currentTarget.target !== '' && + ev.currentTarget.target !== '_self' + ) { + return; + } + + if (ev.metaKey || ev.altKey || ev.ctrlKey || ev.shiftKey) { + return; + } + + ev.preventDefault(); + + if (Array.isArray(routeTo)) { + history.push(...routeTo); + } else if (typeof routeTo === 'string') { + history.push(routeTo); + } else { + history.push(routeTo); + } + }, + [history, onClick, routeTo] + ); +}; From e8e799c39222579e5ae2c61abc0161647a9fddf5 Mon Sep 17 00:00:00 2001 From: Paul Tavares Date: Thu, 16 Apr 2020 13:47:24 -0400 Subject: [PATCH 02/10] Refactor of Header Naviagtion to use useNavigateByRouterEventHandler --- .../view/components/header_navigation.tsx | 45 ++++++++++--------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/components/header_navigation.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/components/header_navigation.tsx index 6c294d9c865488..cc48a6b3d5b4dd 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/components/header_navigation.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/components/header_navigation.tsx @@ -4,12 +4,13 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { MouseEvent, useMemo } from 'react'; +import React, { memo, useMemo } from 'react'; import { i18n } from '@kbn/i18n'; import { EuiTabs, EuiTab } from '@elastic/eui'; -import { useHistory, useLocation } from 'react-router-dom'; +import { useLocation } from 'react-router-dom'; import { useKibana } from '../../../../../../../../src/plugins/kibana_react/public'; import { Immutable } from '../../../../../common/types'; +import { useNavigateByRouterEventHandler } from '../hooks/use_navigate_by_router_event_handler'; interface NavTabs { name: string; @@ -48,33 +49,33 @@ const navTabs: Immutable = [ }, ]; -export const HeaderNavigation: React.FunctionComponent = React.memo(() => { - const history = useHistory(); +const NavTab = memo<{ tab: NavTabs }>(({ tab }) => { const location = useLocation(); const { services } = useKibana(); + const onClickHandler = useNavigateByRouterEventHandler(tab.href); const BASE_PATH = services.application.getUrlForApp('endpoint'); + return ( + + {tab.name} + + ); +}); + +export const HeaderNavigation: React.FunctionComponent = React.memo(() => { const tabList = useMemo(() => { return navTabs.map((tab, index) => { - return ( - { - event.preventDefault(); - history.push(tab.href); - }} - isSelected={ - tab.href === location.pathname || - (tab.href !== '/' && location.pathname.startsWith(tab.href)) - } - > - {tab.name} - - ); + return ; }); - }, [BASE_PATH, history, location.pathname]); + }, []); return {tabList}; }); From 4643efea61d8a1f96f9e70283b4cf0844bcff9db Mon Sep 17 00:00:00 2001 From: Paul Tavares Date: Thu, 16 Apr 2020 15:41:18 -0400 Subject: [PATCH 03/10] Policy list refactor to use useNavigateByRouterEventHandler hook --- .../endpoint/view/components/header_navigation.tsx | 7 ++----- .../endpoint/view/policy/policy_details.tsx | 11 ++--------- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/components/header_navigation.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/components/header_navigation.tsx index cc48a6b3d5b4dd..7475229853698b 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/components/header_navigation.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/components/header_navigation.tsx @@ -50,7 +50,7 @@ const navTabs: Immutable = [ ]; const NavTab = memo<{ tab: NavTabs }>(({ tab }) => { - const location = useLocation(); + const { pathname } = useLocation(); const { services } = useKibana(); const onClickHandler = useNavigateByRouterEventHandler(tab.href); const BASE_PATH = services.application.getUrlForApp('endpoint'); @@ -60,10 +60,7 @@ const NavTab = memo<{ tab: NavTabs }>(({ tab }) => { data-test-subj={`${tab.id}EndpointTab`} href={`${BASE_PATH}${tab.href}`} onClick={onClickHandler} - isSelected={ - tab.href === location.pathname || - (tab.href !== '/' && location.pathname.startsWith(tab.href)) - } + isSelected={tab.href === pathname || (tab.href !== '/' && pathname.startsWith(tab.href))} > {tab.name} diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/policy/policy_details.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/policy/policy_details.tsx index 076de7b57b44b4..ea9eb292dba1a9 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/policy/policy_details.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/policy/policy_details.tsx @@ -20,7 +20,6 @@ import { import { FormattedMessage } from '@kbn/i18n/react'; import { i18n } from '@kbn/i18n'; import { useDispatch } from 'react-redux'; -import { useHistory } from 'react-router-dom'; import { usePolicyDetailsSelector } from './policy_hooks'; import { policyDetails, @@ -36,11 +35,11 @@ import { AgentsSummary } from './agents_summary'; import { VerticalDivider } from './vertical_divider'; import { WindowsEvents, MacEvents, LinuxEvents } from './policy_forms/events'; import { MalwareProtections } from './policy_forms/protections/malware'; +import { useNavigateByRouterEventHandler } from '../hooks/use_navigate_by_router_event_handler'; export const PolicyDetails = React.memo(() => { const dispatch = useDispatch<(action: AppAction) => void>(); const { notifications, services } = useKibana(); - const history = useHistory(); // Store values const policyItem = usePolicyDetailsSelector(policyDetails); @@ -82,13 +81,7 @@ export const PolicyDetails = React.memo(() => { } }, [notifications.toasts, policyItem, policyName, policyUpdateStatus]); - const handleBackToListOnClick: React.MouseEventHandler = useCallback( - ev => { - ev.preventDefault(); - history.push(`/policy`); - }, - [history] - ); + const handleBackToListOnClick = useNavigateByRouterEventHandler('/policy'); const handleSaveOnClick = useCallback(() => { setShowConfirm(true); From bac8e2c7698347c124ebb56a8eda34cd3d86b7fc Mon Sep 17 00:00:00 2001 From: Paul Tavares Date: Thu, 16 Apr 2020 15:52:53 -0400 Subject: [PATCH 04/10] Policy list Policy name link to use useNavigateByRouterEventHandler hook --- .../endpoint/view/policy/policy_list.tsx | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/policy/policy_list.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/policy/policy_list.tsx index 062c7afb6706dd..f7eafff137f519 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/policy/policy_list.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/policy/policy_list.tsx @@ -24,30 +24,26 @@ import { useKibana } from '../../../../../../../../src/plugins/kibana_react/publ import { PageView } from '../components/page_view'; import { LinkToApp } from '../components/link_to_app'; import { Immutable, PolicyData } from '../../../../../common/types'; +import { useNavigateByRouterEventHandler } from '../hooks/use_navigate_by_router_event_handler'; interface TableChangeCallbackArguments { page: { index: number; size: number }; } -const PolicyLink: React.FC<{ name: string; route: string }> = ({ name, route }) => { - const history = useHistory(); - +const PolicyLink: React.FC<{ name: string; route: string; href: string }> = ({ + name, + route, + href, +}) => { + const clickHandler = useNavigateByRouterEventHandler(route); return ( - { - event.preventDefault(); - history.push(route); - }} - > + // eslint-disable-next-line @elastic/eui/href-or-on-click + {name} ); }; -const renderPolicyNameLink = (value: string, item: Immutable) => { - return ; -}; - export const PolicyList = React.memo(() => { const { services, notifications } = useKibana(); const history = useHistory(); @@ -95,7 +91,16 @@ export const PolicyList = React.memo(() => { name: i18n.translate('xpack.endpoint.policyList.nameField', { defaultMessage: 'Policy Name', }), - render: renderPolicyNameLink, + render: (value: string, item: Immutable) => { + const routeUri = `/policy/${item.id}`; + return ( + + ); + }, truncateText: true, }, { From 87d72af4e8835fa1258893c58f7fe59bade3134d Mon Sep 17 00:00:00 2001 From: Paul Tavares Date: Thu, 16 Apr 2020 16:01:02 -0400 Subject: [PATCH 05/10] Host list use of useNavigateByRouteEventHandler --- .../endpoint/view/hosts/index.tsx | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/index.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/index.tsx index 1d81d6e8a16dbe..e662bafed64926 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/index.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/index.tsx @@ -4,9 +4,8 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { useMemo, useCallback } from 'react'; +import React, { useMemo, useCallback, memo } from 'react'; import { useDispatch } from 'react-redux'; -import { useHistory } from 'react-router-dom'; import { EuiPage, EuiPageBody, @@ -31,11 +30,26 @@ import { useHostListSelector } from './hooks'; import { CreateStructuredSelector } from '../../types'; import { urlFromQueryParams } from './url_from_query_params'; import { HostMetadata, Immutable } from '../../../../../common/types'; +import { useNavigateByRouterEventHandler } from '../hooks/use_navigate_by_router_event_handler'; + +const HostLink = memo<{ + name: string; + href: string; + route: ReturnType; +}>(({ name, href, route }) => { + const clickHandler = useNavigateByRouterEventHandler(route); + + return ( + // eslint-disable-next-line @elastic/eui/href-or-on-click + + {name} + + ); +}); const selector = (createStructuredSelector as CreateStructuredSelector)(selectors); export const HostList = () => { const dispatch = useDispatch<(a: HostAction) => void>(); - const history = useHistory(); const { listData, pageIndex, @@ -75,18 +89,9 @@ export const HostList = () => { defaultMessage: 'Hostname', }), render: ({ host: { hostname, id } }: { host: { hostname: string; id: string } }) => { + const newQueryParams = urlFromQueryParams({ ...queryParams, selected_host: id }); return ( - // eslint-disable-next-line @elastic/eui/href-or-on-click - { - ev.preventDefault(); - history.push(urlFromQueryParams({ ...queryParams, selected_host: id })); - }} - > - {hostname} - + ); }, }, @@ -150,7 +155,7 @@ export const HostList = () => { }, }, ]; - }, [queryParams, history]); + }, [queryParams]); return ( From f718e051fab8abcd732919ee225b5bcff2bea916 Mon Sep 17 00:00:00 2001 From: Paul Tavares Date: Fri, 17 Apr 2020 08:39:34 -0400 Subject: [PATCH 06/10] Added test plan for new hook --- .../use_navigate_by_router_event_handler.test.tsx | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 x-pack/plugins/endpoint/public/applications/endpoint/view/hooks/use_navigate_by_router_event_handler.test.tsx diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/hooks/use_navigate_by_router_event_handler.test.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/hooks/use_navigate_by_router_event_handler.test.tsx new file mode 100644 index 00000000000000..4d49bd5a11e765 --- /dev/null +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/hooks/use_navigate_by_router_event_handler.test.tsx @@ -0,0 +1,15 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +describe('useNavigateByRouterEventHandler hook', () => { + it.todo('should support onClick prop'); + it.todo('should navigate to path via Router'); + it.todo('should preventDefault if onClick callback throws'); + it.todo('should not navigate if preventDefault is true'); + it.todo('should not navigate via router if click was not the primary mouse button'); + it.todo('should not navigate via router if anchor has target'); + it.todo('should not to navigate if if meta|alt|ctrl|shift keys are pressed'); +}); From 42a0af68fee19acd217a188c7e93b47dcef3c7e4 Mon Sep 17 00:00:00 2001 From: Paul Tavares Date: Fri, 17 Apr 2020 11:22:19 -0400 Subject: [PATCH 07/10] Refactor latest Host components to use useNavigateByRouterEventHandler hook --- .../details/components/flyout_sub_header.tsx | 4 ++-- .../view/hosts/details/host_details.tsx | 13 +++++------ .../endpoint/view/hosts/details/index.tsx | 22 ++++++++++--------- 3 files changed, 19 insertions(+), 20 deletions(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/details/components/flyout_sub_header.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/details/components/flyout_sub_header.tsx index 26f2203790a9e2..02f91307c988ec 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/details/components/flyout_sub_header.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/details/components/flyout_sub_header.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { memo } from 'react'; +import React, { memo, MouseEventHandler } from 'react'; import { EuiFlyoutHeader, CommonProps, EuiButtonEmpty } from '@elastic/eui'; import styled from 'styled-components'; @@ -12,7 +12,7 @@ export type FlyoutSubHeaderProps = CommonProps & { children: React.ReactNode; backButton?: { title: string; - onClick: (event: React.MouseEvent) => void; + onClick: MouseEventHandler; href?: string; }; }; diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/details/host_details.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/details/host_details.tsx index 32c69426b03f33..336308b2ee2716 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/details/host_details.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/details/host_details.tsx @@ -16,13 +16,13 @@ import { import React, { memo, useMemo } from 'react'; import { FormattedMessage } from '@kbn/i18n/react'; import { i18n } from '@kbn/i18n'; -import { useHistory } from 'react-router-dom'; import { HostMetadata } from '../../../../../../common/types'; import { FormattedDateAndTime } from '../../formatted_date_time'; import { LinkToApp } from '../../components/link_to_app'; import { useHostListSelector, useHostLogsUrl } from '../hooks'; import { urlFromQueryParams } from '../url_from_query_params'; import { uiQueryParams } from '../../../store/hosts/selectors'; +import { useNavigateByRouterEventHandler } from '../../hooks/use_navigate_by_router_event_handler'; const HostIds = styled(EuiListGroupItem)` margin-top: 0; @@ -34,7 +34,6 @@ const HostIds = styled(EuiListGroupItem)` export const HostDetails = memo(({ details }: { details: HostMetadata }) => { const { appId, appPath, url } = useHostLogsUrl(details.host.id); const queryParams = useHostListSelector(uiQueryParams); - const history = useHistory(); const detailsResultsUpper = useMemo(() => { return [ { @@ -65,6 +64,7 @@ export const HostDetails = memo(({ details }: { details: HostMetadata }) => { show: 'policy_response', }); }, [details.host.id, queryParams]); + const policyStatusClickHandler = useNavigateByRouterEventHandler(policyResponseUri); const detailsResultsLower = useMemo(() => { return [ @@ -84,10 +84,7 @@ export const HostDetails = memo(({ details }: { details: HostMetadata }) => { { - ev.preventDefault(); - history.push(policyResponseUri); - }} + onClick={policyStatusClickHandler} > { details.endpoint.policy.id, details.host.hostname, details.host.ip, - history, - policyResponseUri, + policyResponseUri.search, + policyStatusClickHandler, ]); return ( diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/details/index.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/details/index.tsx index a41d4a968f177d..0c43e188225082 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/details/index.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/hosts/details/index.tsx @@ -24,6 +24,7 @@ import { HostDetails } from './host_details'; import { PolicyResponse } from './policy_response'; import { HostMetadata } from '../../../../../../common/types'; import { FlyoutSubHeader, FlyoutSubHeaderProps } from './components/flyout_sub_header'; +import { useNavigateByRouterEventHandler } from '../../hooks/use_navigate_by_router_event_handler'; export const HostDetailsFlyout = memo(() => { const history = useHistory(); @@ -92,24 +93,25 @@ export const HostDetailsFlyout = memo(() => { const PolicyResponseFlyoutPanel = memo<{ hostMeta: HostMetadata; }>(({ hostMeta }) => { - const history = useHistory(); const { show, ...queryParams } = useHostListSelector(uiQueryParams); + const detailsUri = useMemo( + () => + urlFromQueryParams({ + ...queryParams, + selected_host: hostMeta.host.id, + }), + [hostMeta.host.id, queryParams] + ); + const backToDetailsClickHandler = useNavigateByRouterEventHandler(detailsUri); const backButtonProp = useMemo((): FlyoutSubHeaderProps['backButton'] => { - const detailsUri = urlFromQueryParams({ - ...queryParams, - selected_host: hostMeta.host.id, - }); return { title: i18n.translate('xpack.endpoint.host.policyResponse.backLinkTitle', { defaultMessage: 'Endpoint Details', }), href: '?' + detailsUri.search, - onClick: ev => { - ev.preventDefault(); - history.push(detailsUri); - }, + onClick: backToDetailsClickHandler, }; - }, [history, hostMeta.host.id, queryParams]); + }, [backToDetailsClickHandler, detailsUri.search]); return ( <> From a2f5b8dc23c83fb6f6a9bd9412586bf3d5fbe71a Mon Sep 17 00:00:00 2001 From: Paul Tavares Date: Fri, 17 Apr 2020 15:35:59 -0400 Subject: [PATCH 08/10] Fix test cases for policy details --- .../endpoint/view/policy/policy_details.test.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/policy/policy_details.test.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/policy/policy_details.test.tsx index 2ecc2b117bf017..61d4a744192b03 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/policy/policy_details.test.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/policy/policy_details.test.tsx @@ -101,7 +101,8 @@ describe('Policy Details', () => { 'EuiPageHeaderSection[data-test-subj="pageViewHeaderLeft"] EuiButtonEmpty' ); expect(history.location.pathname).toEqual('/policy/1'); - backToListButton.simulate('click'); + backToListButton.simulate('click', { button: 0 }); + await sleep(); expect(history.location.pathname).toEqual('/policy'); }); it('should display agent stats', async () => { @@ -130,7 +131,7 @@ describe('Policy Details', () => { 'EuiButtonEmpty[data-test-subj="policyDetailsCancelButton"]' ); expect(history.location.pathname).toEqual('/policy/1'); - cancelbutton.simulate('click'); + cancelbutton.simulate('click', { button: 0 }); expect(history.location.pathname).toEqual('/policy'); }); it('should display save button', async () => { From 585b511fa1821047717defb25eda3f2dffa5ac3b Mon Sep 17 00:00:00 2001 From: Paul Tavares Date: Mon, 20 Apr 2020 10:50:30 -0400 Subject: [PATCH 09/10] Finish tests for router event handler hook --- ..._navigate_by_router_event_handler.test.tsx | 89 +++++++++++++++++-- 1 file changed, 82 insertions(+), 7 deletions(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/hooks/use_navigate_by_router_event_handler.test.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/hooks/use_navigate_by_router_event_handler.test.tsx index 4d49bd5a11e765..b1f09617f01744 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/hooks/use_navigate_by_router_event_handler.test.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/hooks/use_navigate_by_router_event_handler.test.tsx @@ -3,13 +3,88 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ +import React from 'react'; +import { AppContextTestRender, createAppRootMockRenderer } from '../../mocks'; +import { useNavigateByRouterEventHandler } from './use_navigate_by_router_event_handler'; +import { act, fireEvent, cleanup } from '@testing-library/react'; + +type ClickHandlerMock = jest.Mock< + Return, + [React.MouseEvent] +>; describe('useNavigateByRouterEventHandler hook', () => { - it.todo('should support onClick prop'); - it.todo('should navigate to path via Router'); - it.todo('should preventDefault if onClick callback throws'); - it.todo('should not navigate if preventDefault is true'); - it.todo('should not navigate via router if click was not the primary mouse button'); - it.todo('should not navigate via router if anchor has target'); - it.todo('should not to navigate if if meta|alt|ctrl|shift keys are pressed'); + let render: AppContextTestRender['render']; + let history: AppContextTestRender['history']; + let renderResult: ReturnType; + let linkEle: HTMLAnchorElement; + let clickHandlerSpy: ClickHandlerMock; + const Link = React.memo<{ + routeTo: Parameters[0]; + onClick?: Parameters[1]; + }>(({ routeTo, onClick }) => { + const onClickHandler = useNavigateByRouterEventHandler(routeTo, onClick); + return ( + + mock link + + ); + }); + + beforeEach(async () => { + ({ render, history } = createAppRootMockRenderer()); + clickHandlerSpy = jest.fn(); + renderResult = render(); + linkEle = (await renderResult.findByText('mock link')) as HTMLAnchorElement; + }); + afterEach(cleanup); + + it('should navigate to path via Router', () => { + const containerClickSpy = jest.fn(); + renderResult.container.addEventListener('click', containerClickSpy); + expect(history.location.pathname).not.toEqual('/mock/path'); + act(() => { + fireEvent.click(linkEle); + }); + expect(containerClickSpy.mock.calls[0][0].defaultPrevented).toBe(true); + expect(history.location.pathname).toEqual('/mock/path'); + renderResult.container.removeEventListener('click', containerClickSpy); + }); + it('should support onClick prop', () => { + act(() => { + fireEvent.click(linkEle); + }); + expect(clickHandlerSpy).toHaveBeenCalled(); + expect(history.location.pathname).toEqual('/mock/path'); + }); + it('should not navigate if preventDefault is true', () => { + clickHandlerSpy.mockImplementation(event => { + event.preventDefault(); + }); + act(() => { + fireEvent.click(linkEle); + }); + expect(history.location.pathname).not.toEqual('/mock/path'); + }); + it('should not navigate via router if click was not the primary mouse button', async () => { + act(() => { + fireEvent.click(linkEle, { button: 2 }); + }); + expect(history.location.pathname).not.toEqual('/mock/path'); + }); + it('should not navigate via router if anchor has target', () => { + linkEle.setAttribute('target', '_top'); + act(() => { + fireEvent.click(linkEle, { button: 2 }); + }); + expect(history.location.pathname).not.toEqual('/mock/path'); + }); + it('should not to navigate if meta|alt|ctrl|shift keys are pressed', () => { + ['meta', 'alt', 'ctrl', 'shift'].forEach(key => { + act(() => { + fireEvent.click(linkEle, { [`${key}Key`]: true }); + }); + expect(history.location.pathname).not.toEqual('/mock/path'); + }); + }); }); From 7c2572e6ca19bbdd1a0b8f8f03ef018bace5b86a Mon Sep 17 00:00:00 2001 From: Paul Tavares Date: Mon, 20 Apr 2020 11:39:50 -0400 Subject: [PATCH 10/10] remove unnecessary sleep from test --- .../applications/endpoint/view/components/link_to_app.test.tsx | 2 +- .../applications/endpoint/view/policy/policy_details.test.tsx | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/components/link_to_app.test.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/components/link_to_app.test.tsx index d0a8f9690dafbf..2d4d1ca8a1b5b0 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/components/link_to_app.test.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/components/link_to_app.test.tsx @@ -110,7 +110,7 @@ describe('LinkToApp component', () => { const clickEventArg = spyOnClickHandler.mock.calls[0][0]; expect(clickEventArg.isDefaultPrevented()).toBe(true); }); - it('should not navigate if onClick callback prevents defalut', () => { + it('should not navigate if onClick callback prevents default', () => { const spyOnClickHandler: LinkToAppOnClickMock = jest.fn(ev => { ev.preventDefault(); }); diff --git a/x-pack/plugins/endpoint/public/applications/endpoint/view/policy/policy_details.test.tsx b/x-pack/plugins/endpoint/public/applications/endpoint/view/policy/policy_details.test.tsx index 61d4a744192b03..d780b7bde8af34 100644 --- a/x-pack/plugins/endpoint/public/applications/endpoint/view/policy/policy_details.test.tsx +++ b/x-pack/plugins/endpoint/public/applications/endpoint/view/policy/policy_details.test.tsx @@ -102,7 +102,6 @@ describe('Policy Details', () => { ); expect(history.location.pathname).toEqual('/policy/1'); backToListButton.simulate('click', { button: 0 }); - await sleep(); expect(history.location.pathname).toEqual('/policy'); }); it('should display agent stats', async () => {