Skip to content

Commit 50e50b9

Browse files
nicklemmonprat98
andauthored
Introduces missing tooltips in header toolbar (#3109)
Co-authored-by: Pratyush Bhandari <prbhanda@redhat.com>
1 parent 10304e9 commit 50e50b9

17 files changed

Lines changed: 589 additions & 73 deletions

.eslintrc.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,12 @@
7070
"no-console": "off"
7171
}
7272
},
73+
{
74+
"files": ["**/*.test.tsx"],
75+
"rules": {
76+
"i18next/no-literal-string": "off"
77+
}
78+
},
7379
{
7480
"files": ["frontend/**/*.ts", "frontend/**/*.tsx", "platform/**/*.ts", "platform/**/*.tsx"],
7581
"excludedFiles": ["**/*.test.*", "**/*.cy.*", "**/*.fixture.*", "**/generated/**", "**/vite.config.*", "**/awx-utils.tsx", "**/eda-utils.tsx", "**/formatPath.tsx", "**/gateway-api-utils.tsx"],
Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
import { DropdownItem, TooltipPosition } from '@patternfly/react-core';
2+
import { render, screen } from '@testing-library/react';
3+
import userEvent from '@testing-library/user-event';
4+
import { beforeEach, describe, expect, test, vi } from 'vitest';
5+
import { PageMastheadDropdown } from './PageMastheadDropdown';
6+
7+
vi.mock('../components/useBreakPoint', () => ({
8+
useBreakpoint: vi.fn(() => true),
9+
}));
10+
11+
import { useBreakpoint } from '../components/useBreakPoint';
12+
13+
describe('PageMastheadDropdown', () => {
14+
beforeEach(() => {
15+
vi.clearAllMocks();
16+
vi.mocked(useBreakpoint).mockReturnValue(true);
17+
});
18+
19+
test('should render the toggle button', () => {
20+
render(
21+
<PageMastheadDropdown id="test-dropdown" icon={<span>icon</span>}>
22+
<DropdownItem key="item1">Item 1</DropdownItem>
23+
</PageMastheadDropdown>
24+
);
25+
26+
expect(screen.getByRole('button')).toBeDefined();
27+
});
28+
29+
test('should open the dropdown when the toggle is clicked', async () => {
30+
const user = userEvent.setup();
31+
render(
32+
<PageMastheadDropdown id="test-dropdown" icon={<span>icon</span>}>
33+
<DropdownItem key="item1">Item 1</DropdownItem>
34+
</PageMastheadDropdown>
35+
);
36+
37+
await user.click(screen.getByRole('button'));
38+
expect(screen.getByText('Item 1')).toBeInTheDocument();
39+
});
40+
41+
test('should close the dropdown when an item is selected', async () => {
42+
const user = userEvent.setup();
43+
render(
44+
<PageMastheadDropdown id="test-dropdown" icon={<span>icon</span>}>
45+
<DropdownItem key="item1">Item 1</DropdownItem>
46+
</PageMastheadDropdown>
47+
);
48+
49+
const toggle = screen.getByRole('button');
50+
await user.click(toggle);
51+
expect(toggle).toHaveAttribute('aria-expanded', 'true');
52+
53+
await user.click(screen.getByText('Item 1'));
54+
expect(toggle).toHaveAttribute('aria-expanded', 'false');
55+
});
56+
57+
test('should display the label on larger screens', () => {
58+
vi.mocked(useBreakpoint).mockReturnValue(true);
59+
render(
60+
<PageMastheadDropdown id="test-dropdown" icon={<span>icon</span>} label="Help">
61+
<DropdownItem key="item1">Item 1</DropdownItem>
62+
</PageMastheadDropdown>
63+
);
64+
expect(screen.getByText('Help')).toBeInTheDocument();
65+
});
66+
67+
test('should hide the label on smaller screens', () => {
68+
vi.mocked(useBreakpoint).mockReturnValue(false);
69+
render(
70+
<PageMastheadDropdown id="test-dropdown" icon={<span>icon</span>} label="Help">
71+
<DropdownItem key="item1">Item 1</DropdownItem>
72+
</PageMastheadDropdown>
73+
);
74+
expect(screen.queryByText('Help')).toBeNull();
75+
});
76+
77+
test('should not render a tooltip element when tooltip prop is not provided', async () => {
78+
const user = userEvent.setup();
79+
render(
80+
<PageMastheadDropdown id="test-dropdown" icon={<span>icon</span>}>
81+
<DropdownItem key="item1">Item 1</DropdownItem>
82+
</PageMastheadDropdown>
83+
);
84+
85+
await user.hover(screen.getByRole('button'));
86+
expect(screen.queryByRole('tooltip')).toBeNull();
87+
});
88+
89+
test('should display a tooltip on hover when the tooltip prop is provided', async () => {
90+
const user = userEvent.setup();
91+
render(
92+
<PageMastheadDropdown
93+
id="test-dropdown"
94+
icon={<span>icon</span>}
95+
tooltipContent="Open help menu"
96+
>
97+
<DropdownItem key="item1">Item 1</DropdownItem>
98+
</PageMastheadDropdown>
99+
);
100+
101+
await user.hover(screen.getByRole('button'));
102+
expect(screen.getByRole('tooltip')).toBeInTheDocument();
103+
expect(screen.getByRole('tooltip')).toHaveTextContent('Open help menu');
104+
});
105+
106+
test('should apply tooltipPosition to the tooltip', async () => {
107+
const user = userEvent.setup();
108+
render(
109+
<PageMastheadDropdown
110+
id="test-dropdown"
111+
icon={<span>icon</span>}
112+
tooltipContent="Open help menu"
113+
tooltipPosition={TooltipPosition.bottom}
114+
>
115+
<DropdownItem key="item1">Item 1</DropdownItem>
116+
</PageMastheadDropdown>
117+
);
118+
119+
await user.hover(screen.getByRole('button'));
120+
const tooltip = screen.getByRole('tooltip');
121+
expect(tooltip).toBeInTheDocument();
122+
expect(tooltip).toHaveTextContent('Open help menu');
123+
});
124+
});

framework/PageMasthead/PageMastheadDropdown.tsx

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import {
66
Icon,
77
MenuToggle,
88
MenuToggleElement,
9+
Tooltip,
10+
type TooltipProps,
911
} from '@patternfly/react-core';
1012
import { ReactNode, Ref, useCallback, useState } from 'react';
1113
import { useBreakpoint } from '../components/useBreakPoint';
@@ -14,6 +16,10 @@ export function PageMastheadDropdown(props: {
1416
id: string;
1517
icon: ReactNode;
1618
label?: string;
19+
/** Optional tooltip content used to render a tooltip around the dropdown */
20+
tooltipContent?: TooltipProps['content'];
21+
/** Optional tooltip position. Has no effect if no `tooltipContent` prop is supplied */
22+
tooltipPosition?: TooltipProps['position'];
1723
children: ReactNode;
1824
}) {
1925
const [isOpen, setIsOpen] = useState(false);
@@ -34,12 +40,13 @@ export function PageMastheadDropdown(props: {
3440
id={`${props.id}-menu-toggle`}
3541
isOpen={isOpen}
3642
label={props.label}
43+
tooltipContent={props.tooltipContent}
44+
tooltipPosition={props.tooltipPosition}
3745
onToggle={onToggle}
3846
toggleRef={toggleRef}
3947
/>
4048
)}
4149
isOpen={isOpen}
42-
isPlain
4350
popperProps={{
4451
appendTo: () => document.body,
4552
preventOverflow: true,
@@ -54,19 +61,42 @@ export function PageMastheadDropdown(props: {
5461
);
5562
}
5663

57-
interface ToggleProps {
64+
function Toggle({
65+
icon,
66+
id,
67+
isOpen,
68+
label,
69+
tooltipContent,
70+
tooltipPosition,
71+
onToggle,
72+
toggleRef,
73+
}: {
5874
icon: ReactNode;
5975
id: string;
6076
isOpen: boolean;
6177
label?: string;
78+
/** Optional tooltip content used to render a tooltip around the dropdown */
79+
tooltipContent?: TooltipProps['content'];
80+
/** Optional tooltip position. Has no effect if no `tooltipContent` prop is supplied */
81+
tooltipPosition?: TooltipProps['position'];
6282
onToggle: () => void;
6383
toggleRef: Ref<MenuToggleElement>;
64-
}
65-
66-
function Toggle({ icon, id, isOpen, label, onToggle, toggleRef }: ToggleProps) {
84+
}) {
6785
const showLabel = useBreakpoint('md');
68-
return (
69-
<MenuToggle id={id} isExpanded={isOpen} onClick={onToggle} ref={toggleRef} variant="plain">
86+
const [isHovered, setIsHovered] = useState(false);
87+
88+
const toggle = (
89+
<MenuToggle
90+
id={id}
91+
isExpanded={isOpen}
92+
onClick={onToggle}
93+
ref={toggleRef}
94+
variant="plain"
95+
onMouseEnter={() => setIsHovered(true)}
96+
onMouseLeave={() => setIsHovered(false)}
97+
onFocus={() => setIsHovered(true)}
98+
onBlur={() => setIsHovered(false)}
99+
>
70100
<Flex
71101
alignItems={{ default: 'alignItemsCenter' }}
72102
flexWrap={{ default: 'nowrap' }}
@@ -77,4 +107,17 @@ function Toggle({ icon, id, isOpen, label, onToggle, toggleRef }: ToggleProps) {
77107
</Flex>
78108
</MenuToggle>
79109
);
110+
111+
if (!tooltipContent) return toggle;
112+
113+
return (
114+
<Tooltip
115+
content={tooltipContent}
116+
position={tooltipPosition}
117+
trigger="manual"
118+
isVisible={isHovered && !isOpen}
119+
>
120+
{toggle}
121+
</Tooltip>
122+
);
80123
}
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
import { render, screen } from '@testing-library/react';
2+
import userEvent from '@testing-library/user-event';
3+
import { beforeEach, describe, expect, test, vi } from 'vitest';
4+
5+
vi.mock('../PageNotifications/usePageNotifications');
6+
vi.mock('../PageNotifications/usePageNotificationsRead');
7+
8+
import { usePageNotifications } from '../PageNotifications/usePageNotifications';
9+
import { usePageNotificationsRead } from '../PageNotifications/usePageNotificationsRead';
10+
import { PageNotificationsIcon } from './PageNotificationsIcon';
11+
12+
describe('PageNotificationsIcon', () => {
13+
const mockSetNotificationsDrawerOpen = vi.fn();
14+
15+
beforeEach(() => {
16+
vi.clearAllMocks();
17+
vi.mocked(usePageNotifications).mockReturnValue({
18+
setNotificationsDrawerOpen: mockSetNotificationsDrawerOpen,
19+
notificationGroups: {},
20+
notificationsDrawerOpen: false,
21+
setNotificationGroups: vi.fn(),
22+
});
23+
vi.mocked(usePageNotificationsRead).mockReturnValue({
24+
isNotificationRead: vi.fn().mockReturnValue(false),
25+
markAllNotificationsRead: vi.fn(),
26+
markAllNotificationsUnread: vi.fn(),
27+
setNotificationRead: vi.fn(),
28+
});
29+
});
30+
31+
test('should render the notification badge with read variant when there are no notifications', () => {
32+
render(<PageNotificationsIcon />);
33+
const badge = screen.getByTestId('notification-badge');
34+
expect(badge).toBeInTheDocument();
35+
expect(badge).not.toHaveClass('pf-m-unread');
36+
});
37+
38+
test('should show unread variant when there are unread notifications', () => {
39+
vi.mocked(usePageNotifications).mockReturnValue({
40+
setNotificationsDrawerOpen: mockSetNotificationsDrawerOpen,
41+
notificationGroups: {
42+
group1: {
43+
title: 'Group 1',
44+
notifications: [
45+
{ id: 'notif-1', title: 'Notification 1', variant: 'info' },
46+
{ id: 'notif-2', title: 'Notification 2', variant: 'warning' },
47+
],
48+
},
49+
},
50+
notificationsDrawerOpen: false,
51+
setNotificationGroups: vi.fn(),
52+
});
53+
54+
render(<PageNotificationsIcon />);
55+
expect(screen.getByTestId('notification-badge')).toHaveClass('pf-m-unread');
56+
});
57+
58+
test('should not count already-read notifications as unread', () => {
59+
vi.mocked(usePageNotifications).mockReturnValue({
60+
setNotificationsDrawerOpen: mockSetNotificationsDrawerOpen,
61+
notificationGroups: {
62+
group1: {
63+
title: 'Group 1',
64+
notifications: [{ id: 'notif-1', title: 'Notification 1', variant: 'info' }],
65+
},
66+
},
67+
notificationsDrawerOpen: false,
68+
setNotificationGroups: vi.fn(),
69+
});
70+
vi.mocked(usePageNotificationsRead).mockReturnValue({
71+
isNotificationRead: vi.fn().mockReturnValue(true),
72+
markAllNotificationsRead: vi.fn(),
73+
markAllNotificationsUnread: vi.fn(),
74+
setNotificationRead: vi.fn(),
75+
});
76+
77+
render(<PageNotificationsIcon />);
78+
expect(screen.getByTestId('notification-badge')).not.toHaveClass('pf-m-unread');
79+
});
80+
81+
test('should skip notifications with non-string ids from unread count', () => {
82+
vi.mocked(usePageNotifications).mockReturnValue({
83+
setNotificationsDrawerOpen: mockSetNotificationsDrawerOpen,
84+
notificationGroups: {
85+
group1: {
86+
title: 'Group 1',
87+
notifications: [
88+
{ id: 42 as unknown as string, title: 'Notification 1', variant: 'info' },
89+
],
90+
},
91+
},
92+
notificationsDrawerOpen: false,
93+
setNotificationGroups: vi.fn(),
94+
});
95+
96+
render(<PageNotificationsIcon />);
97+
expect(screen.getByTestId('notification-badge')).not.toHaveClass('pf-m-unread');
98+
});
99+
100+
test('should toggle the notifications drawer when badge is clicked', async () => {
101+
const user = userEvent.setup();
102+
render(<PageNotificationsIcon />);
103+
104+
await user.click(screen.getByTestId('notification-badge'));
105+
106+
expect(mockSetNotificationsDrawerOpen).toHaveBeenCalled();
107+
const toggleFn = mockSetNotificationsDrawerOpen.mock.calls[0][0] as (open: boolean) => boolean;
108+
expect(toggleFn(false)).toBe(true);
109+
expect(toggleFn(true)).toBe(false);
110+
});
111+
});
Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
1-
import { NotificationBadge } from '@patternfly/react-core';
1+
import { NotificationBadge, Tooltip } from '@patternfly/react-core';
22
import { IPageNotification } from '../PageNotifications/PageNotification';
33
import { usePageNotifications } from '../PageNotifications/usePageNotifications';
44
import { usePageNotificationsRead } from '../PageNotifications/usePageNotificationsRead';
5+
import { useTranslation } from 'react-i18next';
56

67
export function PageNotificationsIcon() {
8+
const { t } = useTranslation();
79
const { setNotificationsDrawerOpen, notificationGroups } = usePageNotifications();
810
const { isNotificationRead } = usePageNotificationsRead();
911

@@ -17,12 +19,14 @@ export function PageNotificationsIcon() {
1719
}, 0);
1820

1921
return (
20-
<NotificationBadge
21-
data-cy="notification-badge"
22-
data-testid="notification-badge"
23-
variant={unreadCount === 0 ? 'read' : 'unread'}
24-
count={unreadCount}
25-
onClick={() => setNotificationsDrawerOpen((open) => !open)}
26-
/>
22+
<Tooltip content={t`Notifications`} position="bottom">
23+
<NotificationBadge
24+
data-cy="notification-badge"
25+
data-testid="notification-badge"
26+
variant={unreadCount === 0 ? 'read' : 'unread'}
27+
count={unreadCount}
28+
onClick={() => setNotificationsDrawerOpen((open) => !open)}
29+
/>
30+
</Tooltip>
2731
);
2832
}

0 commit comments

Comments
 (0)