Skip to content

Commit

Permalink
Force last breadcrumb to be inactive (#166785)
Browse files Browse the repository at this point in the history
**Related to:** #161540, #161539

## Summary

Always force the last breadcrumb to be inactive.

## Details

Usual UX expects the last breadcrumb to be inactive as it represents the current page. The same can be seen from EUI [examples](https://eui.elastic.co/#/navigation/breadcrumbs). It turns out Serverless Security Solution plugin does't remove `href` and `onClick` fields from the last breadcrumb and passes it to `chrome.setBreadcrumbs()` or `serverless.setBreadcrumbs()` which renders the last breadcrumb as active but clicking on it only refreshes the page. ESS Security Solution on the other hand processes breadcrumbs currently. The same behavior may be the case for the other plungs as well.

As it's much simpler to strip off undesired fields at one place instead of processing them in plugins it's done in `packages/core/chrome/core-chrome-browser-internal/src/ui/header/header_breadcrumbs.tsx`. Security Solution codebase has been updated accordingly.

A side effect of this PR is consistent ESS and Serverless breadcrumbs behavior and it will help to reuse ESS tests for Serverless.
  • Loading branch information
maximpn authored Sep 21, 2023
1 parent 22a9f4a commit 522f577
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 193 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,40 @@
* Side Public License, v 1.
*/

import { mount } from 'enzyme';
import '@testing-library/jest-dom';
import { act, render, screen } from '@testing-library/react';
import React from 'react';
import { act } from 'react-dom/test-utils';
import { BehaviorSubject } from 'rxjs';
import { BehaviorSubject, of } from 'rxjs';
import { HeaderBreadcrumbs } from './header_breadcrumbs';

describe('HeaderBreadcrumbs', () => {
it('renders updates to the breadcrumbs$ observable', () => {
it('renders updates to the breadcrumbs$ observable', async () => {
const breadcrumbs$ = new BehaviorSubject([{ text: 'First' }]);
const wrapper = mount(<HeaderBreadcrumbs breadcrumbs$={breadcrumbs$} />);
wrapper.find('.euiBreadcrumb').forEach((el) => expect(el.render()).toMatchSnapshot());

render(<HeaderBreadcrumbs breadcrumbs$={breadcrumbs$} />);

expect(await screen.findByLabelText('Breadcrumbs')).toHaveTextContent('First');

act(() => breadcrumbs$.next([{ text: 'First' }, { text: 'Second' }]));
wrapper.update();
wrapper.find('.euiBreadcrumb').forEach((el) => expect(el.render()).toMatchSnapshot());

expect(await screen.findByLabelText('Breadcrumbs')).toHaveTextContent('FirstSecond');

act(() => breadcrumbs$.next([]));
wrapper.update();
wrapper.find('.euiBreadcrumb').forEach((el) => expect(el.render()).toMatchSnapshot());

expect(await screen.findByLabelText('Breadcrumbs')).toHaveTextContent('Kibana');
});

it('forces the last breadcrumb inactivity', async () => {
const breadcrumbs$ = of([
{ text: 'First' },
{ text: 'Last', href: '/something', onClick: jest.fn() },
]);

render(<HeaderBreadcrumbs breadcrumbs$={breadcrumbs$} />);

const lastBreadcrumb = await screen.findByTitle('Last');

expect(lastBreadcrumb).not.toHaveAttribute('href');
expect(lastBreadcrumb.tagName).not.toBe('a');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,21 @@ export function HeaderBreadcrumbs({ breadcrumbs$ }: Props) {
crumbs = [{ text: 'Kibana' }];
}

crumbs = crumbs.map((breadcrumb, i) => ({
...breadcrumb,
'data-test-subj': classNames(
'breadcrumb',
breadcrumb['data-test-subj'],
i === 0 && 'first',
i === breadcrumbs.length - 1 && 'last'
),
}));
crumbs = crumbs.map((breadcrumb, i) => {
const isLast = i === breadcrumbs.length - 1;

return {
...breadcrumb,
href: isLast ? undefined : breadcrumb.href,
onClick: isLast ? undefined : breadcrumb.onClick,
'data-test-subj': classNames(
'breadcrumb',
breadcrumb['data-test-subj'],
i === 0 && 'first',
isLast && 'last'
),
};
});

return <EuiHeaderBreadcrumbs breadcrumbs={crumbs} max={10} data-test-subj="breadcrumbs" />;
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,11 @@
* 2.0.
*/

import type { ChromeBreadcrumb } from '@kbn/core/public';
import type { Services } from '../common/services';

export const subscribeBreadcrumbs = (services: Services) => {
const { chrome, securitySolution } = services;
const { securitySolution, chrome } = services;
securitySolution.getBreadcrumbsNav$().subscribe((breadcrumbsNav) => {
const breadcrumbs = [...breadcrumbsNav.leading, ...breadcrumbsNav.trailing];
if (breadcrumbs.length > 0) {
chrome.setBreadcrumbs(emptyLastBreadcrumbUrl(breadcrumbs));
}
chrome.setBreadcrumbs([...breadcrumbsNav.leading, ...breadcrumbsNav.trailing]);
});
};

export const emptyLastBreadcrumbUrl = (breadcrumbs: ChromeBreadcrumb[]) => {
const lastBreadcrumb = breadcrumbs[breadcrumbs.length - 1];
if (lastBreadcrumb) {
return [...breadcrumbs.slice(0, -1), { ...lastBreadcrumb, href: '', onClick: undefined }];
}
return breadcrumbs;
};

0 comments on commit 522f577

Please sign in to comment.