Skip to content

Commit

Permalink
[Serverless] fix discover breadcrumbs missing search title (#163607)
Browse files Browse the repository at this point in the history
## Summary

Aaddress #163337 for discover
saved search

### Context:

In serverless navigation, we changed how breadcrumbs work. Instead of
setting the full path manually, we automatically calculate the main
parts of the path from the side nav + current URL. This was done to keep
side nav and breadcrumbs in sync as much as possible and solve
consistency issues with breadcrumbs across apps.

https://docs.elastic.dev/kibana-dev-docs/serverless-project-navigation#breadcrumbs

Apps can append custom deeper context using the
serverless.setBreadcrumbs API. Regular core.chrome.setBreadcrumbs has no
effect when the serverless nav is rendered.

<img width="1624" alt="Screenshot 2023-08-10 at 15 22 08"
src="https://github.com/elastic/kibana/assets/7784120/269879b0-6fcc-4606-816e-5d76616db60a">
  • Loading branch information
Dosant authored Aug 14, 2023
1 parent 281cc22 commit 933f2a5
Show file tree
Hide file tree
Showing 10 changed files with 167 additions and 56 deletions.
5 changes: 3 additions & 2 deletions src/plugins/discover/kibana.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"expressions",
"unifiedSearch",
"unifiedHistogram",
"contentManagement"
"contentManagement",
],
"optionalPlugins": [
"home",
Expand All @@ -35,7 +35,8 @@
"spaces",
"triggersActionsUi",
"savedObjectsTaggingOss",
"lens"
"lens",
"serverless"
],
"requiredBundles": ["kibanaUtils", "kibanaReact", "unifiedSearch"],
"extraPublicDirs": ["common"]
Expand Down
17 changes: 8 additions & 9 deletions src/plugins/discover/public/application/context/context_app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import { ContextAppContent } from './context_app_content';
import { SurrDocType } from './services/context';
import { DocViewFilterFn } from '../../services/doc_views/doc_views_types';
import { useDiscoverServices } from '../../hooks/use_discover_services';
import { getRootBreadcrumbs } from '../../utils/breadcrumbs';
import { setBreadcrumbs } from '../../utils/breadcrumbs';

const ContextAppContentMemoized = memo(ContextAppContent);

Expand Down Expand Up @@ -78,14 +78,13 @@ export const ContextApp = ({ dataView, anchorId, referrer }: ContextAppProps) =>
});

useEffect(() => {
services.chrome.setBreadcrumbs([
...getRootBreadcrumbs({ breadcrumb: referrer, services }),
{
text: i18n.translate('discover.context.breadcrumb', {
defaultMessage: 'Surrounding documents',
}),
},
]);
setBreadcrumbs({
services,
rootBreadcrumbPath: referrer,
titleBreadcrumbText: i18n.translate('discover.context.breadcrumb', {
defaultMessage: 'Surrounding documents',
}),
});
}, [locator, referrer, services]);

useExecutionContext(core.executionContext, {
Expand Down
11 changes: 6 additions & 5 deletions src/plugins/discover/public/application/doc/components/doc.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { EuiCallOut, EuiLink, EuiLoadingSpinner, EuiPage, EuiPageBody } from '@e
import type { DataView } from '@kbn/data-views-plugin/public';
import { i18n } from '@kbn/i18n';
import type { DataTableRecord } from '@kbn/discover-utils/types';
import { getRootBreadcrumbs } from '../../../utils/breadcrumbs';
import { setBreadcrumbs } from '../../../utils/breadcrumbs';
import { DocViewer } from '../../../services/doc_views/components/doc_viewer';
import { ElasticRequestState } from '../types';
import { useEsDocSearch } from '../../../hooks/use_es_doc_search';
Expand Down Expand Up @@ -53,10 +53,11 @@ export function Doc(props: DocProps) {
const indexExistsLink = docLinks.links.apis.indexExists;

useEffect(() => {
chrome.setBreadcrumbs([
...getRootBreadcrumbs({ breadcrumb: props.referrer, services }),
{ text: `${props.index}#${props.id}` },
]);
setBreadcrumbs({
services,
titleBreadcrumbText: `${props.index}#${props.id}`,
rootBreadcrumbPath: props.referrer,
});
}, [chrome, props.referrer, props.index, props.id, dataView, locator, services]);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { RootDragDropProvider } from '@kbn/dom-drag-drop';
import { useUrlTracking } from './hooks/use_url_tracking';
import { DiscoverStateContainer } from './services/discover_state';
import { DiscoverLayout } from './components/layout';
import { setBreadcrumbsTitle } from '../../utils/breadcrumbs';
import { setBreadcrumbs } from '../../utils/breadcrumbs';
import { addHelpMenuToAppChrome } from '../../components/help_menu/help_menu_util';
import { useDiscoverServices } from '../../hooks/use_discover_services';
import { useSavedSearchAliasMatchRedirect } from '../../hooks/saved_search_alias_match_redirect';
Expand Down Expand Up @@ -68,7 +68,7 @@ export function DiscoverMainApp(props: DiscoverMainProps) {
if (mode === 'standalone') {
const pageTitleSuffix = savedSearch.id && savedSearch.title ? `: ${savedSearch.title}` : '';
chrome.docTitle.change(`Discover${pageTitleSuffix}`);
setBreadcrumbsTitle({ title: savedSearch.title, services });
setBreadcrumbs({ titleBreadcrumbText: savedSearch.title, services });
}
}, [mode, chrome.docTitle, savedSearch.id, savedSearch.title, services]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { useSingleton } from './hooks/use_singleton';
import { MainHistoryLocationState } from '../../../common/locator';
import { DiscoverStateContainer, getDiscoverStateContainer } from './services/discover_state';
import { DiscoverMainApp } from './discover_main_app';
import { getRootBreadcrumbs, getSavedSearchBreadcrumbs } from '../../utils/breadcrumbs';
import { setBreadcrumbs } from '../../utils/breadcrumbs';
import { LoadingIndicator } from '../../components/common/loading_indicator';
import { DiscoverError } from '../../components/common/error_alert';
import { useDiscoverServices } from '../../hooks/use_discover_services';
Expand Down Expand Up @@ -161,11 +161,7 @@ export function DiscoverMainRoute({
);
}

chrome.setBreadcrumbs(
currentSavedSearch && currentSavedSearch.title
? getSavedSearchBreadcrumbs({ id: currentSavedSearch.title, services })
: getRootBreadcrumbs({ services })
);
setBreadcrumbs({ services, titleBreadcrumbText: currentSavedSearch?.title ?? undefined });
}
setLoading(false);
if (services.analytics) {
Expand Down
3 changes: 3 additions & 0 deletions src/plugins/discover/public/build_services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import type { LensPublicStart } from '@kbn/lens-plugin/public';
import type { UiActionsStart } from '@kbn/ui-actions-plugin/public';
import type { SettingsStart } from '@kbn/core-ui-settings-browser';
import type { ContentClient } from '@kbn/content-management-plugin/public';
import type { ServerlessPluginStart } from '@kbn/serverless/public';
import { getHistory } from './kibana_services';
import { DiscoverStartPlugins } from './plugin';
import { DiscoverContextAppLocator } from './application/context/services/locator';
Expand Down Expand Up @@ -109,6 +110,7 @@ export interface DiscoverServices {
lens: LensPublicStart;
uiActions: UiActionsStart;
contentClient: ContentClient;
serverless?: ServerlessPluginStart;
}

export const buildServices = memoize(function (
Expand Down Expand Up @@ -168,5 +170,6 @@ export const buildServices = memoize(function (
lens: plugins.lens,
uiActions: plugins.uiActions,
contentClient: plugins.contentManagement.client,
serverless: plugins.serverless,
};
});
2 changes: 2 additions & 0 deletions src/plugins/discover/public/plugin.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import type { SavedSearchPublicPluginStart } from '@kbn/saved-search-plugin/publ
import type { UnifiedSearchPublicPluginStart } from '@kbn/unified-search-plugin/public';
import { setStateToKbnUrl } from '@kbn/kibana-utils-plugin/public';
import type { LensPublicStart } from '@kbn/lens-plugin/public';
import type { ServerlessPluginStart } from '@kbn/serverless/public';
import { DOC_TABLE_LEGACY, TRUNCATE_MAX_HEIGHT } from '@kbn/discover-utils';
import { PLUGIN_ID } from '../common';
import { DocViewInput, DocViewInputFn } from './services/doc_views/doc_views_types';
Expand Down Expand Up @@ -211,6 +212,7 @@ export interface DiscoverStartPlugins {
unifiedSearch: UnifiedSearchPublicPluginStart;
lens: LensPublicStart;
contentManagement: ContentManagementPublicStart;
serverless?: ServerlessPluginStart;
}

/**
Expand Down
113 changes: 113 additions & 0 deletions src/plugins/discover/public/utils/breadcrumbs.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/*
* 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 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { serverlessMock } from '@kbn/serverless/public/mocks';
import { createDiscoverServicesMock } from '../__mocks__/services';
import { setBreadcrumbs } from './breadcrumbs';
import { createMemoryHistory } from 'history';
import type { HistoryLocationState } from '../build_services';

describe('Breadcrumbs', () => {
const discoverServiceMock = createDiscoverServicesMock();
beforeEach(() => {
(discoverServiceMock.chrome.setBreadcrumbs as jest.Mock).mockClear();
});

test('should set breadcrumbs with default root', () => {
setBreadcrumbs({ services: discoverServiceMock });
expect(discoverServiceMock.chrome.setBreadcrumbs).toHaveBeenCalledWith([{ text: 'Discover' }]);
});

test('should set breadcrumbs with title', () => {
setBreadcrumbs({ services: discoverServiceMock, titleBreadcrumbText: 'Saved Search' });
expect(discoverServiceMock.chrome.setBreadcrumbs).toHaveBeenCalledWith([
{ text: 'Discover', href: '#/' },
{ text: 'Saved Search' },
]);
});

test('should set breadcrumbs with custom root path', () => {
setBreadcrumbs({
services: discoverServiceMock,
titleBreadcrumbText: 'Saved Search',
rootBreadcrumbPath: '#/custom-path',
});
expect(discoverServiceMock.chrome.setBreadcrumbs).toHaveBeenCalledWith([
{ text: 'Discover', href: '#/custom-path' },
{ text: 'Saved Search' },
]);
});

test('should set breadcrumbs with profile root path', () => {
setBreadcrumbs({
services: {
...discoverServiceMock,
history: () => {
const history = createMemoryHistory<HistoryLocationState>({});
history.push('/p/my-profile');
return history;
},
},
titleBreadcrumbText: 'Saved Search',
});
expect(discoverServiceMock.chrome.setBreadcrumbs).toHaveBeenCalledWith([
{ text: 'Discover', href: '#/p/my-profile/' },
{ text: 'Saved Search' },
]);
});
});

describe('Serverless Breadcrumbs', () => {
const discoverServiceMock = {
...createDiscoverServicesMock(),
serverless: serverlessMock.createStart(),
};
beforeEach(() => {
(discoverServiceMock.serverless.setBreadcrumbs as jest.Mock).mockClear();
});

test('should not set any root', () => {
setBreadcrumbs({ services: discoverServiceMock });
expect(discoverServiceMock.serverless.setBreadcrumbs).toHaveBeenCalledWith([]);
});

test('should set title breadcrumb', () => {
setBreadcrumbs({ services: discoverServiceMock, titleBreadcrumbText: 'Saved Search' });
expect(discoverServiceMock.serverless.setBreadcrumbs).toHaveBeenCalledWith([
{ text: 'Saved Search' },
]);
});

test("shouldn't set root breadcrumbs, even when there is a custom root path", () => {
setBreadcrumbs({
services: discoverServiceMock,
titleBreadcrumbText: 'Saved Search',
rootBreadcrumbPath: '#/custom-path',
});
expect(discoverServiceMock.serverless.setBreadcrumbs).toHaveBeenCalledWith([
{ text: 'Saved Search' },
]);
});

test("shouldn't set root breadcrumbs, even when there is a custom profile", () => {
setBreadcrumbs({
services: {
...discoverServiceMock,
history: () => {
const history = createMemoryHistory<HistoryLocationState>({});
history.push('/p/my-profile');
return history;
},
},
titleBreadcrumbText: 'Saved Search',
});
expect(discoverServiceMock.serverless.setBreadcrumbs).toHaveBeenCalledWith([
{ text: 'Saved Search' },
]);
});
});
59 changes: 27 additions & 32 deletions src/plugins/discover/public/utils/breadcrumbs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const getRootPath = ({ history }: DiscoverServices) => {
return profile ? addProfile(rootPath, profile) : rootPath;
};

export function getRootBreadcrumbs({
function getRootBreadcrumbs({
breadcrumb,
services,
}: {
Expand All @@ -34,49 +34,44 @@ export function getRootBreadcrumbs({
];
}

export function getSavedSearchBreadcrumbs({
id,
services,
}: {
id: string;
services: DiscoverServices;
}) {
return [
...getRootBreadcrumbs({ services }),
{
text: id,
},
];
}

/**
* Helper function to set the Discover's breadcrumb
* if there's an active savedSearch, its title is appended
*/
export function setBreadcrumbsTitle({
title,
export function setBreadcrumbs({
rootBreadcrumbPath,
titleBreadcrumbText,
services,
}: {
title: string | undefined;
rootBreadcrumbPath?: string;
titleBreadcrumbText?: string;
services: DiscoverServices;
}) {
const rootBreadcrumbs = getRootBreadcrumbs({
breadcrumb: rootBreadcrumbPath,
services,
});
const discoverBreadcrumbsTitle = i18n.translate('discover.discoverBreadcrumbTitle', {
defaultMessage: 'Discover',
});

if (title) {
services.chrome.setBreadcrumbs([
{
text: discoverBreadcrumbsTitle,
href: getRootPath(services),
},
{ text: title },
]);
if (services.serverless) {
// in serverless only set breadcrumbs for saved search title
// the root breadcrumbs are set automatically by the serverless navigation
if (titleBreadcrumbText) {
services.serverless.setBreadcrumbs([{ text: titleBreadcrumbText }]);
} else {
services.serverless.setBreadcrumbs([]);
}
} else {
services.chrome.setBreadcrumbs([
{
text: discoverBreadcrumbsTitle,
},
]);
if (titleBreadcrumbText) {
services.chrome.setBreadcrumbs([...rootBreadcrumbs, { text: titleBreadcrumbText }]);
} else {
services.chrome.setBreadcrumbs([
{
text: discoverBreadcrumbsTitle,
},
]);
}
}
}
1 change: 1 addition & 0 deletions src/plugins/discover/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
"@kbn/discover-utils",
"@kbn/search-response-warnings",
"@kbn/content-management-plugin",
"@kbn/serverless",
"@kbn/react-kibana-mount",
"@kbn/react-kibana-context-render"
],
Expand Down

0 comments on commit 933f2a5

Please sign in to comment.