Skip to content

Commit 5fe623b

Browse files
ref(ui): Remove DropdownAutoCompleteMenu from settings breadcrumbs (#98078)
The dropdowns are replaced with the CompactSelect component. There are a couple hacks in here necessary to make hover activation a good experience. In the future we'll probably want to refactor this along with our compact select component to natively support some type of accessible hover activation. See it in action here https://sentry-nqi0z0e0z.sentry.dev/settings/organization/
1 parent f23d2ca commit 5fe623b

File tree

6 files changed

+199
-118
lines changed

6 files changed

+199
-118
lines changed
Lines changed: 61 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,63 +1,107 @@
1-
import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
1+
import {Fragment} from 'react';
2+
3+
import {act, render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
24

35
import BreadcrumbDropdown from './breadcrumbDropdown';
46

57
describe('Settings Breadcrumb Dropdown', () => {
68
const selectMock = jest.fn();
7-
const items = [
8-
{index: 0, value: '1', label: 'foo'},
9-
{index: 1, value: '2', label: 'bar'},
10-
];
119

1210
const createWrapper = () => {
1311
return render(
1412
<BreadcrumbDropdown
1513
route={{path: '/', name: 'root'}}
16-
items={items}
17-
name="Test"
14+
options={[
15+
{value: '1', label: 'foo'},
16+
{value: '2', label: 'bar'},
17+
]}
18+
name="The Crumb"
1819
hasMenu
19-
onSelect={selectMock}
20+
onCrumbSelect={selectMock}
2021
/>
2122
);
2223
};
2324

2425
it('opens when hovered over crumb', async () => {
2526
createWrapper();
26-
expect(screen.getByText('Test')).toBeInTheDocument();
27-
await userEvent.hover(screen.getByText('Test'));
27+
expect(screen.getByText('The Crumb')).toBeInTheDocument();
28+
await userEvent.hover(screen.getByText('The Crumb'));
2829
expect(screen.getByText('foo')).toBeInTheDocument();
2930
expect(screen.getByText('bar')).toBeInTheDocument();
3031
});
3132

3233
it('closes immediately after selecting an item', async () => {
3334
createWrapper();
34-
await userEvent.hover(screen.getByText('Test'));
35+
await userEvent.hover(screen.getByText('The Crumb'));
3536
expect(screen.getByText('foo')).toBeInTheDocument();
3637

3738
await userEvent.click(screen.getByText('foo'));
38-
expect(selectMock).toHaveBeenCalled();
39+
expect(selectMock).toHaveBeenCalledWith('1');
3940

4041
expect(screen.queryByText('foo')).not.toBeInTheDocument();
4142
});
4243

4344
it('stays open when hovered over crumb and then into dropdown menu', async () => {
4445
createWrapper();
45-
await userEvent.hover(screen.getByText('Test'));
46+
await userEvent.hover(screen.getByText('The Crumb'));
4647
expect(screen.getByText('foo')).toBeInTheDocument();
4748

4849
await userEvent.hover(screen.getByText('foo'));
4950
expect(screen.getByText('foo')).toBeInTheDocument();
5051
});
5152

52-
it('closes after entering dropdown and then leaving dropdown', async () => {
53+
it('closes after entering dropdown and then leaving after timeout', async () => {
54+
jest.useFakeTimers();
5355
createWrapper();
54-
await userEvent.hover(screen.getByText('Test'));
56+
57+
await userEvent.hover(screen.getByText('The Crumb'), {delay: null});
5558
expect(screen.getByText('foo')).toBeInTheDocument();
5659

57-
await userEvent.hover(screen.getByText('foo'));
60+
await userEvent.hover(screen.getByText('foo'), {delay: null});
61+
expect(screen.getByText('foo')).toBeInTheDocument();
62+
63+
await userEvent.unhover(screen.getByText('foo'), {delay: null});
64+
65+
// The menu will not disappear until after a timeout
66+
expect(screen.getByText('foo')).toBeInTheDocument();
67+
68+
// Menu disappears after timeout
69+
await act(() => jest.runAllTimersAsync());
70+
expect(screen.queryByText('foo')).not.toBeInTheDocument();
71+
});
72+
73+
it('closes other breadcrumbs upon hover immediately', async () => {
74+
render(
75+
<Fragment>
76+
<BreadcrumbDropdown
77+
route={{path: '/', name: 'root'}}
78+
options={[
79+
{value: '1', label: 'foo'},
80+
{value: '2', label: 'bar'},
81+
]}
82+
name="Crumb One"
83+
hasMenu
84+
onCrumbSelect={selectMock}
85+
/>
86+
<BreadcrumbDropdown
87+
route={{path: '/', name: 'root'}}
88+
options={[
89+
{value: '1', label: 'baz'},
90+
{value: '2', label: 'buzz'},
91+
]}
92+
name="Crumb Two"
93+
hasMenu
94+
onCrumbSelect={selectMock}
95+
/>
96+
</Fragment>
97+
);
98+
99+
await userEvent.hover(screen.getByText('Crumb One'), {delay: null});
58100
expect(screen.getByText('foo')).toBeInTheDocument();
59-
await userEvent.unhover(screen.getByText('foo'));
60101

102+
// One menu closes and the other immediately opens
103+
await userEvent.hover(screen.getByText('Crumb Two'), {delay: null});
61104
expect(screen.queryByText('foo')).not.toBeInTheDocument();
105+
expect(screen.getByText('baz')).toBeInTheDocument();
62106
});
63107
});

static/app/views/settings/components/settingsBreadcrumb/breadcrumbDropdown.tsx

Lines changed: 103 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,20 @@
1-
import {useState} from 'react';
1+
import {useCallback, useContext, useEffect, useRef} from 'react';
2+
import {useHover} from '@react-aria/interactions';
3+
import {type OverlayTriggerState} from '@react-stately/overlays';
24

3-
import DropdownAutoCompleteMenu from 'sentry/components/dropdownAutoComplete/menu';
4-
import type {Item} from 'sentry/components/dropdownAutoComplete/types';
5+
import {
6+
CompactSelect,
7+
type SingleSelectProps,
8+
} from 'sentry/components/core/compactSelect';
9+
import {SelectContext} from 'sentry/components/core/compactSelect/control';
510

611
import Crumb from './crumb';
712
import Divider from './divider';
813
import type {RouteWithName} from './types';
914

10-
interface AdditionalDropdownProps
11-
extends Pick<
12-
React.ComponentProps<typeof DropdownAutoCompleteMenu>,
13-
'onChange' | 'busyItemsStillVisible'
14-
> {}
15-
16-
interface BreadcrumbDropdownProps extends AdditionalDropdownProps {
17-
items: Item[];
15+
interface BreadcrumbDropdownProps extends SingleSelectProps<string> {
1816
name: React.ReactNode;
19-
onSelect: (item: Item) => void;
17+
onCrumbSelect: (value: string) => void;
2018
route: RouteWithName;
2119
hasMenu?: boolean;
2220
isLast?: boolean;
@@ -27,39 +25,104 @@ function BreadcrumbDropdown({
2725
route,
2826
isLast,
2927
name,
30-
onSelect,
31-
...dropdownProps
28+
onCrumbSelect,
29+
options,
30+
value,
31+
...props
3232
}: BreadcrumbDropdownProps) {
33-
const [isActive, setIsActive] = useState(false);
33+
const {
34+
hoverProps: {onPointerEnter, onPointerLeave},
35+
isHovered,
36+
} = useHover({});
37+
38+
if (!hasMenu) {
39+
return (
40+
<Crumb>
41+
<span>{name || route.name} </span>
42+
{isLast ? null : <Divider />}
43+
</Crumb>
44+
);
45+
}
3446

3547
return (
36-
<DropdownAutoCompleteMenu
37-
blendCorner={false}
38-
isOpen={isActive}
39-
virtualizedHeight={41}
40-
onSelect={item => {
41-
setIsActive(false);
42-
onSelect(item);
43-
}}
44-
menuProps={{
45-
onMouseEnter: () => setIsActive(true),
46-
onMouseLeave: () => setIsActive(false),
48+
<CompactSelect<string>
49+
searchable
50+
options={options.map(item => ({...item, hideCheck: true}))}
51+
onChange={selected => {
52+
onCrumbSelect(selected.value);
4753
}}
48-
{...dropdownProps}
49-
>
50-
{({getActorProps, isOpen}) => (
51-
<Crumb
52-
{...getActorProps({
53-
onClick: () => setIsActive(false),
54-
onMouseEnter: () => setIsActive(true),
55-
onMouseLeave: () => setIsActive(false),
56-
})}
57-
>
58-
<span>{name || route.name} </span>
59-
{isLast ? null : <Divider isHover={hasMenu && isOpen} />}
60-
</Crumb>
54+
closeOnSelect
55+
onPointerEnter={onPointerEnter}
56+
onPointerLeave={onPointerLeave}
57+
value={value}
58+
trigger={triggerProps => (
59+
<MenuCrumb
60+
crumbLabel={name || route.name}
61+
menuHasHover={isHovered}
62+
{...triggerProps}
63+
/>
6164
)}
62-
</DropdownAutoCompleteMenu>
65+
{...props}
66+
/>
67+
);
68+
}
69+
70+
interface MenuCrumbProps extends React.ComponentProps<typeof Crumb> {
71+
crumbLabel: React.ReactNode;
72+
menuHasHover: boolean;
73+
isLast?: boolean;
74+
}
75+
76+
// XXX(epurkhiser): We have a couple hacks in place to get hover-activation of
77+
// our CompactSelect working well for these breadcrumbs.
78+
//
79+
// 1. We're using the SelectContext to retrieve the OverlayTriggerState object
80+
// for the CompactSelect that will be rendered upon hover. We need this so
81+
// we can activate the menu. Using the `isOpen` controlled prop on
82+
// CompactSelect does not work since it will not actually focus the menu.
83+
//
84+
// 2. We track the active crumb OverlayTriggerState objects so that when
85+
// activating a second crumb the first one can be immediately closed,
86+
// instead of being closed after the PointerLeave timemout.
87+
const activeCrumbStates = new Set<OverlayTriggerState | undefined>();
88+
89+
const CLOSE_MENU_TIMEOUT = 250;
90+
91+
function MenuCrumb({crumbLabel, menuHasHover, isLast, ...props}: MenuCrumbProps) {
92+
const {overlayState, overlayIsOpen} = useContext(SelectContext);
93+
const {open, close} = overlayState ?? {};
94+
95+
const closeTimeoutRef = useRef<number>(undefined);
96+
97+
useEffect(() => {
98+
activeCrumbStates.add(overlayState);
99+
return () => void activeCrumbStates.delete(overlayState);
100+
}, [overlayState]);
101+
102+
const queueMenuClose = useCallback(() => {
103+
window.clearTimeout(closeTimeoutRef.current);
104+
closeTimeoutRef.current = window.setTimeout(() => close?.(), CLOSE_MENU_TIMEOUT);
105+
}, [close]);
106+
107+
const handleOpen = useCallback(() => {
108+
activeCrumbStates.forEach(state => state?.close());
109+
window.clearTimeout(closeTimeoutRef.current);
110+
open?.();
111+
}, [open]);
112+
113+
useEffect(() => {
114+
if (menuHasHover) {
115+
window.clearTimeout(closeTimeoutRef.current);
116+
} else {
117+
queueMenuClose();
118+
}
119+
}, [menuHasHover, queueMenuClose]);
120+
121+
return (
122+
<Crumb {...props} onPointerEnter={handleOpen} onPointerLeave={queueMenuClose}>
123+
<span>{crumbLabel} </span>
124+
{isLast ? null : <Divider isHover={overlayIsOpen} />}
125+
</Crumb>
63126
);
64127
}
65128

static/app/views/settings/components/settingsBreadcrumb/menuItem.tsx

Lines changed: 0 additions & 8 deletions
This file was deleted.

static/app/views/settings/components/settingsBreadcrumb/organizationCrumb.tsx

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import styled from '@emotion/styled';
22
import sortBy from 'lodash/sortBy';
33

4+
import {OrganizationAvatar} from 'sentry/components/core/avatar/organizationAvatar';
45
import IdBadge from 'sentry/components/idBadge';
6+
import {t} from 'sentry/locale';
57
import OrganizationsStore from 'sentry/stores/organizationsStore';
68
import {useLegacyStore} from 'sentry/stores/useLegacyStore';
7-
import type {Organization} from 'sentry/types/organization';
89
import recreateRoute from 'sentry/utils/recreateRoute';
910
import {resolveRoute} from 'sentry/utils/resolveRoute';
1011
import {testableWindowLocation} from 'sentry/utils/testableWindowLocation';
@@ -14,7 +15,6 @@ import {useParams} from 'sentry/utils/useParams';
1415

1516
import BreadcrumbDropdown from './breadcrumbDropdown';
1617
import findFirstRouteWithoutRouteParam from './findFirstRouteWithoutRouteParam';
17-
import MenuItem from './menuItem';
1818
import type {SettingsBreadcrumbProps} from './types';
1919
import {CrumbLink} from '.';
2020

@@ -24,7 +24,7 @@ function OrganizationCrumb({routes, route, ...props}: SettingsBreadcrumbProps) {
2424
const organization = useOrganization();
2525
const params = useParams();
2626

27-
const handleSelect = (item: {value: Organization}) => {
27+
const handleSelect = (slug: string) => {
2828
// If we are currently in a project context, and we're attempting to switch organizations,
2929
// then we need to default to index route (e.g. `route`)
3030
//
@@ -45,12 +45,12 @@ function OrganizationCrumb({routes, route, ...props}: SettingsBreadcrumbProps) {
4545
if (destinationRoute === undefined) {
4646
return;
4747
}
48-
const itemOrg = item.value;
4948
const path = recreateRoute(destinationRoute, {
5049
routes,
51-
params: {...params, orgId: itemOrg.slug},
50+
params: {...params, orgId: slug},
5251
});
53-
const resolvedUrl = resolveRoute(path, organization, itemOrg);
52+
const newOrg = organizations.find(org => org.slug === slug)!;
53+
const resolvedUrl = resolveRoute(path, organization, newOrg);
5454
// If we have a shift in domains, we can't use history
5555
if (resolvedUrl.startsWith('http')) {
5656
testableWindowLocation.assign(resolvedUrl);
@@ -75,19 +75,18 @@ function OrganizationCrumb({routes, route, ...props}: SettingsBreadcrumbProps) {
7575
</BadgeWrapper>
7676
</CrumbLink>
7777
}
78-
onSelect={handleSelect}
78+
onCrumbSelect={handleSelect}
7979
hasMenu={hasMenu}
8080
route={route}
81-
items={sortBy(organizations, ['name']).map((org, index) => ({
82-
index,
83-
value: org,
84-
searchKey: org.name,
85-
label: (
86-
<MenuItem>
87-
<IdBadge organization={org} />
88-
</MenuItem>
89-
),
90-
}))}
81+
value={organization.slug}
82+
searchPlaceholder={t('Search Organizations')}
83+
options={sortBy(organizations, ['name'])
84+
.filter(org => org.status.id === 'active')
85+
.map(org => ({
86+
value: org.slug,
87+
leadingItems: <OrganizationAvatar organization={org} size={20} />,
88+
label: org.name,
89+
}))}
9190
{...props}
9291
/>
9392
);

0 commit comments

Comments
 (0)