From b8258d52fd8bbe77917d29a760406fcca1d01d96 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Mon, 14 Jun 2021 21:43:06 -0700 Subject: [PATCH 1/8] Set up shared EnterpriseSearchPageTemplate component --- x-pack/plugins/enterprise_search/kibana.json | 2 +- .../applications/shared/layout/index.ts | 3 + .../shared/layout/page_template.scss | 15 ++ .../shared/layout/page_template.test.tsx | 192 ++++++++++++++++++ .../shared/layout/page_template.tsx | 90 ++++++++ 5 files changed, 301 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugins/enterprise_search/public/applications/shared/layout/page_template.scss create mode 100644 x-pack/plugins/enterprise_search/public/applications/shared/layout/page_template.test.tsx create mode 100644 x-pack/plugins/enterprise_search/public/applications/shared/layout/page_template.tsx diff --git a/x-pack/plugins/enterprise_search/kibana.json b/x-pack/plugins/enterprise_search/kibana.json index a7b29a1e6b457f..f8b4261114a22d 100644 --- a/x-pack/plugins/enterprise_search/kibana.json +++ b/x-pack/plugins/enterprise_search/kibana.json @@ -7,7 +7,7 @@ "optionalPlugins": ["usageCollection", "security", "home", "spaces", "cloud"], "server": true, "ui": true, - "requiredBundles": ["home"], + "requiredBundles": ["home", "kibanaReact"], "owner": { "name": "Enterprise Search", "githubTeam": "enterprise-search-frontend" diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/layout/index.ts b/x-pack/plugins/enterprise_search/public/applications/shared/layout/index.ts index 2dd5254cee7f1e..6f5b7b724cef41 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/layout/index.ts +++ b/x-pack/plugins/enterprise_search/public/applications/shared/layout/index.ts @@ -5,5 +5,8 @@ * 2.0. */ +export { EnterpriseSearchPageTemplate, PageTemplateProps } from './page_template'; + +// TODO: Delete these once KibanaPageTemplate migration is done export { Layout } from './layout'; export { SideNav, SideNavLink, SideNavItem } from './side_nav'; diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/layout/page_template.scss b/x-pack/plugins/enterprise_search/public/applications/shared/layout/page_template.scss new file mode 100644 index 00000000000000..9ddd68277c9bc9 --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/shared/layout/page_template.scss @@ -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 + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +.enterpriseSearchPageTemplate { + position: relative; + + &__content { + // Note: relative positioning is required for our centered Loading component + position: relative; + } +} diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/layout/page_template.test.tsx b/x-pack/plugins/enterprise_search/public/applications/shared/layout/page_template.test.tsx new file mode 100644 index 00000000000000..ee71331666fad4 --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/shared/layout/page_template.test.tsx @@ -0,0 +1,192 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { setMockValues } from '../../__mocks__/kea_logic'; + +import React from 'react'; + +import { shallow } from 'enzyme'; + +import { EuiCallOut } from '@elastic/eui'; + +import { KibanaPageTemplate } from '../../../../../../../src/plugins/kibana_react/public'; +import { FlashMessages } from '../flash_messages'; +import { Loading } from '../loading'; + +import { EnterpriseSearchPageTemplate } from './page_template'; + +describe('EnterpriseSearchPageTemplate', () => { + beforeEach(() => { + jest.clearAllMocks(); + setMockValues({ readOnlyMode: false }); + }); + + it('renders', () => { + const wrapper = shallow(); + + expect(wrapper.type()).toEqual(KibanaPageTemplate); + }); + + it('renders children', () => { + const wrapper = shallow( + +
world
+ + ); + + expect(wrapper.find('.hello').text()).toEqual('world'); + }); + + describe('loading state', () => { + it('renders a loading icon in place of children', () => { + const wrapper = shallow( + +
+ + ); + + expect(wrapper.find(Loading).exists()).toBe(true); + expect(wrapper.find('.test').exists()).toBe(false); + }); + + it('renders children & does not render a loading icon when the page is done loading', () => { + const wrapper = shallow( + +
+ + ); + + expect(wrapper.find(Loading).exists()).toBe(false); + expect(wrapper.find('.test').exists()).toBe(true); + }); + }); + + describe('empty state', () => { + it('renders a custom empty state in place of children', () => { + const wrapper = shallow( + Nothing here yet!
} + > +
+ + ); + + expect(wrapper.find('.emptyState').exists()).toBe(true); + expect(wrapper.find('.test').exists()).toBe(false); + + // @see https://github.com/elastic/kibana/blob/master/dev_docs/tutorials/kibana_page_template.mdx#isemptystate + // if you want to use KibanaPageTemplate's `isEmptyState` without a custom emptyState + }); + + it('does not render the custom empty state if the page is not empty', () => { + const wrapper = shallow( + Nothing here yet!
} + > +
+ + ); + + expect(wrapper.find('.emptyState').exists()).toBe(false); + expect(wrapper.find('.test').exists()).toBe(true); + }); + + it('does not render an empty state if the page is still loading', () => { + const wrapper = shallow( + } + /> + ); + + expect(wrapper.find(Loading).exists()).toBe(true); + expect(wrapper.find('.emptyState').exists()).toBe(false); + }); + }); + + describe('read-only mode', () => { + it('renders a callout if in read-only mode', () => { + setMockValues({ readOnlyMode: true }); + const wrapper = shallow(); + + expect(wrapper.find(EuiCallOut).exists()).toBe(true); + }); + + it('does not render a callout if not in read-only mode', () => { + setMockValues({ readOnlyMode: false }); + const wrapper = shallow(); + + expect(wrapper.find(EuiCallOut).exists()).toBe(false); + }); + }); + + describe('flash messages', () => { + it('renders FlashMessages by default', () => { + const wrapper = shallow(); + + expect(wrapper.find(FlashMessages).exists()).toBe(true); + }); + + it('does not render FlashMessages if hidden', () => { + // Example use case: manually showing flash messages in an open flyout or modal + // and not wanting to duplicate flash messages on the overlayed page + const wrapper = shallow(); + + expect(wrapper.find(FlashMessages).exists()).toBe(false); + }); + }); + + describe('EuiPageTemplate props', () => { + it('overrides the restrictWidth prop', () => { + const wrapper = shallow(); + + expect(wrapper.find(KibanaPageTemplate).prop('restrictWidth')).toEqual(true); + }); + + it('passes down any ...pageTemplateProps that EuiPageTemplate accepts', () => { + const wrapper = shallow( + + ); + + expect(wrapper.find(KibanaPageTemplate).prop('template')).toEqual('empty'); + expect(wrapper.find(KibanaPageTemplate).prop('paddingSize')).toEqual('s'); + expect(wrapper.find(KibanaPageTemplate).prop('pageHeader')!.pageTitle).toEqual('hello world'); + }); + + it('sets enterpriseSearchPageTemplate classNames while still accepting custom classNames', () => { + const wrapper = shallow( + + ); + + expect(wrapper.find(KibanaPageTemplate).prop('className')).toEqual( + 'enterpriseSearchPageTemplate hello' + ); + expect(wrapper.find(KibanaPageTemplate).prop('pageContentProps')!.className).toEqual( + 'enterpriseSearchPageTemplate__content world' + ); + }); + + it('automatically sets the Enterprise Search logo onto passed solution navs', () => { + const wrapper = shallow( + + ); + + expect(wrapper.find(KibanaPageTemplate).prop('solutionNav')).toEqual({ + icon: 'logoEnterpriseSearch', + name: 'Enterprise Search', + items: [], + }); + }); + }); +}); diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/layout/page_template.tsx b/x-pack/plugins/enterprise_search/public/applications/shared/layout/page_template.tsx new file mode 100644 index 00000000000000..08fd28799d69b4 --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/shared/layout/page_template.tsx @@ -0,0 +1,90 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React from 'react'; + +import classNames from 'classnames'; +import { useValues } from 'kea'; + +import { EuiCallOut, EuiSpacer } from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; + +import { + KibanaPageTemplate, + KibanaPageTemplateProps, +} from '../../../../../../../src/plugins/kibana_react/public'; + +import { FlashMessages } from '../flash_messages'; +import { HttpLogic } from '../http'; +import { Loading } from '../loading'; + +import './page_template.scss'; + +/* + * EnterpriseSearchPageTemplate is a light wrapper for KibanaPageTemplate (which + * is a light wrapper for EuiPageTemplate). It should contain only concerns shared + * between both AS & WS, which should have their own AppSearchPageTemplate & + * WorkplaceSearchPageTemplate sitting on top of this template (:nesting_dolls:), + * which in turn manages individual product-specific concerns (e.g. side navs, telemetry, etc.) + * + * @see https://github.com/elastic/kibana/tree/master/src/plugins/kibana_react/public/page_template + * @see https://elastic.github.io/eui/#/layout/page + */ + +export type PageTemplateProps = KibanaPageTemplateProps & { + hideFlashMessages?: boolean; + isLoading?: boolean; + emptyState?: React.ReactNode; +}; + +export const EnterpriseSearchPageTemplate: React.FC = ({ + children, + className, + hideFlashMessages, + isLoading, + isEmptyState, + emptyState, + solutionNav, + ...pageTemplateProps +}) => { + const { readOnlyMode } = useValues(HttpLogic); + const hasCustomEmptyState = !!emptyState; + const showCustomEmptyState = hasCustomEmptyState && isEmptyState; + + return ( + + {readOnlyMode && ( + <> + + + + )} + {!hideFlashMessages && } + {isLoading ? : showCustomEmptyState ? emptyState : children} + + ); +}; From 1b041f75e806ccc6e8ed128dd6bf3b0b6057aac4 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Mon, 14 Jun 2021 22:19:42 -0700 Subject: [PATCH 2/8] Set up product-specific page templates + setPageChrome + misc tech debt - create AS components/layout/index.ts for imports --- .../app_search/components/layout/index.ts | 9 +++ .../components/layout/page_template.test.tsx | 65 ++++++++++++++++ .../components/layout/page_template.tsx | 34 +++++++++ .../public/applications/app_search/index.tsx | 2 +- .../shared/layout/page_template.test.tsx | 21 ++++++ .../shared/layout/page_template.tsx | 7 ++ .../components/layout/index.ts | 1 + .../components/layout/page_template.test.tsx | 75 +++++++++++++++++++ .../components/layout/page_template.tsx | 37 +++++++++ 9 files changed, 250 insertions(+), 1 deletion(-) create mode 100644 x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/index.ts create mode 100644 x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/page_template.test.tsx create mode 100644 x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/page_template.tsx create mode 100644 x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/page_template.test.tsx create mode 100644 x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/page_template.tsx diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/index.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/index.ts new file mode 100644 index 00000000000000..177eb5de5bb672 --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/index.ts @@ -0,0 +1,9 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +export { AppSearchPageTemplate } from './page_template'; +export { KibanaHeaderActions } from './kibana_header_actions'; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/page_template.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/page_template.test.tsx new file mode 100644 index 00000000000000..2a996c845e83f2 --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/page_template.test.tsx @@ -0,0 +1,65 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React from 'react'; + +import { shallow } from 'enzyme'; + +import { SetAppSearchChrome } from '../../../shared/kibana_chrome'; +import { EnterpriseSearchPageTemplate } from '../../../shared/layout'; +import { SendAppSearchTelemetry } from '../../../shared/telemetry'; + +import { AppSearchPageTemplate } from './page_template'; + +describe('AppSearchPageTemplate', () => { + it('renders', () => { + const wrapper = shallow( + +
world
+ + ); + + expect(wrapper.type()).toEqual(EnterpriseSearchPageTemplate); + expect(wrapper.prop('solutionNav')).toEqual({ name: 'App Search', items: [] }); + expect(wrapper.find('.hello').text()).toEqual('world'); + }); + + describe('page chrome', () => { + it('takes a breadcrumb array & renders a product-specific page chrome', () => { + const wrapper = shallow(); + const setPageChrome = wrapper.find(EnterpriseSearchPageTemplate).prop('setPageChrome') as any; + + expect(setPageChrome.type).toEqual(SetAppSearchChrome); + expect(setPageChrome.props.trail).toEqual(['Some page']); + }); + }); + + describe('page telemetry', () => { + it('takes a metric & renders product-specific telemetry viewed event', () => { + const wrapper = shallow(); + + expect(wrapper.find(SendAppSearchTelemetry).prop('action')).toEqual('viewed'); + expect(wrapper.find(SendAppSearchTelemetry).prop('metric')).toEqual('some_page'); + }); + }); + + it('passes down any ...pageTemplateProps that EnterpriseSearchPageTemplate accepts', () => { + const wrapper = shallow( + } + /> + ); + + expect(wrapper.find(EnterpriseSearchPageTemplate).prop('pageHeader')!.pageTitle).toEqual( + 'hello world' + ); + expect(wrapper.find(EnterpriseSearchPageTemplate).prop('isLoading')).toEqual(false); + expect(wrapper.find(EnterpriseSearchPageTemplate).prop('emptyState')).toEqual(
); + }); +}); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/page_template.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/page_template.tsx new file mode 100644 index 00000000000000..d89544e7040311 --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/page_template.tsx @@ -0,0 +1,34 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React from 'react'; + +import { APP_SEARCH_PLUGIN } from '../../../../../common/constants'; +import { SetAppSearchChrome } from '../../../shared/kibana_chrome'; +import { EnterpriseSearchPageTemplate, PageTemplateProps } from '../../../shared/layout'; +import { SendAppSearchTelemetry } from '../../../shared/telemetry'; + +export const AppSearchPageTemplate: React.FC = ({ + children, + pageChrome, + pageViewTelemetry, + ...pageTemplateProps +}) => { + return ( + } + > + {pageViewTelemetry && } + {children} + + ); +}; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/index.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/index.tsx index a491efcb234dca..20f65ba2c346ca 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/index.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/index.tsx @@ -25,7 +25,7 @@ import { EngineNav, EngineRouter } from './components/engine'; import { EngineCreation } from './components/engine_creation'; import { EnginesOverview, ENGINES_TITLE } from './components/engines'; import { ErrorConnecting } from './components/error_connecting'; -import { KibanaHeaderActions } from './components/layout/kibana_header_actions'; +import { KibanaHeaderActions } from './components/layout'; import { Library } from './components/library'; import { MetaEngineCreation } from './components/meta_engine_creation'; import { RoleMappings } from './components/role_mappings'; diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/layout/page_template.test.tsx b/x-pack/plugins/enterprise_search/public/applications/shared/layout/page_template.test.tsx index ee71331666fad4..5b02756e44b524 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/layout/page_template.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/shared/layout/page_template.test.tsx @@ -143,6 +143,27 @@ describe('EnterpriseSearchPageTemplate', () => { }); }); + describe('page chrome', () => { + const SetPageChrome = () =>
; + + it('renders a product-specific ', () => { + const wrapper = shallow(} />); + + expect(wrapper.find(SetPageChrome).exists()).toBe(true); + }); + + it('invokes page chrome immediately (without waiting for isLoading to be finished)', () => { + const wrapper = shallow( + } isLoading /> + ); + + expect(wrapper.find(SetPageChrome).exists()).toBe(true); + + // This behavior is in contrast to page view telemetry, which is invoked after isLoading finishes + // In addition to the pageHeader prop also changing immediately, this makes navigation feel much snappier + }); + }); + describe('EuiPageTemplate props', () => { it('overrides the restrictWidth prop', () => { const wrapper = shallow(); diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/layout/page_template.tsx b/x-pack/plugins/enterprise_search/public/applications/shared/layout/page_template.tsx index 08fd28799d69b4..affec119215455 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/layout/page_template.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/shared/layout/page_template.tsx @@ -20,6 +20,7 @@ import { import { FlashMessages } from '../flash_messages'; import { HttpLogic } from '../http'; +import { BreadcrumbTrail } from '../kibana_chrome/generate_breadcrumbs'; import { Loading } from '../loading'; import './page_template.scss'; @@ -39,6 +40,10 @@ export type PageTemplateProps = KibanaPageTemplateProps & { hideFlashMessages?: boolean; isLoading?: boolean; emptyState?: React.ReactNode; + setPageChrome?: React.ReactNode; + // Used by product-specific page templates + pageChrome?: BreadcrumbTrail; + pageViewTelemetry?: string; }; export const EnterpriseSearchPageTemplate: React.FC = ({ @@ -48,6 +53,7 @@ export const EnterpriseSearchPageTemplate: React.FC = ({ isLoading, isEmptyState, emptyState, + setPageChrome, solutionNav, ...pageTemplateProps }) => { @@ -70,6 +76,7 @@ export const EnterpriseSearchPageTemplate: React.FC = ({ isEmptyState={isEmptyState && !isLoading} solutionNav={solutionNav ? { icon: 'logoEnterpriseSearch', ...solutionNav } : undefined} > + {setPageChrome} {readOnlyMode && ( <> { + it('renders', () => { + const wrapper = shallow( + +
world
+ + ); + + expect(wrapper.type()).toEqual(EnterpriseSearchPageTemplate); + expect(wrapper.prop('solutionNav')).toEqual({ name: 'Workplace Search', items: [] }); + expect(wrapper.find('.hello').text()).toEqual('world'); + }); + + describe('page chrome', () => { + it('takes a breadcrumb array & renders a product-specific page chrome', () => { + const wrapper = shallow(); + const setPageChrome = wrapper.find(EnterpriseSearchPageTemplate).prop('setPageChrome') as any; + + expect(setPageChrome.type).toEqual(SetWorkplaceSearchChrome); + expect(setPageChrome.props.trail).toEqual(['Some page']); + }); + }); + + describe('page telemetry', () => { + it('takes a metric & renders product-specific telemetry viewed event', () => { + const wrapper = shallow(); + + expect(wrapper.find(SendWorkplaceSearchTelemetry).prop('action')).toEqual('viewed'); + expect(wrapper.find(SendWorkplaceSearchTelemetry).prop('metric')).toEqual('some_page'); + }); + }); + + describe('props', () => { + it('allows overriding the restrictWidth default', () => { + const wrapper = shallow(); + expect(wrapper.find(EnterpriseSearchPageTemplate).prop('restrictWidth')).toEqual(true); + + wrapper.setProps({ restrictWidth: false }); + expect(wrapper.find(EnterpriseSearchPageTemplate).prop('restrictWidth')).toEqual(false); + }); + + it('passes down any ...pageTemplateProps that EnterpriseSearchPageTemplate accepts', () => { + const wrapper = shallow( + } + /> + ); + + expect(wrapper.find(EnterpriseSearchPageTemplate).prop('pageHeader')!.pageTitle).toEqual( + 'hello world' + ); + expect(wrapper.find(EnterpriseSearchPageTemplate).prop('isLoading')).toEqual(false); + expect(wrapper.find(EnterpriseSearchPageTemplate).prop('emptyState')).toEqual(
); + }); + }); +}); diff --git a/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/page_template.tsx b/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/page_template.tsx new file mode 100644 index 00000000000000..7a86f7904c92ed --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/page_template.tsx @@ -0,0 +1,37 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React from 'react'; + +import { WORKPLACE_SEARCH_PLUGIN } from '../../../../../common/constants'; +import { SetWorkplaceSearchChrome } from '../../../shared/kibana_chrome'; +import { EnterpriseSearchPageTemplate, PageTemplateProps } from '../../../shared/layout'; +import { SendWorkplaceSearchTelemetry } from '../../../shared/telemetry'; + +export const WorkplaceSearchPageTemplate: React.FC = ({ + children, + pageChrome, + pageViewTelemetry, + ...pageTemplateProps +}) => { + return ( + } + > + {pageViewTelemetry && ( + + )} + {children} + + ); +}; From 09c249714a7a11096ff1e26778bcb201cdfbf017 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Tue, 15 Jun 2021 01:28:09 -0700 Subject: [PATCH 3/8] Set up navigation helpers for EuiSideNav usage - Update react_router_helpers to pass back props as a plain JS obj instead of only working with React components (+ update react components to use new simpler helper) - Convert SideNavLink active logic to a plain JS helper --- .../applications/shared/layout/index.ts | 1 + .../shared/layout/nav_link_helpers.test.ts | 69 ++++++++++++++++ .../shared/layout/nav_link_helpers.ts | 40 ++++++++++ .../eui_components.test.tsx | 72 +++-------------- .../react_router_helpers/eui_components.tsx | 79 ++----------------- .../generate_react_router_props.test.ts | 70 ++++++++++++++++ .../generate_react_router_props.ts | 55 +++++++++++++ .../shared/react_router_helpers/index.ts | 1 + 8 files changed, 256 insertions(+), 131 deletions(-) create mode 100644 x-pack/plugins/enterprise_search/public/applications/shared/layout/nav_link_helpers.test.ts create mode 100644 x-pack/plugins/enterprise_search/public/applications/shared/layout/nav_link_helpers.ts create mode 100644 x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/generate_react_router_props.test.ts create mode 100644 x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/generate_react_router_props.ts diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/layout/index.ts b/x-pack/plugins/enterprise_search/public/applications/shared/layout/index.ts index 6f5b7b724cef41..856d483e174a69 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/layout/index.ts +++ b/x-pack/plugins/enterprise_search/public/applications/shared/layout/index.ts @@ -6,6 +6,7 @@ */ export { EnterpriseSearchPageTemplate, PageTemplateProps } from './page_template'; +export { generateNavLink } from './nav_link_helpers'; // TODO: Delete these once KibanaPageTemplate migration is done export { Layout } from './layout'; diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/layout/nav_link_helpers.test.ts b/x-pack/plugins/enterprise_search/public/applications/shared/layout/nav_link_helpers.test.ts new file mode 100644 index 00000000000000..b51416ac76ca78 --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/shared/layout/nav_link_helpers.test.ts @@ -0,0 +1,69 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { mockKibanaValues } from '../../__mocks__/kea_logic'; + +jest.mock('../react_router_helpers', () => ({ + generateReactRouterProps: ({ to }: { to: string }) => ({ + href: `/app/enterprise_search${to}`, + onClick: () => mockKibanaValues.navigateToUrl(to), + }), +})); + +import { generateNavLink, getNavLinkActive } from './nav_link_helpers'; + +describe('generateNavLink', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockKibanaValues.history.location.pathname = '/current_page'; + }); + + it('generates React Router props & isSelected (active) state for use within an EuiSideNavItem obj', () => { + const navItem = generateNavLink({ to: '/test' }); + + expect(navItem.href).toEqual('/app/enterprise_search/test'); + + navItem.onClick({} as any); + expect(mockKibanaValues.navigateToUrl).toHaveBeenCalledWith('/test'); + + expect(navItem.isSelected).toEqual(false); + }); + + describe('getNavLinkActive', () => { + it('returns true when the current path matches the link path', () => { + mockKibanaValues.history.location.pathname = '/test'; + const isSelected = getNavLinkActive({ to: '/test' }); + + expect(isSelected).toEqual(true); + }); + + describe('isRoot', () => { + it('returns true if the current path is "/"', () => { + mockKibanaValues.history.location.pathname = '/'; + const isSelected = getNavLinkActive({ to: '/overview', isRoot: true }); + + expect(isSelected).toEqual(true); + }); + }); + + describe('shouldShowActiveForSubroutes', () => { + it('returns true if the current path is a subroute of the passed path', () => { + mockKibanaValues.history.location.pathname = '/hello/world'; + const isSelected = getNavLinkActive({ to: '/hello', shouldShowActiveForSubroutes: true }); + + expect(isSelected).toEqual(true); + }); + + it('returns false if not', () => { + mockKibanaValues.history.location.pathname = '/hello/world'; + const isSelected = getNavLinkActive({ to: '/hello' }); + + expect(isSelected).toEqual(false); + }); + }); + }); +}); diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/layout/nav_link_helpers.ts b/x-pack/plugins/enterprise_search/public/applications/shared/layout/nav_link_helpers.ts new file mode 100644 index 00000000000000..6124636af3f992 --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/shared/layout/nav_link_helpers.ts @@ -0,0 +1,40 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { stripTrailingSlash } from '../../../../common/strip_slashes'; + +import { KibanaLogic } from '../kibana'; +import { generateReactRouterProps, ReactRouterProps } from '../react_router_helpers'; + +interface Params { + to: string; + isRoot?: boolean; + shouldShowActiveForSubroutes?: boolean; +} + +export const generateNavLink = ({ to, ...rest }: Params & ReactRouterProps) => { + return { + ...generateReactRouterProps({ to, ...rest }), + isSelected: getNavLinkActive({ to, ...rest }), + }; +}; + +export const getNavLinkActive = ({ + to, + isRoot = false, + shouldShowActiveForSubroutes = false, +}: Params): boolean => { + const { pathname } = KibanaLogic.values.history.location; + const currentPath = stripTrailingSlash(pathname); + + const isActive = + currentPath === to || + (shouldShowActiveForSubroutes && currentPath.startsWith(to)) || + (isRoot && currentPath === ''); + + return isActive; +}; diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/eui_components.test.tsx b/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/eui_components.test.tsx index 7fded20cdd87e6..a04e628e0c4f9c 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/eui_components.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/eui_components.test.tsx @@ -5,22 +5,22 @@ * 2.0. */ -import { mockKibanaValues } from '../../__mocks__/kea_logic'; -import { mockHistory } from '../../__mocks__/react_router'; +jest.mock('./', () => ({ + generateReactRouterProps: ({ to }: { to: string }) => ({ + href: `/app/enterprise_search${to}`, + onClick: () => {}, + }), +})); import React from 'react'; -import { shallow, mount } from 'enzyme'; +import { shallow } from 'enzyme'; import { EuiLink, EuiButton, EuiButtonEmpty, EuiPanel, EuiCard } from '@elastic/eui'; import { EuiLinkTo, EuiButtonTo, EuiButtonEmptyTo, EuiPanelTo, EuiCardTo } from './eui_components'; -describe('EUI & React Router Component Helpers', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - +describe('React Router EUI component helpers', () => { it('renders an EuiLink', () => { const wrapper = shallow(); @@ -54,64 +54,18 @@ describe('EUI & React Router Component Helpers', () => { }); it('passes down all ...rest props', () => { - const wrapper = shallow(); + const wrapper = shallow(); const link = wrapper.find(EuiLink); expect(link.prop('external')).toEqual(true); - expect(link.prop('data-test-subj')).toEqual('foo'); + expect(link.prop('data-test-subj')).toEqual('test'); }); - it('renders with the correct href and onClick props', () => { - const wrapper = mount(); + it('renders with generated href and onClick props', () => { + const wrapper = shallow(); const link = wrapper.find(EuiLink); expect(link.prop('onClick')).toBeInstanceOf(Function); - expect(link.prop('href')).toEqual('/app/enterprise_search/foo/bar'); - expect(mockHistory.createHref).toHaveBeenCalled(); - }); - - it('renders with the correct non-basenamed href when shouldNotCreateHref is passed', () => { - const wrapper = mount(); - const link = wrapper.find(EuiLink); - - expect(link.prop('href')).toEqual('/foo/bar'); - expect(mockHistory.createHref).not.toHaveBeenCalled(); - }); - - describe('onClick', () => { - it('prevents default navigation and uses React Router history', () => { - const wrapper = mount(); - - const simulatedEvent = { - button: 0, - target: { getAttribute: () => '_self' }, - preventDefault: jest.fn(), - }; - wrapper.find(EuiLink).simulate('click', simulatedEvent); - - expect(simulatedEvent.preventDefault).toHaveBeenCalled(); - expect(mockKibanaValues.navigateToUrl).toHaveBeenCalled(); - }); - - it('does not prevent default browser behavior on new tab/window clicks', () => { - const wrapper = mount(); - - const simulatedEvent = { - shiftKey: true, - target: { getAttribute: () => '_blank' }, - }; - wrapper.find(EuiLink).simulate('click', simulatedEvent); - - expect(mockKibanaValues.navigateToUrl).not.toHaveBeenCalled(); - }); - - it('calls inherited onClick actions in addition to default navigation', () => { - const customOnClick = jest.fn(); // Can be anything from telemetry to a state reset - const wrapper = mount(); - - wrapper.find(EuiLink).simulate('click', { shiftKey: true }); - - expect(customOnClick).toHaveBeenCalled(); - }); + expect(link.prop('href')).toEqual('/app/enterprise_search/hello/world'); }); }); diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/eui_components.tsx b/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/eui_components.tsx index b9fee9d16273b8..e7eb36f279fc75 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/eui_components.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/eui_components.tsx @@ -7,8 +7,6 @@ import React from 'react'; -import { useValues } from 'kea'; - import { EuiLink, EuiButton, @@ -22,55 +20,10 @@ import { } from '@elastic/eui'; import { EuiPanelProps } from '@elastic/eui/src/components/panel/panel'; -import { HttpLogic } from '../http'; -import { KibanaLogic } from '../kibana'; - -import { letBrowserHandleEvent, createHref } from './'; +import { generateReactRouterProps, ReactRouterProps } from './'; /** - * Generates EUI components with React-Router-ified links - * - * Based off of EUI's recommendations for handling React Router: - * https://github.com/elastic/eui/blob/master/wiki/react-router.md#react-router-51 - */ - -interface ReactRouterProps { - to: string; - onClick?(): void; - // Used to navigate outside of the React Router plugin basename but still within Kibana, - // e.g. if we need to go from Enterprise Search to App Search - shouldNotCreateHref?: boolean; -} - -export const ReactRouterHelper: React.FC = ({ - to, - onClick, - shouldNotCreateHref, - children, -}) => { - const { navigateToUrl, history } = useValues(KibanaLogic); - const { http } = useValues(HttpLogic); - - // Generate the correct link href (with basename etc. accounted for) - const href = createHref(to, { history, http }, { shouldNotCreateHref }); - - const reactRouterLinkClick = (event: React.MouseEvent) => { - if (onClick) onClick(); // Run any passed click events (e.g. telemetry) - if (letBrowserHandleEvent(event)) return; // Return early if the link behavior shouldn't be handled by React Router - - // Prevent regular link behavior, which causes a browser refresh. - event.preventDefault(); - - // Perform SPA navigation. - navigateToUrl(to, { shouldNotCreateHref }); - }; - - const reactRouterProps = { href, onClick: reactRouterLinkClick }; - return React.cloneElement(children as React.ReactElement, reactRouterProps); -}; - -/** - * Component helpers + * Correctly typed component helpers with React-Router-friendly `href` and `onClick` props */ type ReactRouterEuiLinkProps = ReactRouterProps & EuiLinkAnchorProps; @@ -79,11 +32,7 @@ export const EuiLinkTo: React.FC = ({ onClick, shouldNotCreateHref, ...rest -}) => ( - - - -); +}) => ; type ReactRouterEuiButtonProps = ReactRouterProps & EuiButtonProps; export const EuiButtonTo: React.FC = ({ @@ -91,11 +40,7 @@ export const EuiButtonTo: React.FC = ({ onClick, shouldNotCreateHref, ...rest -}) => ( - - - -); +}) => ; type ReactRouterEuiButtonEmptyProps = ReactRouterProps & EuiButtonEmptyProps; export const EuiButtonEmptyTo: React.FC = ({ @@ -104,9 +49,7 @@ export const EuiButtonEmptyTo: React.FC = ({ shouldNotCreateHref, ...rest }) => ( - - - + ); type ReactRouterEuiPanelProps = ReactRouterProps & EuiPanelProps; @@ -115,11 +58,7 @@ export const EuiPanelTo: React.FC = ({ onClick, shouldNotCreateHref, ...rest -}) => ( - - - -); +}) => ; type ReactRouterEuiCardProps = ReactRouterProps & EuiCardProps; export const EuiCardTo: React.FC = ({ @@ -127,8 +66,4 @@ export const EuiCardTo: React.FC = ({ onClick, shouldNotCreateHref, ...rest -}) => ( - - - -); +}) => ; diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/generate_react_router_props.test.ts b/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/generate_react_router_props.test.ts new file mode 100644 index 00000000000000..dc8bf28a444071 --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/generate_react_router_props.test.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 + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { mockKibanaValues } from '../../__mocks__/kea_logic'; +import { mockHistory } from '../../__mocks__/react_router'; + +import { generateReactRouterProps } from './'; + +describe('generateReactRouterProps', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('generates React-Router-friendly href and onClick props', () => { + expect(generateReactRouterProps({ to: '/hello/world' })).toEqual({ + href: '/app/enterprise_search/hello/world', + onClick: expect.any(Function), + }); + expect(mockHistory.createHref).toHaveBeenCalled(); + }); + + it('renders with the correct non-basenamed href when shouldNotCreateHref is passed', () => { + expect(generateReactRouterProps({ to: '/hello/world', shouldNotCreateHref: true })).toEqual({ + href: '/hello/world', + onClick: expect.any(Function), + }); + }); + + describe('onClick', () => { + it('prevents default navigation and uses React Router history', () => { + const mockEvent = { + button: 0, + target: { getAttribute: () => '_self' }, + preventDefault: jest.fn(), + } as any; + + const { onClick } = generateReactRouterProps({ to: '/test' }); + onClick(mockEvent); + + expect(mockEvent.preventDefault).toHaveBeenCalled(); + expect(mockKibanaValues.navigateToUrl).toHaveBeenCalled(); + }); + + it('does not prevent default browser behavior on new tab/window clicks', () => { + const mockEvent = { + shiftKey: true, + target: { getAttribute: () => '_blank' }, + } as any; + + const { onClick } = generateReactRouterProps({ to: '/test' }); + onClick(mockEvent); + + expect(mockKibanaValues.navigateToUrl).not.toHaveBeenCalled(); + }); + + it('calls inherited onClick actions in addition to default navigation', () => { + const mockEvent = { preventDefault: jest.fn() } as any; + const customOnClick = jest.fn(); // Can be anything from telemetry to a state reset + + const { onClick } = generateReactRouterProps({ to: '/test', onClick: customOnClick }); + onClick(mockEvent); + + expect(customOnClick).toHaveBeenCalled(); + }); + }); +}); diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/generate_react_router_props.ts b/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/generate_react_router_props.ts new file mode 100644 index 00000000000000..d80eca19207bd5 --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/generate_react_router_props.ts @@ -0,0 +1,55 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React from 'react'; + +import { HttpLogic } from '../http'; +import { KibanaLogic } from '../kibana'; + +import { letBrowserHandleEvent, createHref } from './'; + +/** + * Generates the `href` and `onClick` props for React-Router-friendly links + * + * Based off of EUI's recommendations for handling React Router: + * https://github.com/elastic/eui/blob/master/wiki/react-router.md#react-router-51 + * + * but separated out from EuiLink portion as we use this for multiple EUI components + */ + +export interface ReactRouterProps { + to: string; + onClick?(): void; + // Used to navigate outside of the React Router plugin basename but still within Kibana, + // e.g. if we need to go from Enterprise Search to App Search + shouldNotCreateHref?: boolean; +} + +export const generateReactRouterProps = ({ + to, + onClick, + shouldNotCreateHref, +}: ReactRouterProps) => { + const { navigateToUrl, history } = KibanaLogic.values; + const { http } = HttpLogic.values; + + // Generate the correct link href (with basename etc. accounted for) + const href = createHref(to, { history, http }, { shouldNotCreateHref }); + + const reactRouterLinkClick = (event: React.MouseEvent) => { + if (onClick) onClick(); // Run any passed click events (e.g. telemetry) + if (letBrowserHandleEvent(event)) return; // Return early if the link behavior shouldn't be handled by React Router + + // Prevent regular link behavior, which causes a browser refresh. + event.preventDefault(); + + // Perform SPA navigation. + navigateToUrl(to, { shouldNotCreateHref }); + }; + + return { href, onClick: reactRouterLinkClick }; +}; diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/index.ts b/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/index.ts index 1a73c9c281b21f..17827b02302377 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/index.ts +++ b/x-pack/plugins/enterprise_search/public/applications/shared/react_router_helpers/index.ts @@ -7,4 +7,5 @@ export { letBrowserHandleEvent } from './link_events'; export { createHref, CreateHrefOptions } from './create_href'; +export { generateReactRouterProps, ReactRouterProps } from './generate_react_router_props'; export { EuiLinkTo, EuiButtonTo, EuiButtonEmptyTo, EuiPanelTo, EuiCardTo } from './eui_components'; From d1861c0aaaf0ade8de4e8cc9e0084ee85076758c Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Tue, 15 Jun 2021 01:28:42 -0700 Subject: [PATCH 4/8] Set up top-level product navigations NYI: sub navigations (future separate PRs) --- .../app_search/components/layout/index.ts | 1 + .../app_search/components/layout/nav.test.tsx | 70 +++++++++++++++++++ .../app_search/components/layout/nav.tsx | 60 ++++++++++++++++ .../components/layout/page_template.test.tsx | 4 ++ .../components/layout/page_template.tsx | 4 +- .../components/layout/index.ts | 2 +- .../components/layout/nav.test.tsx | 49 ++++++++++++- .../components/layout/nav.tsx | 45 +++++++++++- .../components/layout/page_template.test.tsx | 4 ++ .../components/layout/page_template.tsx | 4 +- 10 files changed, 236 insertions(+), 7 deletions(-) create mode 100644 x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/nav.test.tsx create mode 100644 x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/nav.tsx diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/index.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/index.ts index 177eb5de5bb672..a7699848831b25 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/index.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/index.ts @@ -6,4 +6,5 @@ */ export { AppSearchPageTemplate } from './page_template'; +export { useAppSearchNav } from './nav'; export { KibanaHeaderActions } from './kibana_header_actions'; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/nav.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/nav.test.tsx new file mode 100644 index 00000000000000..ac3c31b0f88fb6 --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/nav.test.tsx @@ -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 + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { setMockValues } from '../../../__mocks__/kea_logic'; + +jest.mock('../../../shared/layout', () => ({ + generateNavLink: jest.fn(({ to }) => ({ href: to })), +})); + +import { useAppSearchNav } from './nav'; + +describe('useAppSearchNav', () => { + const MOCK_DEFAULT_NAV = [ + { + id: 'engines', + name: 'Engines', + href: '/engines', + items: [], + }, + ]; + + it('always generates a default engines nav item', () => { + setMockValues({ myRole: {} }); + + expect(useAppSearchNav()).toEqual(MOCK_DEFAULT_NAV); + }); + + it('generates a settings nav item if the user can view settings', () => { + setMockValues({ myRole: { canViewSettings: true } }); + + expect(useAppSearchNav()).toEqual([ + ...MOCK_DEFAULT_NAV, + { + id: 'settings', + name: 'Settings', + href: '/settings', + }, + ]); + }); + + it('generates a credentials nav item if the user can view credentials', () => { + setMockValues({ myRole: { canViewAccountCredentials: true } }); + + expect(useAppSearchNav()).toEqual([ + ...MOCK_DEFAULT_NAV, + { + id: 'credentials', + name: 'Credentials', + href: '/credentials', + }, + ]); + }); + + it('generates a users & roles nav item if the user can view role mappings', () => { + setMockValues({ myRole: { canViewRoleMappings: true } }); + + expect(useAppSearchNav()).toEqual([ + ...MOCK_DEFAULT_NAV, + { + id: 'usersRoles', + name: 'Users & roles', + href: '/role_mappings', + }, + ]); + }); +}); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/nav.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/nav.tsx new file mode 100644 index 00000000000000..313108e86b0414 --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/nav.tsx @@ -0,0 +1,60 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { useValues } from 'kea'; + +import { EuiSideNavItemType } from '@elastic/eui'; + +import { generateNavLink } from '../../../shared/layout'; +import { ROLE_MAPPINGS_TITLE } from '../../../shared/role_mapping/constants'; + +import { AppLogic } from '../../app_logic'; +import { ENGINES_PATH, SETTINGS_PATH, CREDENTIALS_PATH, ROLE_MAPPINGS_PATH } from '../../routes'; +import { CREDENTIALS_TITLE } from '../credentials'; +import { ENGINES_TITLE } from '../engines'; +import { SETTINGS_TITLE } from '../settings'; + +export const useAppSearchNav = () => { + const { + myRole: { canViewSettings, canViewAccountCredentials, canViewRoleMappings }, + } = useValues(AppLogic); + + const navItems: Array> = [ + { + id: 'engines', + name: ENGINES_TITLE, + ...generateNavLink({ to: ENGINES_PATH, isRoot: true }), + items: [], // TODO: Engine nav + }, + ]; + + if (canViewSettings) { + navItems.push({ + id: 'settings', + name: SETTINGS_TITLE, + ...generateNavLink({ to: SETTINGS_PATH }), + }); + } + + if (canViewAccountCredentials) { + navItems.push({ + id: 'credentials', + name: CREDENTIALS_TITLE, + ...generateNavLink({ to: CREDENTIALS_PATH }), + }); + } + + if (canViewRoleMappings) { + navItems.push({ + id: 'usersRoles', + name: ROLE_MAPPINGS_TITLE, + ...generateNavLink({ to: ROLE_MAPPINGS_PATH }), + }); + } + + return navItems; +}; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/page_template.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/page_template.test.tsx index 2a996c845e83f2..8f47d5f1c46444 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/page_template.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/page_template.test.tsx @@ -5,6 +5,10 @@ * 2.0. */ +jest.mock('./nav', () => ({ + useAppSearchNav: () => [], +})); + import React from 'react'; import { shallow } from 'enzyme'; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/page_template.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/page_template.tsx index d89544e7040311..31f2eb3215e05a 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/page_template.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/page_template.tsx @@ -12,6 +12,8 @@ import { SetAppSearchChrome } from '../../../shared/kibana_chrome'; import { EnterpriseSearchPageTemplate, PageTemplateProps } from '../../../shared/layout'; import { SendAppSearchTelemetry } from '../../../shared/telemetry'; +import { useAppSearchNav } from './nav'; + export const AppSearchPageTemplate: React.FC = ({ children, pageChrome, @@ -23,7 +25,7 @@ export const AppSearchPageTemplate: React.FC = ({ {...pageTemplateProps} solutionNav={{ name: APP_SEARCH_PLUGIN.NAME, - items: [], // TODO: Nav items + items: useAppSearchNav(), }} setPageChrome={pageChrome && } > diff --git a/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/index.ts b/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/index.ts index 7b3b7a1fff2ad2..8cdc1336817629 100644 --- a/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/index.ts +++ b/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/index.ts @@ -6,7 +6,7 @@ */ export { WorkplaceSearchPageTemplate } from './page_template'; -export { WorkplaceSearchNav } from './nav'; +export { useWorkplaceSearchNav, WorkplaceSearchNav } from './nav'; export { WorkplaceSearchHeaderActions } from './kibana_header_actions'; export { AccountHeader } from './account_header'; export { PersonalDashboardLayout } from './personal_dashboard_layout'; diff --git a/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/nav.test.tsx b/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/nav.test.tsx index 8f37f608f4e282..db38ac977276fd 100644 --- a/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/nav.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/nav.test.tsx @@ -5,7 +5,10 @@ * 2.0. */ -import '../../../__mocks__/enterprise_search_url.mock'; +jest.mock('../../../shared/layout', () => ({ + ...jest.requireActual('../../../shared/layout'), + generateNavLink: jest.fn(({ to }) => ({ href: to })), +})); import React from 'react'; @@ -13,7 +16,49 @@ import { shallow } from 'enzyme'; import { SideNav, SideNavLink } from '../../../shared/layout'; -import { WorkplaceSearchNav } from './'; +import { useWorkplaceSearchNav, WorkplaceSearchNav } from './'; + +describe('useWorkplaceSearchNav', () => { + it('returns an array of top-level Workplace Search nav items', () => { + expect(useWorkplaceSearchNav()).toEqual([ + { + id: 'root', + name: 'Overview', + href: '/', + }, + { + id: 'sources', + name: 'Sources', + href: '/sources', + items: [], + }, + { + id: 'groups', + name: 'Groups', + href: '/groups', + items: [], + }, + { + id: 'usersRoles', + name: 'Users & roles', + href: '/role_mappings', + }, + { + id: 'security', + name: 'Security', + href: '/security', + }, + { + id: 'settings', + name: 'Settings', + href: '/settings', + items: [], + }, + ]); + }); +}); + +// TODO: Delete below once fully migrated to KibanaPageTemplate describe('WorkplaceSearchNav', () => { it('renders', () => { diff --git a/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/nav.tsx b/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/nav.tsx index fb3c8556029b25..5232de8a03bfc7 100644 --- a/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/nav.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/nav.tsx @@ -7,10 +7,10 @@ import React from 'react'; -import { EuiSpacer } from '@elastic/eui'; +import { EuiSideNavItemType, EuiSpacer } from '@elastic/eui'; import { WORKPLACE_SEARCH_PLUGIN } from '../../../../../common/constants'; -import { SideNav, SideNavLink } from '../../../shared/layout'; +import { generateNavLink, SideNav, SideNavLink } from '../../../shared/layout'; import { NAV } from '../../constants'; import { SOURCES_PATH, @@ -20,6 +20,47 @@ import { ORG_SETTINGS_PATH, } from '../../routes'; +export const useWorkplaceSearchNav = () => { + const navItems: Array> = [ + { + id: 'root', + name: NAV.OVERVIEW, + ...generateNavLink({ to: '/', isRoot: true }), + }, + { + id: 'sources', + name: NAV.SOURCES, + ...generateNavLink({ to: SOURCES_PATH }), + items: [], // TODO: Source subnav + }, + { + id: 'groups', + name: NAV.GROUPS, + ...generateNavLink({ to: GROUPS_PATH }), + items: [], // TODO: Group subnav + }, + { + id: 'usersRoles', + name: NAV.ROLE_MAPPINGS, + ...generateNavLink({ to: ROLE_MAPPINGS_PATH }), + }, + { + id: 'security', + name: NAV.SECURITY, + ...generateNavLink({ to: SECURITY_PATH }), + }, + { + id: 'settings', + name: NAV.SETTINGS, + ...generateNavLink({ to: ORG_SETTINGS_PATH }), + items: [], // TODO: Settings subnav + }, + ]; + return navItems; +}; + +// TODO: Delete below once fully migrated to KibanaPageTemplate + interface Props { sourcesSubNav?: React.ReactNode; groupsSubNav?: React.ReactNode; diff --git a/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/page_template.test.tsx b/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/page_template.test.tsx index 716e35490e4ce5..622fddc449ca7d 100644 --- a/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/page_template.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/page_template.test.tsx @@ -5,6 +5,10 @@ * 2.0. */ +jest.mock('./nav', () => ({ + useWorkplaceSearchNav: () => [], +})); + import React from 'react'; import { shallow } from 'enzyme'; diff --git a/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/page_template.tsx b/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/page_template.tsx index 7a86f7904c92ed..4a6e0d9c6e2ddc 100644 --- a/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/page_template.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/page_template.tsx @@ -12,6 +12,8 @@ import { SetWorkplaceSearchChrome } from '../../../shared/kibana_chrome'; import { EnterpriseSearchPageTemplate, PageTemplateProps } from '../../../shared/layout'; import { SendWorkplaceSearchTelemetry } from '../../../shared/telemetry'; +import { useWorkplaceSearchNav } from './nav'; + export const WorkplaceSearchPageTemplate: React.FC = ({ children, pageChrome, @@ -24,7 +26,7 @@ export const WorkplaceSearchPageTemplate: React.FC = ({ {...pageTemplateProps} solutionNav={{ name: WORKPLACE_SEARCH_PLUGIN.NAME, - items: [], // TODO: Nav items + items: useWorkplaceSearchNav(), }} setPageChrome={pageChrome && } > From 16753424ee94def9918c21aad99b552a5e2e4b97 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Tue, 15 Jun 2021 01:31:23 -0700 Subject: [PATCH 5/8] Set up test_helpers for inspecting pageHeaders - primarily useful for rightSideItems, which often contain conditional logic --- .../test_helpers/get_page_header.tsx | 43 +++++++++++++++++++ .../public/applications/test_helpers/index.ts | 6 +++ 2 files changed, 49 insertions(+) create mode 100644 x-pack/plugins/enterprise_search/public/applications/test_helpers/get_page_header.tsx diff --git a/x-pack/plugins/enterprise_search/public/applications/test_helpers/get_page_header.tsx b/x-pack/plugins/enterprise_search/public/applications/test_helpers/get_page_header.tsx new file mode 100644 index 00000000000000..6e89274dca5703 --- /dev/null +++ b/x-pack/plugins/enterprise_search/public/applications/test_helpers/get_page_header.tsx @@ -0,0 +1,43 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import React, { Fragment } from 'react'; + +import { shallow, ShallowWrapper } from 'enzyme'; + +import { EuiPageHeaderProps } from '@elastic/eui'; + +/* + * Given an AppSearchPageTemplate or WorkplaceSearchPageTemplate, these + * helpers dive into various parts of the EuiPageHeader to make assertions + * slightly less of a pain in shallow renders + */ + +export const getPageHeader = (wrapper: ShallowWrapper) => { + const pageHeader = wrapper.prop('pageHeader') as EuiPageHeaderProps; + return pageHeader || {}; +}; + +export const getPageTitle = (wrapper: ShallowWrapper) => { + return getPageHeader(wrapper).pageTitle; +}; + +export const getPageDescription = (wrapper: ShallowWrapper) => { + return getPageHeader(wrapper).description; +}; + +export const getPageHeaderActions = (wrapper: ShallowWrapper) => { + const actions = getPageHeader(wrapper).rightSideItems || []; + + return shallow( +
+ {actions.map((action: React.ReactNode, i) => ( + {action} + ))} +
+ ); +}; diff --git a/x-pack/plugins/enterprise_search/public/applications/test_helpers/index.ts b/x-pack/plugins/enterprise_search/public/applications/test_helpers/index.ts index e34ff763637b5a..ed5c3f85a888ee 100644 --- a/x-pack/plugins/enterprise_search/public/applications/test_helpers/index.ts +++ b/x-pack/plugins/enterprise_search/public/applications/test_helpers/index.ts @@ -10,6 +10,12 @@ export { mountAsync } from './mount_async'; export { mountWithIntl } from './mount_with_i18n'; export { shallowWithIntl } from './shallow_with_i18n'; export { rerender } from './enzyme_rerender'; +export { + getPageHeader, + getPageTitle, + getPageDescription, + getPageHeaderActions, +} from './get_page_header'; // Misc export { expectedAsyncError } from './expected_async_error'; From 4b1d526857bcc0390b75e2383dfcbf67c363a2fd Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Tue, 15 Jun 2021 10:36:44 -0700 Subject: [PATCH 6/8] Initial example: Convert RoleMappings views to new page template Minor refactors: + remove unnecessary type union + fix un-i18n'ed product names + add full stop to documentation sentence + add semantic HTML tags around various page landmarks (header, section) --- .../role_mappings/role_mappings.test.tsx | 8 ------ .../role_mappings/role_mappings.tsx | 26 ++++++++++--------- .../public/applications/app_search/index.tsx | 10 +++---- .../shared/role_mapping/constants.ts | 6 ++--- .../role_mapping/role_mappings_heading.tsx | 8 +++--- .../public/applications/shared/types.ts | 2 -- .../applications/workplace_search/index.tsx | 4 +-- .../role_mappings/role_mappings.test.tsx | 8 ------ .../views/role_mappings/role_mappings.tsx | 26 ++++++++++--------- 9 files changed, 39 insertions(+), 59 deletions(-) diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/role_mappings/role_mappings.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/role_mappings/role_mappings.test.tsx index d7ce8053c71f02..308022ccb2e5a7 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/role_mappings/role_mappings.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/role_mappings/role_mappings.test.tsx @@ -12,7 +12,6 @@ import React from 'react'; import { shallow } from 'enzyme'; -import { Loading } from '../../../shared/loading'; import { RoleMappingsTable, RoleMappingsHeading } from '../../../shared/role_mapping'; import { wsRoleMapping } from '../../../shared/role_mapping/__mocks__/roles'; @@ -44,13 +43,6 @@ describe('RoleMappings', () => { expect(wrapper.find(RoleMappingsTable)).toHaveLength(1); }); - it('returns Loading when loading', () => { - setMockValues({ ...mockValues, dataLoading: true }); - const wrapper = shallow(); - - expect(wrapper.find(Loading)).toHaveLength(1); - }); - it('renders RoleMapping flyout', () => { setMockValues({ ...mockValues, roleMappingFlyoutOpen: true }); const wrapper = shallow(); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/role_mappings/role_mappings.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/role_mappings/role_mappings.tsx index 78d0a5cbc8638e..db0e6e6dead111 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/role_mappings/role_mappings.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/role_mappings/role_mappings.tsx @@ -9,11 +9,10 @@ import React, { useEffect } from 'react'; import { useActions, useValues } from 'kea'; -import { FlashMessages } from '../../../shared/flash_messages'; -import { SetAppSearchChrome as SetPageChrome } from '../../../shared/kibana_chrome'; -import { Loading } from '../../../shared/loading'; +import { APP_SEARCH_PLUGIN } from '../../../../../common/constants'; import { RoleMappingsTable, RoleMappingsHeading } from '../../../shared/role_mapping'; import { ROLE_MAPPINGS_TITLE } from '../../../shared/role_mapping/constants'; +import { AppSearchPageTemplate } from '../layout'; import { ROLE_MAPPINGS_ENGINE_ACCESS_HEADING } from './constants'; import { RoleMapping } from './role_mapping'; @@ -38,11 +37,12 @@ export const RoleMappings: React.FC = () => { return resetState; }, []); - if (dataLoading) return ; - const roleMappingsSection = ( - <> - initializeRoleMapping()} /> +
+ initializeRoleMapping()} + /> { shouldShowAuthProvider={multipleAuthProvidersConfig} handleDeleteMapping={handleDeleteMapping} /> - +
); return ( - <> - + {roleMappingFlyoutOpen && } - {roleMappingsSection} - + ); }; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/index.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/index.tsx index 20f65ba2c346ca..caf0f805e8ca7e 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/index.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/index.tsx @@ -92,6 +92,11 @@ export const AppSearchConfigured: React.FC> = (props) = )} + {canViewRoleMappings && ( + + + + )} } readOnlyMode={readOnlyMode}> @@ -110,11 +115,6 @@ export const AppSearchConfigured: React.FC> = (props) = - {canViewRoleMappings && ( - - - - )} {canManageEngines && ( diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/constants.ts b/x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/constants.ts index 47d481630510e2..e5312c6c0343f7 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/constants.ts +++ b/x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/constants.ts @@ -7,8 +7,6 @@ import { i18n } from '@kbn/i18n'; -import { ProductName } from '../types'; - export const ANY_AUTH_PROVIDER = '*'; export const ANY_AUTH_PROVIDER_OPTION_LABEL = i18n.translate( @@ -184,7 +182,7 @@ export const ROLE_MAPPINGS_HEADING_TITLE = i18n.translate( { defaultMessage: 'Role mappings' } ); -export const ROLE_MAPPINGS_HEADING_DESCRIPTION = (productName: ProductName) => +export const ROLE_MAPPINGS_HEADING_DESCRIPTION = (productName: string) => i18n.translate('xpack.enterpriseSearch.roleMapping.roleMappingsHeadingDescription', { defaultMessage: 'Role mappings provide an interface to associate native or SAML-governed role attributes with {productName} permissions.', @@ -193,7 +191,7 @@ export const ROLE_MAPPINGS_HEADING_DESCRIPTION = (productName: ProductName) => export const ROLE_MAPPINGS_HEADING_DOCS_LINK = i18n.translate( 'xpack.enterpriseSearch.roleMapping.roleMappingsHeadingDocsLink', - { defaultMessage: 'Learn more about role mappings' } + { defaultMessage: 'Learn more about role mappings.' } ); export const ROLE_MAPPINGS_HEADING_BUTTON = i18n.translate( diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/role_mappings_heading.tsx b/x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/role_mappings_heading.tsx index b2143c6ff44028..d36bdfda06d31c 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/role_mappings_heading.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/role_mappings_heading.tsx @@ -17,8 +17,6 @@ import { EuiTitle, } from '@elastic/eui'; -import { ProductName } from '../types'; - import { ROLE_MAPPINGS_HEADING_TITLE, ROLE_MAPPINGS_HEADING_DESCRIPTION, @@ -27,7 +25,7 @@ import { } from './constants'; interface Props { - productName: ProductName; + productName: string; onClick(): void; } @@ -35,7 +33,7 @@ interface Props { const ROLE_MAPPINGS_DOCS_HREF = '#TODO'; export const RoleMappingsHeading: React.FC = ({ productName, onClick }) => ( - <> +
@@ -58,5 +56,5 @@ export const RoleMappingsHeading: React.FC = ({ productName, onClick }) = - +
); diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/types.ts b/x-pack/plugins/enterprise_search/public/applications/shared/types.ts index f450ca556ebe25..2cdadc1c6b0d30 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/types.ts +++ b/x-pack/plugins/enterprise_search/public/applications/shared/types.ts @@ -35,5 +35,3 @@ export interface RoleMapping { content: string; }; } - -export type ProductName = 'App Search' | 'Workplace Search'; diff --git a/x-pack/plugins/enterprise_search/public/applications/workplace_search/index.tsx b/x-pack/plugins/enterprise_search/public/applications/workplace_search/index.tsx index 7e911b31c516b7..dd263c3bd69f5d 100644 --- a/x-pack/plugins/enterprise_search/public/applications/workplace_search/index.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/workplace_search/index.tsx @@ -141,9 +141,7 @@ export const WorkplaceSearchConfigured: React.FC = (props) => { - } restrictWidth readOnlyMode={readOnlyMode}> - - + } restrictWidth readOnlyMode={readOnlyMode}> diff --git a/x-pack/plugins/enterprise_search/public/applications/workplace_search/views/role_mappings/role_mappings.test.tsx b/x-pack/plugins/enterprise_search/public/applications/workplace_search/views/role_mappings/role_mappings.test.tsx index d7ce8053c71f02..308022ccb2e5a7 100644 --- a/x-pack/plugins/enterprise_search/public/applications/workplace_search/views/role_mappings/role_mappings.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/workplace_search/views/role_mappings/role_mappings.test.tsx @@ -12,7 +12,6 @@ import React from 'react'; import { shallow } from 'enzyme'; -import { Loading } from '../../../shared/loading'; import { RoleMappingsTable, RoleMappingsHeading } from '../../../shared/role_mapping'; import { wsRoleMapping } from '../../../shared/role_mapping/__mocks__/roles'; @@ -44,13 +43,6 @@ describe('RoleMappings', () => { expect(wrapper.find(RoleMappingsTable)).toHaveLength(1); }); - it('returns Loading when loading', () => { - setMockValues({ ...mockValues, dataLoading: true }); - const wrapper = shallow(); - - expect(wrapper.find(Loading)).toHaveLength(1); - }); - it('renders RoleMapping flyout', () => { setMockValues({ ...mockValues, roleMappingFlyoutOpen: true }); const wrapper = shallow(); diff --git a/x-pack/plugins/enterprise_search/public/applications/workplace_search/views/role_mappings/role_mappings.tsx b/x-pack/plugins/enterprise_search/public/applications/workplace_search/views/role_mappings/role_mappings.tsx index 46c426c3dad2a5..b153d012241939 100644 --- a/x-pack/plugins/enterprise_search/public/applications/workplace_search/views/role_mappings/role_mappings.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/workplace_search/views/role_mappings/role_mappings.tsx @@ -9,11 +9,10 @@ import React, { useEffect } from 'react'; import { useActions, useValues } from 'kea'; -import { FlashMessages } from '../../../shared/flash_messages'; -import { SetWorkplaceSearchChrome as SetPageChrome } from '../../../shared/kibana_chrome'; -import { Loading } from '../../../shared/loading'; +import { WORKPLACE_SEARCH_PLUGIN } from '../../../../../common/constants'; import { RoleMappingsTable, RoleMappingsHeading } from '../../../shared/role_mapping'; import { ROLE_MAPPINGS_TITLE } from '../../../shared/role_mapping/constants'; +import { WorkplaceSearchPageTemplate } from '../../components/layout'; import { ROLE_MAPPINGS_TABLE_HEADER } from './constants'; @@ -36,11 +35,12 @@ export const RoleMappings: React.FC = () => { initializeRoleMappings(); }, []); - if (dataLoading) return ; - const roleMappingsSection = ( - <> - initializeRoleMapping()} /> +
+ initializeRoleMapping()} + /> { initializeRoleMapping={initializeRoleMapping} handleDeleteMapping={handleDeleteMapping} /> - +
); return ( - <> - + {roleMappingFlyoutOpen && } - {roleMappingsSection} - + ); }; From 74956bd20274343cfe71132c2365515d3a97b2d7 Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 16 Jun 2021 09:58:54 -0700 Subject: [PATCH 7/8] EUI feedback: add empty root parent section --- .../app_search/components/layout/nav.test.tsx | 81 ++++++++++++++----- .../app_search/components/layout/nav.tsx | 5 +- .../components/layout/nav.test.tsx | 68 +++++++++------- .../components/layout/nav.tsx | 6 +- 4 files changed, 105 insertions(+), 55 deletions(-) diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/nav.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/nav.test.tsx index ac3c31b0f88fb6..8b06f4b26835d4 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/nav.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/nav.test.tsx @@ -14,30 +14,45 @@ jest.mock('../../../shared/layout', () => ({ import { useAppSearchNav } from './nav'; describe('useAppSearchNav', () => { - const MOCK_DEFAULT_NAV = [ - { - id: 'engines', - name: 'Engines', - href: '/engines', - items: [], - }, - ]; - it('always generates a default engines nav item', () => { setMockValues({ myRole: {} }); - expect(useAppSearchNav()).toEqual(MOCK_DEFAULT_NAV); + expect(useAppSearchNav()).toEqual([ + { + id: '', + name: '', + items: [ + { + id: 'engines', + name: 'Engines', + href: '/engines', + items: [], + }, + ], + }, + ]); }); it('generates a settings nav item if the user can view settings', () => { setMockValues({ myRole: { canViewSettings: true } }); expect(useAppSearchNav()).toEqual([ - ...MOCK_DEFAULT_NAV, { - id: 'settings', - name: 'Settings', - href: '/settings', + id: '', + name: '', + items: [ + { + id: 'engines', + name: 'Engines', + href: '/engines', + items: [], + }, + { + id: 'settings', + name: 'Settings', + href: '/settings', + }, + ], }, ]); }); @@ -46,11 +61,22 @@ describe('useAppSearchNav', () => { setMockValues({ myRole: { canViewAccountCredentials: true } }); expect(useAppSearchNav()).toEqual([ - ...MOCK_DEFAULT_NAV, { - id: 'credentials', - name: 'Credentials', - href: '/credentials', + id: '', + name: '', + items: [ + { + id: 'engines', + name: 'Engines', + href: '/engines', + items: [], + }, + { + id: 'credentials', + name: 'Credentials', + href: '/credentials', + }, + ], }, ]); }); @@ -59,11 +85,22 @@ describe('useAppSearchNav', () => { setMockValues({ myRole: { canViewRoleMappings: true } }); expect(useAppSearchNav()).toEqual([ - ...MOCK_DEFAULT_NAV, { - id: 'usersRoles', - name: 'Users & roles', - href: '/role_mappings', + id: '', + name: '', + items: [ + { + id: 'engines', + name: 'Engines', + href: '/engines', + items: [], + }, + { + id: 'usersRoles', + name: 'Users & roles', + href: '/role_mappings', + }, + ], }, ]); }); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/nav.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/nav.tsx index 313108e86b0414..57fa740caebec2 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/nav.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/layout/nav.tsx @@ -56,5 +56,8 @@ export const useAppSearchNav = () => { }); } - return navItems; + // Root level items are meant to be section headers, but the AS nav (currently) + // isn't organized this way. So we create a fake empty parent item here + // to cause all our navItems to properly render as nav links. + return [{ id: '', name: '', items: navItems }]; }; diff --git a/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/nav.test.tsx b/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/nav.test.tsx index db38ac977276fd..90da5b3163ecfc 100644 --- a/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/nav.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/nav.test.tsx @@ -22,37 +22,43 @@ describe('useWorkplaceSearchNav', () => { it('returns an array of top-level Workplace Search nav items', () => { expect(useWorkplaceSearchNav()).toEqual([ { - id: 'root', - name: 'Overview', - href: '/', - }, - { - id: 'sources', - name: 'Sources', - href: '/sources', - items: [], - }, - { - id: 'groups', - name: 'Groups', - href: '/groups', - items: [], - }, - { - id: 'usersRoles', - name: 'Users & roles', - href: '/role_mappings', - }, - { - id: 'security', - name: 'Security', - href: '/security', - }, - { - id: 'settings', - name: 'Settings', - href: '/settings', - items: [], + id: '', + name: '', + items: [ + { + id: 'root', + name: 'Overview', + href: '/', + }, + { + id: 'sources', + name: 'Sources', + href: '/sources', + items: [], + }, + { + id: 'groups', + name: 'Groups', + href: '/groups', + items: [], + }, + { + id: 'usersRoles', + name: 'Users & roles', + href: '/role_mappings', + }, + { + id: 'security', + name: 'Security', + href: '/security', + }, + { + id: 'settings', + name: 'Settings', + href: '/settings', + items: [], + }, + ], }, ]); }); diff --git a/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/nav.tsx b/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/nav.tsx index 5232de8a03bfc7..8e7b13a6218214 100644 --- a/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/nav.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/workplace_search/components/layout/nav.tsx @@ -56,7 +56,11 @@ export const useWorkplaceSearchNav = () => { items: [], // TODO: Settings subnav }, ]; - return navItems; + + // Root level items are meant to be section headers, but the WS nav (currently) + // isn't organized this way. So we crate a fake empty parent item here + // to cause all our navItems to properly render as nav links. + return [{ id: '', name: '', items: navItems }]; }; // TODO: Delete below once fully migrated to KibanaPageTemplate From b7475869847a1dafd93b9d437e20178e00e7be3f Mon Sep 17 00:00:00 2001 From: Constance Chen Date: Wed, 16 Jun 2021 12:58:03 -0700 Subject: [PATCH 8/8] Revert Role Mappings union type removal - but shenanigans it a bit to take our i18n'd shared product names (requires as const assertion) - done to reduce merge conflicts for Scotty / make his life (hopefully) a bit easier between ent-search and Kibana --- .../public/applications/shared/role_mapping/constants.ts | 4 +++- .../shared/role_mapping/role_mappings_heading.tsx | 4 +++- .../enterprise_search/public/applications/shared/types.ts | 5 +++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/constants.ts b/x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/constants.ts index e5312c6c0343f7..9f40844e52470a 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/constants.ts +++ b/x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/constants.ts @@ -7,6 +7,8 @@ import { i18n } from '@kbn/i18n'; +import { ProductName } from '../types'; + export const ANY_AUTH_PROVIDER = '*'; export const ANY_AUTH_PROVIDER_OPTION_LABEL = i18n.translate( @@ -182,7 +184,7 @@ export const ROLE_MAPPINGS_HEADING_TITLE = i18n.translate( { defaultMessage: 'Role mappings' } ); -export const ROLE_MAPPINGS_HEADING_DESCRIPTION = (productName: string) => +export const ROLE_MAPPINGS_HEADING_DESCRIPTION = (productName: ProductName) => i18n.translate('xpack.enterpriseSearch.roleMapping.roleMappingsHeadingDescription', { defaultMessage: 'Role mappings provide an interface to associate native or SAML-governed role attributes with {productName} permissions.', diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/role_mappings_heading.tsx b/x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/role_mappings_heading.tsx index d36bdfda06d31c..eee8b180d32819 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/role_mappings_heading.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/shared/role_mapping/role_mappings_heading.tsx @@ -17,6 +17,8 @@ import { EuiTitle, } from '@elastic/eui'; +import { ProductName } from '../types'; + import { ROLE_MAPPINGS_HEADING_TITLE, ROLE_MAPPINGS_HEADING_DESCRIPTION, @@ -25,7 +27,7 @@ import { } from './constants'; interface Props { - productName: string; + productName: ProductName; onClick(): void; } diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/types.ts b/x-pack/plugins/enterprise_search/public/applications/shared/types.ts index 2cdadc1c6b0d30..67208c63ddf4cc 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/types.ts +++ b/x-pack/plugins/enterprise_search/public/applications/shared/types.ts @@ -5,6 +5,8 @@ * 2.0. */ +import { APP_SEARCH_PLUGIN, WORKPLACE_SEARCH_PLUGIN } from '../../../common/constants'; + import { ADD, UPDATE } from './constants/operations'; export type TOperation = typeof ADD | typeof UPDATE; @@ -35,3 +37,6 @@ export interface RoleMapping { content: string; }; } + +const productNames = [APP_SEARCH_PLUGIN.NAME, WORKPLACE_SEARCH_PLUGIN.NAME] as const; +export type ProductName = typeof productNames[number];