Skip to content

Commit

Permalink
Refactor observability plugin breadcrumbs (#102290) (#102447)
Browse files Browse the repository at this point in the history
Previously the observability plugin set the page title and breadcrumbs in the main app rendering component based on the `breadcrumb` property of the current route.

In addition, there's a `useBreadcrumb` hook used by the UX app, exploratory view, and cases.

The conflict between these was creating situations where neither would work and the breadcrumbs would just show "Kibana".

Remove the breadcrumb properties from the routes and the main app breadcrumb handling and just use `useBreadcrumb` on all pages.

Fixes #102131.

Co-authored-by: Nathan L Smith <nathan.smith@elastic.co>
  • Loading branch information
kibanamachine and smith committed Jun 17, 2021
1 parent ad2ebc7 commit 9836afd
Show file tree
Hide file tree
Showing 14 changed files with 211 additions and 141 deletions.
40 changes: 7 additions & 33 deletions x-pack/plugins/observability/public/application/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,25 @@
*/

import { i18n } from '@kbn/i18n';
import React, { MouseEvent, useEffect } from 'react';
import React from 'react';
import ReactDOM from 'react-dom';
import { Route, Router, Switch } from 'react-router-dom';
import { EuiThemeProvider } from '../../../../../src/plugins/kibana_react/common';
import { ConfigSchema } from '..';
import { AppMountParameters, APP_WRAPPER_CLASS, CoreStart } from '../../../../../src/core/public';
import { EuiThemeProvider } from '../../../../../src/plugins/kibana_react/common';
import {
KibanaContextProvider,
RedirectAppLinks,
} from '../../../../../src/plugins/kibana_react/public';
import { Storage } from '../../../../../src/plugins/kibana_utils/public';
import type { LazyObservabilityPageTemplateProps } from '../components/shared/page_template/lazy_page_template';
import { HasDataContextProvider } from '../context/has_data_context';
import { PluginContext } from '../context/plugin_context';
import { usePluginContext } from '../hooks/use_plugin_context';
import { useRouteParams } from '../hooks/use_route_params';
import { ObservabilityPublicPluginsStart } from '../plugin';
import type { LazyObservabilityPageTemplateProps } from '../components/shared/page_template/lazy_page_template';
import { HasDataContextProvider } from '../context/has_data_context';
import { Breadcrumbs, routes } from '../routes';
import { Storage } from '../../../../../src/plugins/kibana_utils/public';
import { ConfigSchema } from '..';
import { routes } from '../routes';
import { ObservabilityRuleTypeRegistry } from '../rules/create_observability_rule_type_registry';

function getTitleFromBreadCrumbs(breadcrumbs: Breadcrumbs) {
return breadcrumbs.map(({ text }) => text).reverse();
}

function App() {
return (
<>
Expand All @@ -38,27 +33,6 @@ function App() {
const path = key as keyof typeof routes;
const route = routes[path];
const Wrapper = () => {
const { core } = usePluginContext();

useEffect(() => {
const href = core.http.basePath.prepend('/app/observability');
const breadcrumbs = [
{
href,
text: i18n.translate('xpack.observability.observability.breadcrumb.', {
defaultMessage: 'Observability',
}),
onClick: (event: MouseEvent<HTMLAnchorElement>) => {
event.preventDefault();
core.application.navigateToUrl(href);
},
},
...route.breadcrumb,
];
core.chrome.setBreadcrumbs(breadcrumbs);
core.chrome.docTitle.change(getTitleFromBreadCrumbs(breadcrumbs));
}, [core]);

const params = useRouteParams(path);
return route.handler(params);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import React, { useCallback, useState } from 'react';
import {
casesBreadcrumbs,
getCaseDetailsUrl,
getCaseDetailsUrlWithCommentId,
getCaseUrl,
Expand All @@ -17,7 +18,7 @@ import { Case } from '../../../../../../cases/common';
import { useFetchAlertData } from './helpers';
import { useKibana } from '../../../../utils/kibana_react';
import { CASES_APP_ID } from '../constants';
import { casesBreadcrumbs, useBreadcrumbs } from '../../../../hooks/use_breadcrumbs';
import { useBreadcrumbs } from '../../../../hooks/use_breadcrumbs';

interface Props {
caseId: string;
Expand Down
116 changes: 116 additions & 0 deletions x-pack/plugins/observability/public/hooks/use_breadcrumbs.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
/*
* 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 { renderHook } from '@testing-library/react-hooks';
import React, { ReactNode } from 'react';
import { MemoryRouter } from 'react-router-dom';
import { CoreStart } from '../../../../../src/core/public';
import { createKibanaReactContext } from '../../../../../src/plugins/kibana_react/public';
import { useBreadcrumbs } from './use_breadcrumbs';

const setBreadcrumbs = jest.fn();
const setTitle = jest.fn();
const kibanaServices = ({
application: { getUrlForApp: () => {}, navigateToApp: () => {} },
chrome: { setBreadcrumbs, docTitle: { change: setTitle } },
uiSettings: { get: () => true },
} as unknown) as Partial<CoreStart>;
const KibanaContext = createKibanaReactContext(kibanaServices);

function Wrapper({ children }: { children?: ReactNode }) {
return (
<MemoryRouter>
<KibanaContext.Provider>{children}</KibanaContext.Provider>
</MemoryRouter>
);
}

describe('useBreadcrumbs', () => {
afterEach(() => {
setBreadcrumbs.mockClear();
setTitle.mockClear();
});

describe('when setBreadcrumbs and setTitle are not defined', () => {
it('does not set breadcrumbs or the title', () => {
renderHook(() => useBreadcrumbs([]), {
wrapper: ({ children }) => (
<MemoryRouter>
<KibanaContext.Provider
services={
({ ...kibanaServices, chrome: { docTitle: {} } } as unknown) as Partial<CoreStart>
}
>
{children}
</KibanaContext.Provider>
</MemoryRouter>
),
});

expect(setBreadcrumbs).not.toHaveBeenCalled();
expect(setTitle).not.toHaveBeenCalled();
});
});

describe('with an empty array', () => {
it('sets the overview breadcrumb', () => {
renderHook(() => useBreadcrumbs([]), { wrapper: Wrapper });

expect(setBreadcrumbs).toHaveBeenCalledWith([
{ href: '/overview', onClick: expect.any(Function), text: 'Observability' },
]);
});

it('sets the overview title', () => {
renderHook(() => useBreadcrumbs([]), { wrapper: Wrapper });

expect(setTitle).toHaveBeenCalledWith(['Observability']);
});
});

describe('given breadcrumbs', () => {
it('sets the breadcrumbs', () => {
renderHook(
() =>
useBreadcrumbs([
{ text: 'One', href: '/one' },
{
text: 'Two',
},
]),
{ wrapper: Wrapper }
);

expect(setBreadcrumbs).toHaveBeenCalledWith([
{ href: '/overview', onClick: expect.any(Function), text: 'Observability' },
{
href: '/one',
onClick: expect.any(Function),
text: 'One',
},
{
text: 'Two',
},
]);
});

it('sets the title', () => {
renderHook(
() =>
useBreadcrumbs([
{ text: 'One', href: '/one' },
{
text: 'Two',
},
]),
{ wrapper: Wrapper }
);

expect(setTitle).toHaveBeenCalledWith(['Two', 'One', 'Observability']);
});
});
});
62 changes: 23 additions & 39 deletions x-pack/plugins/observability/public/hooks/use_breadcrumbs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@
* 2.0.
*/

import { ChromeBreadcrumb } from 'kibana/public';
import { i18n } from '@kbn/i18n';
import { ChromeBreadcrumb } from 'kibana/public';
import { MouseEvent, useEffect } from 'react';
import { EuiBreadcrumb } from '@elastic/eui';
import { useQueryParams } from './use_query_params';
import { useKibana } from '../utils/kibana_react';
import { useQueryParams } from './use_query_params';

function handleBreadcrumbClick(
function addClickHandlers(
breadcrumbs: ChromeBreadcrumb[],
navigateToHref?: (url: string) => Promise<void>
) {
Expand All @@ -31,52 +30,37 @@ function handleBreadcrumbClick(
}));
}

export const makeBaseBreadcrumb = (href: string): EuiBreadcrumb => {
return {
text: i18n.translate('xpack.observability.breadcrumbs.observability', {
defaultMessage: 'Observability',
}),
href,
};
};
export const casesBreadcrumbs = {
cases: {
text: i18n.translate('xpack.observability.breadcrumbs.observability.cases', {
defaultMessage: 'Cases',
}),
},
create: {
text: i18n.translate('xpack.observability.breadcrumbs.observability.cases.create', {
defaultMessage: 'Create',
}),
},
configure: {
text: i18n.translate('xpack.observability.breadcrumbs.observability.cases.configure', {
defaultMessage: 'Configure',
}),
},
};
function getTitleFromBreadCrumbs(breadcrumbs: ChromeBreadcrumb[]) {
return breadcrumbs.map(({ text }) => text?.toString() ?? '').reverse();
}

export const useBreadcrumbs = (extraCrumbs: ChromeBreadcrumb[]) => {
const params = useQueryParams();

const {
services: {
chrome: { setBreadcrumbs },
chrome: { docTitle, setBreadcrumbs },
application: { getUrlForApp, navigateToUrl },
},
} = useKibana();

const setTitle = docTitle.change;
const appPath = getUrlForApp('observability-overview') ?? '';
const navigate = navigateToUrl;

useEffect(() => {
const breadcrumbs = [
{
text: i18n.translate('xpack.observability.breadcrumbs.observabilityLinkText', {
defaultMessage: 'Observability',
}),
href: appPath + '/overview',
},
...extraCrumbs,
];
if (setBreadcrumbs) {
setBreadcrumbs(
handleBreadcrumbClick(
[makeBaseBreadcrumb(appPath + '/overview')].concat(extraCrumbs),
navigate
)
);
setBreadcrumbs(addClickHandlers(breadcrumbs, navigateToUrl));
}
if (setTitle) {
setTitle(getTitleFromBreadCrumbs(breadcrumbs));
}
}, [appPath, extraCrumbs, navigate, params, setBreadcrumbs]);
}, [appPath, extraCrumbs, navigateToUrl, params, setBreadcrumbs, setTitle]);
};
9 changes: 9 additions & 0 deletions x-pack/plugins/observability/public/pages/alerts/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { useHistory } from 'react-router-dom';
import { ParsedTechnicalFields } from '../../../../rule_registry/common/parse_technical_fields';
import type { AlertStatus } from '../../../common/typings';
import { ExperimentalBadge } from '../../components/shared/experimental_badge';
import { useBreadcrumbs } from '../../hooks/use_breadcrumbs';
import { useFetcher } from '../../hooks/use_fetcher';
import { usePluginContext } from '../../hooks/use_plugin_context';
import { RouteParams } from '../../routes';
Expand Down Expand Up @@ -44,6 +45,14 @@ export function AlertsPage({ routeParams }: AlertsPageProps) {
query: { rangeFrom = 'now-15m', rangeTo = 'now', kuery = '', status = 'open' },
} = routeParams;

useBreadcrumbs([
{
text: i18n.translate('xpack.observability.breadcrumbs.alertsLinkText', {
defaultMessage: 'Alerts',
}),
},
]);

// In a future milestone we'll have a page dedicated to rule management in
// observability. For now link to the settings page.
const manageDetectionRulesHref = prepend(
Expand Down
5 changes: 5 additions & 0 deletions x-pack/plugins/observability/public/pages/cases/all_cases.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,15 @@ import { permissionsReadOnlyErrorMessage, CaseCallOut } from '../../components/a
import { CaseFeatureNoPermissions } from './feature_no_permissions';
import { useGetUserCasesPermissions } from '../../hooks/use_get_user_cases_permissions';
import { usePluginContext } from '../../hooks/use_plugin_context';
import { casesBreadcrumbs } from './links';
import { useBreadcrumbs } from '../../hooks/use_breadcrumbs';

export const AllCasesPage = React.memo(() => {
const userPermissions = useGetUserCasesPermissions();
const { ObservabilityPageTemplate } = usePluginContext();

useBreadcrumbs([casesBreadcrumbs.cases]);

return userPermissions == null || userPermissions?.read ? (
<>
{userPermissions != null && !userPermissions?.crud && userPermissions?.read && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import { CASES_APP_ID, CASES_OWNER } from '../../components/app/cases/constants'
import { useKibana } from '../../utils/kibana_react';
import { useGetUserCasesPermissions } from '../../hooks/use_get_user_cases_permissions';
import { usePluginContext } from '../../hooks/use_plugin_context';
import { casesBreadcrumbs, useBreadcrumbs } from '../../hooks/use_breadcrumbs';
import { getCaseUrl, useFormatUrl } from './links';
import { useBreadcrumbs } from '../../hooks/use_breadcrumbs';
import { casesBreadcrumbs, getCaseUrl, useFormatUrl } from './links';

const ButtonEmpty = styled(EuiButtonEmpty)`
display: block;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import { CASES_APP_ID } from '../../components/app/cases/constants';
import { useKibana } from '../../utils/kibana_react';
import { useGetUserCasesPermissions } from '../../hooks/use_get_user_cases_permissions';
import { usePluginContext } from '../../hooks/use_plugin_context';
import { getCaseUrl, useFormatUrl } from './links';
import { casesBreadcrumbs, useBreadcrumbs } from '../../hooks/use_breadcrumbs';
import { casesBreadcrumbs, getCaseUrl, useFormatUrl } from './links';
import { useBreadcrumbs } from '../../hooks/use_breadcrumbs';

const ButtonEmpty = styled(EuiButtonEmpty)`
display: block;
Expand Down
21 changes: 20 additions & 1 deletion x-pack/plugins/observability/public/pages/cases/links.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,29 @@
* 2.0.
*/

import { useCallback } from 'react';
import { i18n } from '@kbn/i18n';
import { isEmpty } from 'lodash/fp';
import { useCallback } from 'react';
import { useKibana } from '../../utils/kibana_react';

export const casesBreadcrumbs = {
cases: {
text: i18n.translate('xpack.observability.breadcrumbs.casesLinkText', {
defaultMessage: 'Cases',
}),
},
create: {
text: i18n.translate('xpack.observability.breadcrumbs.casesCreateLinkText', {
defaultMessage: 'Create',
}),
},
configure: {
text: i18n.translate('xpack.observability.breadcrumbs.casesConfigureLinkText', {
defaultMessage: 'Configure',
}),
},
};

export const getCaseDetailsUrl = ({ id, subCaseId }: { id: string; subCaseId?: string }) => {
if (subCaseId) {
return `/${encodeURIComponent(id)}/sub-cases/${encodeURIComponent(subCaseId)}`;
Expand Down
Loading

0 comments on commit 9836afd

Please sign in to comment.