diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine_creation/engine_creation.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine_creation/engine_creation.tsx index 913aa4f0ec84..18b839008146 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine_creation/engine_creation.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engine_creation/engine_creation.tsx @@ -22,6 +22,7 @@ import { EuiButton, } from '@elastic/eui'; +import { ENGINES_TITLE } from '../engines'; import { AppSearchPageTemplate } from '../layout'; import { @@ -43,7 +44,7 @@ export const EngineCreation: React.FC = () => { return ( diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/components/empty_state.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/components/empty_state.test.tsx index 159a986096ae..9117fdd0be87 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/components/empty_state.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/components/empty_state.test.tsx @@ -53,7 +53,7 @@ describe('EmptyState', () => { }); it('sends a user to engine creation', () => { - expect(button.prop('to')).toEqual('/engine_creation'); + expect(button.prop('to')).toEqual('/engines/new'); }); }); }); 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 80230394ce2a..c9f5452e254e 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 @@ -8,7 +8,7 @@ import { setMockValues } from '../../../__mocks__/kea_logic'; jest.mock('../../../shared/layout', () => ({ - generateNavLink: jest.fn(({ to }) => ({ href: to })), + generateNavLink: jest.fn(({ to, items }) => ({ href: to, items })), })); jest.mock('../engine/engine_nav', () => ({ useEngineNav: () => [], 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 4737fbcf07e2..c3b8ec642233 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 @@ -28,8 +28,12 @@ export const useAppSearchNav = () => { { id: 'engines', name: ENGINES_TITLE, - ...generateNavLink({ to: ENGINES_PATH, isRoot: true }), - items: useEngineNav(), + ...generateNavLink({ + to: ENGINES_PATH, + isRoot: true, + shouldShowActiveForSubroutes: true, + items: useEngineNav(), + }), }, ]; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/meta_engine_creation/meta_engine_creation.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/meta_engine_creation/meta_engine_creation.tsx index 325e557acec0..1455444ab2b4 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/meta_engine_creation/meta_engine_creation.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/meta_engine_creation/meta_engine_creation.tsx @@ -25,6 +25,7 @@ import { } from '@elastic/eui'; import { AppLogic } from '../../app_logic'; +import { ENGINES_TITLE } from '../engines'; import { AppSearchPageTemplate } from '../layout'; import { @@ -73,7 +74,7 @@ export const MetaEngineCreation: React.FC = () => { return ( > = (props) = - - - {canManageEngines && ( @@ -117,6 +114,9 @@ export const AppSearchConfigured: React.FC> = (props) = )} + + + {canViewSettings && ( diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/routes.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/routes.ts index bd5bdb7b2f66..d9d1935c648f 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/routes.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/routes.ts @@ -18,7 +18,7 @@ export const CREDENTIALS_PATH = '/credentials'; export const ROLE_MAPPINGS_PATH = '/role_mappings'; export const ENGINES_PATH = '/engines'; -export const ENGINE_CREATION_PATH = '/engine_creation'; +export const ENGINE_CREATION_PATH = `${ENGINES_PATH}/new`; // This is safe from conflicting with an :engineName path because new is a reserved name export const ENGINE_PATH = `${ENGINES_PATH}/:engineName`; export const ENGINE_ANALYTICS_PATH = `${ENGINE_PATH}/analytics`; @@ -39,7 +39,7 @@ export const ENGINE_REINDEX_JOB_PATH = `${ENGINE_SCHEMA_PATH}/reindex_job/:reind export const ENGINE_CRAWLER_PATH = `${ENGINE_PATH}/crawler`; export const ENGINE_CRAWLER_DOMAIN_PATH = `${ENGINE_CRAWLER_PATH}/domains/:domainId`; -export const META_ENGINE_CREATION_PATH = '/meta_engine_creation'; +export const META_ENGINE_CREATION_PATH = `${ENGINES_PATH}/new_meta_engine`; // This is safe from conflicting with an :engineName path because engine names cannot have underscores export const META_ENGINE_SOURCE_ENGINES_PATH = `${ENGINE_PATH}/engines`; export const ENGINE_RELEVANCE_TUNING_PATH = `${ENGINE_PATH}/relevance_tuning`; 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 index b51416ac76ca..8cfca3bade99 100644 --- 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 @@ -19,21 +19,23 @@ import { generateNavLink, getNavLinkActive } from './nav_link_helpers'; describe('generateNavLink', () => { beforeEach(() => { jest.clearAllMocks(); - mockKibanaValues.history.location.pathname = '/current_page'; + mockKibanaValues.history.location.pathname = '/'; }); - it('generates React Router props & isSelected (active) state for use within an EuiSideNavItem obj', () => { + it('generates React Router props for use within an EuiSideNavItem obj', () => { const navItem = generateNavLink({ to: '/test' }); - expect(navItem.href).toEqual('/app/enterprise_search/test'); + expect(navItem).toEqual({ + href: '/app/enterprise_search/test', + onClick: expect.any(Function), + isSelected: false, + }); navItem.onClick({} as any); expect(mockKibanaValues.navigateToUrl).toHaveBeenCalledWith('/test'); - - expect(navItem.isSelected).toEqual(false); }); - describe('getNavLinkActive', () => { + describe('isSelected / getNavLinkActive', () => { it('returns true when the current path matches the link path', () => { mockKibanaValues.history.location.pathname = '/test'; const isSelected = getNavLinkActive({ to: '/test' }); @@ -41,6 +43,13 @@ describe('generateNavLink', () => { expect(isSelected).toEqual(true); }); + it('return false when the current path does not match the link path', () => { + mockKibanaValues.history.location.pathname = '/hello'; + const isSelected = getNavLinkActive({ to: '/world' }); + + expect(isSelected).toEqual(false); + }); + describe('isRoot', () => { it('returns true if the current path is "/"', () => { mockKibanaValues.history.location.pathname = '/'; @@ -58,7 +67,31 @@ describe('generateNavLink', () => { expect(isSelected).toEqual(true); }); - it('returns false if not', () => { + /* NOTE: This logic is primarily used for the following routing scenario: + * 1. /item/{itemId} shows a child subnav, e.g. /items/{itemId}/settings + * - BUT when the child subnav is open, the parent `Item` nav link should not show as active - its child nav links should + * 2. /item/create_item (example) does *not* show a child subnav + * - BUT the parent `Item` nav link should highlight when on this non-subnav route + */ + it('returns false if subroutes already have their own items subnav (with active state)', () => { + mockKibanaValues.history.location.pathname = '/items/123/settings'; + const isSelected = getNavLinkActive({ + to: '/items', + shouldShowActiveForSubroutes: true, + items: [{ id: 'settings', name: 'Settings' }], + }); + + expect(isSelected).toEqual(false); + }); + + it('returns false if not a valid subroute', () => { + mockKibanaValues.history.location.pathname = '/hello/world'; + const isSelected = getNavLinkActive({ to: '/world', shouldShowActiveForSubroutes: true }); + + expect(isSelected).toEqual(false); + }); + + it('returns false for subroutes if the flag is not passed', () => { mockKibanaValues.history.location.pathname = '/hello/world'; const isSelected = getNavLinkActive({ to: '/hello' }); @@ -66,4 +99,10 @@ describe('generateNavLink', () => { }); }); }); + + it('optionally passes items', () => { + const navItem = generateNavLink({ to: '/test', items: [] }); + + expect(navItem.items).toEqual([]); + }); }); 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 index 6124636af3f9..9caf58886c52 100644 --- 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 @@ -5,6 +5,8 @@ * 2.0. */ +import { EuiSideNavItemType } from '@elastic/eui'; + import { stripTrailingSlash } from '../../../../common/strip_slashes'; import { KibanaLogic } from '../kibana'; @@ -14,12 +16,14 @@ interface Params { to: string; isRoot?: boolean; shouldShowActiveForSubroutes?: boolean; + items?: Array>; // Primarily passed if using `items` to determine isSelected - if not, you can just set `items` outside of this helper } -export const generateNavLink = ({ to, ...rest }: Params & ReactRouterProps) => { +export const generateNavLink = ({ to, items, ...rest }: Params & ReactRouterProps) => { return { ...generateReactRouterProps({ to, ...rest }), - isSelected: getNavLinkActive({ to, ...rest }), + isSelected: getNavLinkActive({ to, items, ...rest }), + items, }; }; @@ -27,14 +31,19 @@ export const getNavLinkActive = ({ to, isRoot = false, shouldShowActiveForSubroutes = false, + items = [], }: Params): boolean => { const { pathname } = KibanaLogic.values.history.location; const currentPath = stripTrailingSlash(pathname); - const isActive = - currentPath === to || - (shouldShowActiveForSubroutes && currentPath.startsWith(to)) || - (isRoot && currentPath === ''); + if (currentPath === to) return true; - return isActive; + if (isRoot && currentPath === '') return true; + + if (shouldShowActiveForSubroutes) { + if (items.length) return false; // If a nav link has sub-nav items open, never show it as active + if (currentPath.startsWith(to)) return true; + } + + return false; }; 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 04b0880a7351..f2601ff98db1 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 @@ -7,7 +7,7 @@ jest.mock('../../../shared/layout', () => ({ ...jest.requireActual('../../../shared/layout'), - generateNavLink: jest.fn(({ to }) => ({ href: to })), + generateNavLink: jest.fn(({ to, items }) => ({ href: to, items })), })); jest.mock('../../views/content_sources/components/source_sub_nav', () => ({ useSourceSubNav: () => [], 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 99225bc36e89..ce2f8bf7ef7e 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 @@ -33,8 +33,11 @@ export const useWorkplaceSearchNav = () => { { id: 'sources', name: NAV.SOURCES, - ...generateNavLink({ to: SOURCES_PATH }), - items: useSourceSubNav(), + ...generateNavLink({ + to: SOURCES_PATH, + shouldShowActiveForSubroutes: true, + items: useSourceSubNav(), + }), }, { id: 'groups',