Skip to content

Commit 0cf6585

Browse files
committed
back improvements
1 parent bf10fa1 commit 0cf6585

3 files changed

Lines changed: 68 additions & 12 deletions

File tree

src/core/packages/chrome/app-header/src/app_header/app_header.test.tsx

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
import React from 'react';
1111
import '@testing-library/jest-dom';
1212
import { BehaviorSubject } from 'rxjs';
13-
import { fireEvent, render, screen } from '@testing-library/react';
13+
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
1414
import { EuiButtonIcon } from '@elastic/eui';
1515
import type { InternalChromeStart } from '@kbn/core-chrome-browser-internal-types';
1616
import { ChromeServiceProvider } from '@kbn/core-chrome-browser-context';
@@ -99,4 +99,33 @@ describe('AppHeaderView', () => {
9999
expect(screen.getByTestId('alertsTab')).toBeInTheDocument();
100100
expect(screen.getByText('3')).toBeInTheDocument();
101101
});
102+
103+
it('only treats exact base path prefixes as already prepended for back links', () => {
104+
const chrome = chromeServiceMock.createStartContract();
105+
chrome.componentDeps.basePath.get.mockReturnValue('/base');
106+
chrome.componentDeps.basePath.prepend.mockImplementation((path: string) => `/base${path}`);
107+
108+
renderAppHeader(<AppHeaderView back="/base-other/app" />, chrome);
109+
110+
expect(screen.getByTestId('appHeaderBack')).toHaveAttribute('href', '/base/base-other/app');
111+
});
112+
113+
it('renders multiple back targets as a menu and closes it after selection', async () => {
114+
const backClick = jest.fn((event: React.MouseEvent) => event.preventDefault());
115+
116+
renderAppHeader(
117+
<AppHeaderView
118+
back={[
119+
{ href: '/app/first', label: 'First app' },
120+
{ href: '/app/second', label: 'Second app', onClick: backClick },
121+
]}
122+
/>
123+
);
124+
125+
fireEvent.click(screen.getByRole('button', { name: 'Open back navigation menu' }));
126+
fireEvent.click(screen.getByText('Second app'));
127+
128+
expect(backClick).toHaveBeenCalledTimes(1);
129+
await waitFor(() => expect(screen.queryByText('Second app')).not.toBeInTheDocument());
130+
});
102131
});

src/core/packages/chrome/app-header/src/app_header/back_button.tsx

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ const getBackToLabel = (destination: string) =>
3030
values: { destination },
3131
});
3232

33+
const backMenuLabel = i18n.translate('core.ui.chrome.appHeader.backButtonMenuAriaLabel', {
34+
defaultMessage: 'Open back navigation menu',
35+
});
36+
3337
const useBackButtonStyles = () => {
3438
const { euiTheme } = useEuiTheme();
3539

@@ -57,6 +61,7 @@ export const BackButton = React.memo<BackButtonProps>(({ targets }) => {
5761
const tooltip = primary?.backDestinationLabel
5862
? getBackToLabel(primary.backDestinationLabel)
5963
: backLabel;
64+
const buttonLabel = targets.length > 1 ? backMenuLabel : tooltip;
6065

6166
if (!primary) {
6267
return null;
@@ -69,10 +74,14 @@ export const BackButton = React.memo<BackButtonProps>(({ targets }) => {
6974
display="empty"
7075
size="xs"
7176
css={styles.button}
72-
aria-label={tooltip}
77+
aria-label={buttonLabel}
7378
data-test-subj="appHeaderBack"
7479
{...(targets.length > 1
75-
? { onClick: togglePopover }
80+
? {
81+
onClick: togglePopover,
82+
'aria-haspopup': 'menu' as const,
83+
'aria-expanded': isPopoverOpen,
84+
}
7685
: { href: primary.backHref, onClick: primary.backOnClick })}
7786
/>
7887
);
@@ -89,7 +98,14 @@ export const BackButton = React.memo<BackButtonProps>(({ targets }) => {
8998
>
9099
<EuiContextMenuPanel
91100
items={targets.map((target, idx) => (
92-
<EuiContextMenuItem key={idx} href={target.backHref} onClick={target.backOnClick}>
101+
<EuiContextMenuItem
102+
key={idx}
103+
href={target.backHref}
104+
onClick={(event) => {
105+
closePopover();
106+
target.backOnClick?.(event);
107+
}}
108+
>
93109
{target.backDestinationLabel ?? target.backHref}
94110
</EuiContextMenuItem>
95111
))}

src/core/packages/chrome/app-header/src/app_header/hooks/use_back_navigation.ts

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ export interface BackNavigation {
2020

2121
const EMPTY: BackNavigation[] = [];
2222

23+
const hasBasePathPrefix = (href: string, basePath: string): boolean => {
24+
return href === basePath || href.startsWith(`${basePath}/`);
25+
};
26+
2327
export function useBackNavTargets(
2428
back: AppHeaderBack | AppHeaderBack[] | undefined
2529
): BackNavigation[] {
@@ -32,17 +36,24 @@ export function useBackNavTargets(
3236
const backItems = Array.isArray(back) ? back : [back];
3337
const base = basePath.get();
3438
const explicit: BackNavigation[] = [];
39+
const seenHrefs = new Set<string>();
3540
for (const b of backItems) {
3641
const target = typeof b === 'string' ? { href: b } : b;
37-
if (target.href?.trim()) {
38-
const href =
39-
base && target.href.startsWith(base) ? target.href : basePath.prepend(target.href);
40-
explicit.push({
41-
backHref: href,
42-
backOnClick: target.onClick,
43-
backDestinationLabel: target.label,
44-
});
42+
const targetHref = target.href?.trim();
43+
if (!targetHref) {
44+
continue;
45+
}
46+
const href =
47+
base && hasBasePathPrefix(targetHref, base) ? targetHref : basePath.prepend(targetHref);
48+
if (seenHrefs.has(href)) {
49+
continue;
4550
}
51+
seenHrefs.add(href);
52+
explicit.push({
53+
backHref: href,
54+
backOnClick: target.onClick,
55+
backDestinationLabel: target.label,
56+
});
4657
}
4758
return explicit.length > 0 ? explicit : EMPTY;
4859
}, [back, basePath]);

0 commit comments

Comments
 (0)