Skip to content

Commit

Permalink
[Enterprise Search] Change last breadcrumb to inactive/non-linked bre…
Browse files Browse the repository at this point in the history
…adcrumb (#96489)

* Update our EUI breadcrumb helper to skip generating links for the last breadcrumb in the list

* Fix useEuiBreadcrumbs tests

- add a describe block to make it clear we're testing link behavior in non-last breadcrumbs
- add a helper that automatically adds a last breadcrumb so that link generation still works

* Add comment/note as to why I didn't add last-breadcrumb-specific logic to useGenerateBreadcrumbs
  • Loading branch information
Constance authored Apr 7, 2021
1 parent d8ef85e commit e6e3b16
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ jest.mock('../react_router_helpers', () => ({
import { letBrowserHandleEvent } from '../react_router_helpers';

import {
Breadcrumb,
useGenerateBreadcrumbs,
useEuiBreadcrumbs,
useEnterpriseSearchBreadcrumbs,
Expand All @@ -40,6 +41,9 @@ describe('useGenerateBreadcrumbs', () => {
{ text: 'Groups', path: '/groups' },
{ text: 'Example Group Name', path: '/groups/{id}' },
{ text: 'Source Prioritization', path: '/groups/{id}/source_prioritization' },
// Note: We're still generating a path for the last breadcrumb even though useEuiBreadcrumbs
// will not render a link for it. This is because it's easier to keep our last-breadcrumb-specific
// logic in one place, & this way we still have a current path if (for some reason) we need it later.
]);
});

Expand Down Expand Up @@ -89,48 +93,51 @@ describe('useEuiBreadcrumbs', () => {
},
{
text: 'World',
href: '/app/enterprise_search/world',
onClick: expect.any(Function),
// Per EUI best practices, the last breadcrumb is inactive/is not a link
},
]);
});

it('prevents default navigation and uses React Router history on click', () => {
const breadcrumb = useEuiBreadcrumbs([{ text: '', path: '/test' }])[0] as any;
describe('link behavior for non-last breadcrumbs', () => {
// Test helper - adds a 2nd dummy breadcrumb so that paths from the first breadcrumb are generated
const useEuiBreadcrumb = (breadcrumb: Breadcrumb) =>
useEuiBreadcrumbs([breadcrumb, { text: '' }])[0] as any;

expect(breadcrumb.href).toEqual('/app/enterprise_search/test');
expect(mockHistory.createHref).toHaveBeenCalled();
it('prevents default navigation and uses React Router history on click', () => {
const breadcrumb = useEuiBreadcrumb({ text: '', path: '/test' });

const event = { preventDefault: jest.fn() };
breadcrumb.onClick(event);
expect(breadcrumb.href).toEqual('/app/enterprise_search/test');
expect(mockHistory.createHref).toHaveBeenCalled();

expect(event.preventDefault).toHaveBeenCalled();
expect(mockKibanaValues.navigateToUrl).toHaveBeenCalled();
});
const event = { preventDefault: jest.fn() };
breadcrumb.onClick(event);

it('does not call createHref if shouldNotCreateHref is passed', () => {
const breadcrumb = useEuiBreadcrumbs([
{ text: '', path: '/test', shouldNotCreateHref: true },
])[0] as any;
expect(event.preventDefault).toHaveBeenCalled();
expect(mockKibanaValues.navigateToUrl).toHaveBeenCalled();
});

expect(breadcrumb.href).toEqual('/test');
expect(mockHistory.createHref).not.toHaveBeenCalled();
});
it('does not call createHref if shouldNotCreateHref is passed', () => {
const breadcrumb = useEuiBreadcrumb({ text: '', path: '/test', shouldNotCreateHref: true });

it('does not prevent default browser behavior on new tab/window clicks', () => {
const breadcrumb = useEuiBreadcrumbs([{ text: '', path: '/' }])[0] as any;
expect(breadcrumb.href).toEqual('/test');
expect(mockHistory.createHref).not.toHaveBeenCalled();
});

(letBrowserHandleEvent as jest.Mock).mockImplementationOnce(() => true);
breadcrumb.onClick();
it('does not prevent default browser behavior on new tab/window clicks', () => {
const breadcrumb = useEuiBreadcrumb({ text: '', path: '/' });

expect(mockKibanaValues.navigateToUrl).not.toHaveBeenCalled();
});
(letBrowserHandleEvent as jest.Mock).mockImplementationOnce(() => true);
breadcrumb.onClick();

expect(mockKibanaValues.navigateToUrl).not.toHaveBeenCalled();
});

it('does not generate link behavior if path is excluded', () => {
const breadcrumb = useEuiBreadcrumbs([{ text: 'Unclickable breadcrumb' }])[0];
it('does not generate link behavior if path is excluded', () => {
const breadcrumb = useEuiBreadcrumb({ text: 'Unclickable breadcrumb' });

expect(breadcrumb.href).toBeUndefined();
expect(breadcrumb.onClick).toBeUndefined();
expect(breadcrumb.href).toBeUndefined();
expect(breadcrumb.onClick).toBeUndefined();
});
});
});

Expand Down Expand Up @@ -164,8 +171,6 @@ describe('useEnterpriseSearchBreadcrumbs', () => {
},
{
text: 'Page 2',
href: '/app/enterprise_search/page2',
onClick: expect.any(Function),
},
]);
});
Expand All @@ -174,8 +179,6 @@ describe('useEnterpriseSearchBreadcrumbs', () => {
expect(useEnterpriseSearchBreadcrumbs()).toEqual([
{
text: 'Enterprise Search',
href: '/app/enterprise_search/overview',
onClick: expect.any(Function),
},
]);
});
Expand Down Expand Up @@ -219,8 +222,6 @@ describe('useAppSearchBreadcrumbs', () => {
},
{
text: 'Page 2',
href: '/app/enterprise_search/app_search/page2',
onClick: expect.any(Function),
},
]);
});
Expand All @@ -234,8 +235,6 @@ describe('useAppSearchBreadcrumbs', () => {
},
{
text: 'App Search',
href: '/app/enterprise_search/app_search/',
onClick: expect.any(Function),
},
]);
});
Expand Down Expand Up @@ -279,8 +278,6 @@ describe('useWorkplaceSearchBreadcrumbs', () => {
},
{
text: 'Page 2',
href: '/app/enterprise_search/workplace_search/page2',
onClick: expect.any(Function),
},
]);
});
Expand All @@ -294,8 +291,6 @@ describe('useWorkplaceSearchBreadcrumbs', () => {
},
{
text: 'Workplace Search',
href: '/app/enterprise_search/workplace_search/',
onClick: expect.any(Function),
},
]);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { letBrowserHandleEvent, createHref } from '../react_router_helpers';
* Types
*/

interface Breadcrumb {
export interface Breadcrumb {
text: string;
path?: string;
// Used to navigate outside of the React Router basename,
Expand Down Expand Up @@ -64,16 +64,20 @@ export const useGenerateBreadcrumbs = (trail: BreadcrumbTrail): Breadcrumbs => {
/**
* Convert IBreadcrumb objects to React-Router-friendly EUI breadcrumb objects
* https://elastic.github.io/eui/#/navigation/breadcrumbs
*
* NOTE: Per EUI best practices, we remove the link behavior and
* generate an inactive breadcrumb for the last breadcrumb in the list.
*/

export const useEuiBreadcrumbs = (breadcrumbs: Breadcrumbs): EuiBreadcrumb[] => {
const { navigateToUrl, history } = useValues(KibanaLogic);
const { http } = useValues(HttpLogic);

return breadcrumbs.map(({ text, path, shouldNotCreateHref }) => {
return breadcrumbs.map(({ text, path, shouldNotCreateHref }, i) => {
const breadcrumb: EuiBreadcrumb = { text };
const isLastBreadcrumb = i === breadcrumbs.length - 1;

if (path) {
if (path && !isLastBreadcrumb) {
breadcrumb.href = createHref(path, { history, http }, { shouldNotCreateHref });
breadcrumb.onClick = (event) => {
if (letBrowserHandleEvent(event)) return;
Expand Down

0 comments on commit e6e3b16

Please sign in to comment.