From 3fc6e98896cff77d557b8beae7047016d654e5db Mon Sep 17 00:00:00 2001 From: Kerry Gallagher Date: Wed, 22 Apr 2020 14:25:39 +0100 Subject: [PATCH] [Logs / Metrics UI] Switch to scopedHistory and enhance useLinkProps hook (#61667) --- .../plugins/infra/public/apps/start_app.tsx | 10 ++- .../source_configuration_settings.tsx | 13 ++-- .../public/hooks/use_link_props.test.tsx | 24 +++--- .../infra/public/hooks/use_link_props.tsx | 78 +++++++++---------- .../infra/public/utils/history_context.ts | 4 +- .../navigation_warning_prompt/context.tsx | 31 ++++++++ .../utils/navigation_warning_prompt/index.ts | 8 ++ .../navigation_warning_prompt/prompt.tsx | 25 ++++++ 8 files changed, 126 insertions(+), 67 deletions(-) create mode 100644 x-pack/plugins/infra/public/utils/navigation_warning_prompt/context.tsx create mode 100644 x-pack/plugins/infra/public/utils/navigation_warning_prompt/index.ts create mode 100644 x-pack/plugins/infra/public/utils/navigation_warning_prompt/prompt.tsx diff --git a/x-pack/plugins/infra/public/apps/start_app.tsx b/x-pack/plugins/infra/public/apps/start_app.tsx index ebf9562c38d7a..4c213700b62e6 100644 --- a/x-pack/plugins/infra/public/apps/start_app.tsx +++ b/x-pack/plugins/infra/public/apps/start_app.tsx @@ -4,7 +4,6 @@ * you may not use this file except in compliance with the Elastic License. */ -import { createBrowserHistory } from 'history'; import React from 'react'; import ReactDOM from 'react-dom'; import { ApolloProvider } from 'react-apollo'; @@ -25,6 +24,7 @@ import { AppRouter } from '../routers'; import { TriggersAndActionsUIPublicPluginSetup } from '../../../triggers_actions_ui/public'; import { TriggersActionsProvider } from '../utils/triggers_actions_context'; import '../index.scss'; +import { NavigationWarningPromptProvider } from '../utils/navigation_warning_prompt'; export const CONTAINER_CLASSNAME = 'infra-container-element'; @@ -36,8 +36,8 @@ export async function startApp( Router: AppRouter, triggersActionsUI: TriggersAndActionsUIPublicPluginSetup ) { - const { element, appBasePath } = params; - const history = createBrowserHistory({ basename: appBasePath }); + const { element, history } = params; + const InfraPluginRoot: React.FunctionComponent = () => { const [darkMode] = useUiSetting$('theme:darkMode'); @@ -49,7 +49,9 @@ export async function startApp( - + + + diff --git a/x-pack/plugins/infra/public/components/source_configuration/source_configuration_settings.tsx b/x-pack/plugins/infra/public/components/source_configuration/source_configuration_settings.tsx index 36645fa3f1f35..7f248cd103003 100644 --- a/x-pack/plugins/infra/public/components/source_configuration/source_configuration_settings.tsx +++ b/x-pack/plugins/infra/public/components/source_configuration/source_configuration_settings.tsx @@ -17,7 +17,6 @@ import { import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n/react'; import React, { useCallback, useContext, useMemo } from 'react'; -import { Prompt } from 'react-router-dom'; import { Source } from '../../containers/source'; import { FieldsConfigurationPanel } from './fields_configuration_panel'; @@ -26,6 +25,7 @@ import { NameConfigurationPanel } from './name_configuration_panel'; import { LogColumnsConfigurationPanel } from './log_columns_configuration_panel'; import { useSourceConfigurationFormState } from './source_configuration_form_state'; import { SourceLoadingPage } from '../source_loading_page'; +import { Prompt } from '../../utils/navigation_warning_prompt'; interface SourceConfigurationSettingsProps { shouldAllowEdit: boolean; @@ -100,10 +100,13 @@ export const SourceConfigurationSettings = ({ data-test-subj="sourceConfigurationContent" > { const INTERNAL_APP = 'metrics'; -// Note: Memory history doesn't support basename, -// we'll work around this by re-assigning 'createHref' so that -// it includes a basename, this then acts as our browserHistory instance would. const history = createMemoryHistory(); -const originalCreateHref = history.createHref; -history.createHref = (location: LocationDescriptorObject): string => { - return `${PREFIX}${INTERNAL_APP}${originalCreateHref.call(history, location)}`; -}; +history.push(`${PREFIX}${INTERNAL_APP}`); +const scopedHistory = new ScopedHistory(history, `${PREFIX}${INTERNAL_APP}`); const ProviderWrapper: React.FC = ({ children }) => { return ( - + {children}; ); @@ -111,7 +107,7 @@ describe('useLinkProps hook', () => { pathname: '/', }); expect(result.current.href).toBe('/test-basepath/s/test-space/app/ml/'); - expect(result.current.onClick).not.toBeDefined(); + expect(result.current.onClick).toBeDefined(); }); it('Provides the correct props with pathname options', () => { @@ -127,7 +123,7 @@ describe('useLinkProps hook', () => { expect(result.current.href).toBe( '/test-basepath/s/test-space/app/ml/explorer?type=host&id=some-id&count=12345' ); - expect(result.current.onClick).not.toBeDefined(); + expect(result.current.onClick).toBeDefined(); }); it('Provides the correct props with hash options', () => { @@ -143,7 +139,7 @@ describe('useLinkProps hook', () => { expect(result.current.href).toBe( '/test-basepath/s/test-space/app/ml#/explorer?type=host&id=some-id&count=12345' ); - expect(result.current.onClick).not.toBeDefined(); + expect(result.current.onClick).toBeDefined(); }); it('Provides the correct props with more complex encoding', () => { @@ -161,7 +157,7 @@ describe('useLinkProps hook', () => { expect(result.current.href).toBe( '/test-basepath/s/test-space/app/ml#/explorer?type=host%20%2B%20host&name=this%20name%20has%20spaces%20and%20**%20and%20%25&id=some-id&count=12345&animals=dog,cat,bear' ); - expect(result.current.onClick).not.toBeDefined(); + expect(result.current.onClick).toBeDefined(); }); it('Provides the correct props with a consumer using Rison encoding for search', () => { @@ -180,7 +176,7 @@ describe('useLinkProps hook', () => { expect(result.current.href).toBe( '/test-basepath/s/test-space/app/rison-app#rison-route?type=host%20%2B%20host&state=(refreshInterval:(pause:!t,value:0),time:(from:12345,to:54321))' ); - expect(result.current.onClick).not.toBeDefined(); + expect(result.current.onClick).toBeDefined(); }); }); }); diff --git a/x-pack/plugins/infra/public/hooks/use_link_props.tsx b/x-pack/plugins/infra/public/hooks/use_link_props.tsx index e60ab32046832..8c522bb7fa764 100644 --- a/x-pack/plugins/infra/public/hooks/use_link_props.tsx +++ b/x-pack/plugins/infra/public/hooks/use_link_props.tsx @@ -9,7 +9,8 @@ import { stringify } from 'query-string'; import url from 'url'; import { url as urlUtils } from '../../../../../src/plugins/kibana_utils/public'; import { usePrefixPathWithBasepath } from './use_prefix_path_with_basepath'; -import { useHistory } from '../utils/history_context'; +import { useKibana } from '../../../../../src/plugins/kibana_react/public'; +import { useNavigationWarningPrompt } from '../utils/navigation_warning_prompt'; type Search = Record; @@ -28,31 +29,26 @@ interface LinkProps { export const useLinkProps = ({ app, pathname, hash, search }: LinkDescriptor): LinkProps => { validateParams({ app, pathname, hash, search }); - const history = useHistory(); + const { prompt } = useNavigationWarningPrompt(); const prefixer = usePrefixPathWithBasepath(); + const navigateToApp = useKibana().services.application?.navigateToApp; const encodedSearch = useMemo(() => { return search ? encodeSearch(search) : undefined; }, [search]); - const internalLinkResult = useMemo(() => { - // When the logs / metrics apps are first mounted a history instance is setup with a 'basename' equal to the - // 'appBasePath' received from Core's 'AppMountParams', e.g. /BASE_PATH/s/SPACE_ID/app/APP_ID. With internal - // linking we are using 'createHref' and 'push' on top of this history instance. So a pathname of /inventory used within - // the metrics app will ultimatey end up as /BASE_PATH/s/SPACE_ID/app/metrics/inventory. React-router responds to this - // as it is instantiated with the same history instance. - return history?.createHref({ - pathname: pathname ? formatPathname(pathname) : undefined, - search: encodedSearch, - }); - }, [history, pathname, encodedSearch]); - - const externalLinkResult = useMemo(() => { + const mergedHash = useMemo(() => { // The URI spec defines that the query should appear before the fragment // https://tools.ietf.org/html/rfc3986#section-3 (e.g. url.format()). However, in Kibana, apps that use // hash based routing expect the query to be part of the hash. This will handle that. - const mergedHash = hash && encodedSearch ? `${hash}?${encodedSearch}` : hash; + return hash && encodedSearch ? `${hash}?${encodedSearch}` : hash; + }, [hash, encodedSearch]); + + const mergedPathname = useMemo(() => { + return pathname && encodedSearch ? `${pathname}?${encodedSearch}` : pathname; + }, [pathname, encodedSearch]); + const href = useMemo(() => { const link = url.format({ pathname, hash: mergedHash, @@ -60,28 +56,36 @@ export const useLinkProps = ({ app, pathname, hash, search }: LinkDescriptor): L }); return prefixer(app, link); - }, [hash, encodedSearch, pathname, prefixer, app]); + }, [mergedHash, hash, encodedSearch, pathname, prefixer, app]); const onClick = useMemo(() => { - // If these results are equal we know we're trying to navigate within the same application - // that the current history instance is representing - if (internalLinkResult && linksAreEquivalent(externalLinkResult, internalLinkResult)) { - return (e: React.MouseEvent | React.MouseEvent) => { - e.preventDefault(); - if (history) { - history.push({ - pathname: pathname ? formatPathname(pathname) : undefined, - search: encodedSearch, - }); + return (e: React.MouseEvent | React.MouseEvent) => { + e.preventDefault(); + + const navigate = () => { + if (navigateToApp) { + const navigationPath = mergedHash ? `#${mergedHash}` : mergedPathname; + navigateToApp(app, { path: navigationPath ? navigationPath : undefined }); } }; - } else { - return undefined; - } - }, [internalLinkResult, externalLinkResult, history, pathname, encodedSearch]); + + // A component somewhere within the app hierarchy is requesting that we + // prompt the user before navigating. + if (prompt) { + const wantsToNavigate = window.confirm(prompt); + if (wantsToNavigate) { + navigate(); + } else { + return; + } + } else { + navigate(); + } + }; + }, [navigateToApp, mergedHash, mergedPathname, app, prompt]); return { - href: externalLinkResult, + href, onClick, }; }; @@ -90,10 +94,6 @@ const encodeSearch = (search: Search) => { return stringify(urlUtils.encodeQuery(search), { sort: false, encode: false }); }; -const formatPathname = (pathname: string) => { - return pathname[0] === '/' ? pathname : `/${pathname}`; -}; - const validateParams = ({ app, pathname, hash, search }: LinkDescriptor) => { if (!app && hash) { throw new Error( @@ -101,9 +101,3 @@ const validateParams = ({ app, pathname, hash, search }: LinkDescriptor) => { ); } }; - -const linksAreEquivalent = (externalLink: string, internalLink: string): boolean => { - // Compares with trailing slashes removed. This handles the case where the pathname is '/' - // and 'createHref' will include the '/' but Kibana's 'getUrlForApp' will remove it. - return externalLink.replace(/\/$/, '') === internalLink.replace(/\/$/, ''); -}; diff --git a/x-pack/plugins/infra/public/utils/history_context.ts b/x-pack/plugins/infra/public/utils/history_context.ts index fe036e3179ec1..844d5b5e8e76f 100644 --- a/x-pack/plugins/infra/public/utils/history_context.ts +++ b/x-pack/plugins/infra/public/utils/history_context.ts @@ -5,9 +5,9 @@ */ import { createContext, useContext } from 'react'; -import { History } from 'history'; +import { ScopedHistory } from 'src/core/public'; -export const HistoryContext = createContext(undefined); +export const HistoryContext = createContext(undefined); export const useHistory = () => { return useContext(HistoryContext); diff --git a/x-pack/plugins/infra/public/utils/navigation_warning_prompt/context.tsx b/x-pack/plugins/infra/public/utils/navigation_warning_prompt/context.tsx new file mode 100644 index 0000000000000..10f8fb9e71f43 --- /dev/null +++ b/x-pack/plugins/infra/public/utils/navigation_warning_prompt/context.tsx @@ -0,0 +1,31 @@ +/* + * 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 React, { useState } from 'react'; +import { createContext, useContext } from 'react'; + +interface ContextValues { + prompt?: string; + setPrompt: (prompt: string | undefined) => void; +} + +export const NavigationWarningPromptContext = createContext({ + setPrompt: (prompt: string | undefined) => {}, +}); + +export const useNavigationWarningPrompt = () => { + return useContext(NavigationWarningPromptContext); +}; + +export const NavigationWarningPromptProvider: React.FC = ({ children }) => { + const [prompt, setPrompt] = useState(undefined); + + return ( + + {children} + + ); +}; diff --git a/x-pack/plugins/infra/public/utils/navigation_warning_prompt/index.ts b/x-pack/plugins/infra/public/utils/navigation_warning_prompt/index.ts new file mode 100644 index 0000000000000..dcdbf8e912a83 --- /dev/null +++ b/x-pack/plugins/infra/public/utils/navigation_warning_prompt/index.ts @@ -0,0 +1,8 @@ +/* + * 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. + */ + +export * from './context'; +export * from './prompt'; diff --git a/x-pack/plugins/infra/public/utils/navigation_warning_prompt/prompt.tsx b/x-pack/plugins/infra/public/utils/navigation_warning_prompt/prompt.tsx new file mode 100644 index 0000000000000..65ec4729c036d --- /dev/null +++ b/x-pack/plugins/infra/public/utils/navigation_warning_prompt/prompt.tsx @@ -0,0 +1,25 @@ +/* + * 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 React, { useEffect } from 'react'; +import { useNavigationWarningPrompt } from './context'; + +interface Props { + prompt?: string; +} + +export const Prompt: React.FC = ({ prompt }) => { + const { setPrompt } = useNavigationWarningPrompt(); + + useEffect(() => { + setPrompt(prompt); + return () => { + setPrompt(undefined); + }; + }, [prompt, setPrompt]); + + return null; +};