Skip to content

Commit e6f182e

Browse files
malwilleymalwilley
authored
feat(nav): Improve preview of other nav groups on hover (#92352)
- Switches back to a full-height secondary navigation when collapsed - Adds previewing of other nav groups even when the navigation is expanded - Adds some subtle animations when switching the active nav group
1 parent 50140ce commit e6f182e

File tree

10 files changed

+340
-107
lines changed

10 files changed

+340
-107
lines changed

‎static/app/views/nav/constants.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,6 @@ export const NAV_SIDEBAR_COLLAPSED_LOCAL_STORAGE_KEY = 'navigation-sidebar-is-co
22

33
export const PRIMARY_SIDEBAR_WIDTH = 74;
44
export const SECONDARY_SIDEBAR_WIDTH = 190;
5+
6+
// Slightly delay closing the nav to prevent accidental dismissal
7+
export const NAV_SIDEBAR_COLLAPSE_DELAY_MS = 200;

‎static/app/views/nav/context.tsx

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,17 @@ import {useTheme} from '@emotion/react';
44
import {useLocalStorageState} from 'sentry/utils/useLocalStorageState';
55
import useMedia from 'sentry/utils/useMedia';
66
import {NAV_SIDEBAR_COLLAPSED_LOCAL_STORAGE_KEY} from 'sentry/views/nav/constants';
7+
import type {PrimaryNavGroup} from 'sentry/views/nav/types';
78
import {NavLayout} from 'sentry/views/nav/types';
89

910
interface NavContext {
11+
activePrimaryNavGroup: PrimaryNavGroup | null;
1012
endInteraction: () => void;
1113
isCollapsed: boolean;
1214
isInteractingRef: React.RefObject<boolean | null>;
1315
layout: NavLayout;
1416
navParentRef: React.RefObject<HTMLDivElement | null>;
17+
setActivePrimaryNavGroup: (activePrimaryNavGroup: PrimaryNavGroup | null) => void;
1518
setIsCollapsed: (isCollapsed: boolean) => void;
1619
setShowTourReminder: (showTourReminder: boolean) => void;
1720
showTourReminder: boolean;
@@ -28,6 +31,8 @@ const NavContext = createContext<NavContext>({
2831
endInteraction: () => {},
2932
showTourReminder: false,
3033
setShowTourReminder: () => {},
34+
activePrimaryNavGroup: null,
35+
setActivePrimaryNavGroup: () => {},
3136
});
3237

3338
export function useNavContext(): NavContext {
@@ -42,6 +47,8 @@ export function NavContextProvider({children}: {children: React.ReactNode}) {
4247
false
4348
);
4449
const [showTourReminder, setShowTourReminder] = useState(false);
50+
const [activePrimaryNavGroup, setActivePrimaryNavGroup] =
51+
useState<PrimaryNavGroup | null>(null);
4552

4653
const theme = useTheme();
4754
const isMobile = useMedia(`(max-width: ${theme.breakpoints.medium})`);
@@ -65,6 +72,8 @@ export function NavContextProvider({children}: {children: React.ReactNode}) {
6572
endInteraction,
6673
showTourReminder,
6774
setShowTourReminder,
75+
activePrimaryNavGroup,
76+
setActivePrimaryNavGroup,
6877
}),
6978
[
7079
isMobile,
@@ -74,6 +83,8 @@ export function NavContextProvider({children}: {children: React.ReactNode}) {
7483
endInteraction,
7584
showTourReminder,
7685
setShowTourReminder,
86+
activePrimaryNavGroup,
87+
setActivePrimaryNavGroup,
7788
]
7889
);
7990

‎static/app/views/nav/index.spec.tsx

Lines changed: 42 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,16 @@ describe('Nav', function () {
195195
).toBeInTheDocument();
196196
});
197197

198+
it('previews secondary nav when hovering over other primary items', async function () {
199+
renderNav();
200+
201+
await userEvent.hover(screen.getByRole('link', {name: 'Explore'}));
202+
await screen.findByRole('link', {name: 'Traces'});
203+
204+
await userEvent.hover(screen.getByRole('link', {name: 'Dashboards'}));
205+
await screen.findByRole('link', {name: 'All Dashboards'});
206+
});
207+
198208
describe('sections', function () {
199209
it('renders organization/account settings secondary nav when on settings routes', function () {
200210
renderNav({initialPathname: '/settings/organization/'});
@@ -253,48 +263,58 @@ describe('Nav', function () {
253263

254264
await userEvent.click(screen.getByRole('button', {name: 'Collapse'}));
255265

256-
await waitFor(() => {
257-
expect(
258-
screen.queryByRole('navigation', {name: 'Secondary Navigation'})
259-
).not.toBeInTheDocument();
260-
});
266+
expect(screen.getByTestId('collapsed-secondary-sidebar')).toBeInTheDocument();
261267

262-
await userEvent.hover(screen.getByRole('link', {name: 'Issues'}));
268+
await userEvent.click(screen.getByRole('button', {name: 'Expand'}));
263269

264270
expect(
265-
await screen.findByRole('navigation', {name: 'Secondary Navigation'})
266-
).toBeInTheDocument();
267-
268-
await userEvent.click(screen.getByRole('button', {name: 'Expand'}));
271+
screen.queryByTestId('collapsed-secondary-sidebar')
272+
).not.toBeInTheDocument();
269273
});
270274

271275
it('remembers collapsed state', async function () {
272276
localStorage.setItem(NAV_SIDEBAR_COLLAPSED_LOCAL_STORAGE_KEY, 'true');
273277

274278
renderNav();
275279

276-
await waitFor(() => {
277-
expect(
278-
screen.queryByRole('navigation', {name: 'Secondary Navigation'})
279-
).not.toBeInTheDocument();
280-
});
280+
expect(
281+
await screen.findByTestId('collapsed-secondary-sidebar')
282+
).toBeInTheDocument();
283+
expect(screen.getByRole('button', {name: 'Expand'})).toBeInTheDocument();
281284
});
282285

283-
it('closes secondary nav overlay when navigating to a new route', async function () {
286+
it('expands on hover', async function () {
284287
localStorage.setItem(NAV_SIDEBAR_COLLAPSED_LOCAL_STORAGE_KEY, 'true');
285288

286289
renderNav();
287290

288-
await userEvent.hover(screen.getByRole('link', {name: 'Explore'}));
291+
expect(
292+
await screen.findByTestId('collapsed-secondary-sidebar')
293+
).toBeInTheDocument();
289294

290-
await screen.findByRole('navigation', {name: 'Secondary Navigation'});
295+
expect(screen.getByTestId('collapsed-secondary-sidebar')).toHaveAttribute(
296+
'data-visible',
297+
'false'
298+
);
291299

292-
await userEvent.click(screen.getByRole('link', {name: 'Traces'}));
300+
// Moving pointer over the primary navigation should expand the sidebar
301+
await userEvent.hover(
302+
screen.getByRole('navigation', {name: 'Primary Navigation'})
303+
);
304+
expect(screen.getByTestId('collapsed-secondary-sidebar')).toHaveAttribute(
305+
'data-visible',
306+
'true'
307+
);
293308

309+
// Moving pointer away should hide the sidebar
310+
await userEvent.unhover(
311+
screen.getByRole('navigation', {name: 'Primary Navigation'})
312+
);
294313
await waitFor(() => {
295-
expect(
296-
screen.queryByRole('navigation', {name: 'Secondary Navigation'})
297-
).not.toBeInTheDocument();
314+
expect(screen.getByTestId('collapsed-secondary-sidebar')).toHaveAttribute(
315+
'data-visible',
316+
'false'
317+
);
298318
});
299319
});
300320
});

‎static/app/views/nav/index.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,13 @@ import {
1010
useStackedNavigationTour,
1111
} from 'sentry/views/nav/tour/tour';
1212
import {NavLayout} from 'sentry/views/nav/types';
13+
import {useResetActiveNavGroup} from 'sentry/views/nav/useResetActiveNavGroup';
1314

1415
function NavContent() {
1516
const {layout, navParentRef} = useNavContext();
1617
const {currentStepId, endTour} = useStackedNavigationTour();
1718
const tourIsActive = currentStepId !== null;
19+
const hoverProps = useResetActiveNavGroup();
1820

1921
// The tour only works with the sidebar layout, so if we change to the mobile
2022
// layout in the middle of the tour, it needs to end.
@@ -29,6 +31,7 @@ function NavContent() {
2931
ref={navParentRef}
3032
tourIsActive={tourIsActive}
3133
isMobile={layout === NavLayout.MOBILE}
34+
{...hoverProps}
3235
>
3336
{layout === NavLayout.SIDEBAR ? <Sidebar /> : <MobileTopbar />}
3437
</NavContainer>

‎static/app/views/nav/primary/components.tsx

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@ import {Fragment, type MouseEventHandler} from 'react';
22
import type {Theme} from '@emotion/react';
33
import {css, useTheme} from '@emotion/react';
44
import styled from '@emotion/styled';
5+
import {useHover} from '@react-aria/interactions';
56

67
import {Button} from 'sentry/components/core/button';
78
import {Tooltip} from 'sentry/components/core/tooltip';
89
import {DropdownMenu, type MenuItemProps} from 'sentry/components/dropdownMenu';
9-
import {useHovercardContext} from 'sentry/components/hovercard';
1010
import InteractionStateLayer from 'sentry/components/interactionStateLayer';
1111
import Link, {linkStyles} from 'sentry/components/links/link';
1212
import {SIDEBAR_NAVIGATION_SOURCE} from 'sentry/components/sidebar/utils';
@@ -22,7 +22,6 @@ import useOrganization from 'sentry/utils/useOrganization';
2222
import {PRIMARY_SIDEBAR_WIDTH} from 'sentry/views/nav/constants';
2323
import {useNavContext} from 'sentry/views/nav/context';
2424
import {PRIMARY_NAV_GROUP_CONFIG} from 'sentry/views/nav/primary/config';
25-
import {SecondaryHovercard} from 'sentry/views/nav/primary/secondaryHovercard';
2625
import type {PrimaryNavGroup} from 'sentry/views/nav/types';
2726
import {NavLayout} from 'sentry/views/nav/types';
2827
import {isLinkActive} from 'sentry/views/nav/utils';
@@ -148,26 +147,29 @@ function SidebarNavLink({
148147
group,
149148
}: SidebarItemLinkProps) {
150149
const organization = useOrganization();
151-
const {reset: closeCollapsedNavHovercard} = useHovercardContext();
152-
const {layout} = useNavContext();
150+
const {layout, activePrimaryNavGroup, setActivePrimaryNavGroup} = useNavContext();
153151
const theme = useTheme();
154152
const location = useLocation();
155153
const isActive = isLinkActive(normalizeUrl(activeTo, location), location.pathname);
156154
const label = PRIMARY_NAV_GROUP_CONFIG[group].label;
157155

156+
const {hoverProps} = useHover({
157+
onHoverStart: () => {
158+
setActivePrimaryNavGroup(group);
159+
},
160+
});
161+
158162
return (
159163
<NavLink
160164
to={to}
161165
state={{source: SIDEBAR_NAVIGATION_SOURCE}}
162166
onClick={() => {
163167
recordPrimaryItemClick(analyticsKey, organization);
164-
165-
// When the nav is collapsed, clicking on a link will close the hovercard.
166-
closeCollapsedNavHovercard();
167168
}}
168-
aria-selected={isActive}
169+
aria-selected={activePrimaryNavGroup === group ? true : isActive}
169170
aria-current={isActive ? 'page' : undefined}
170171
isMobile={layout === NavLayout.MOBILE}
172+
{...hoverProps}
171173
>
172174
{layout === NavLayout.MOBILE ? (
173175
<Fragment>
@@ -196,16 +198,14 @@ export function SidebarLink({
196198

197199
return (
198200
<SidebarItem label={label} showLabel>
199-
<SecondaryHovercard group={group}>
200-
<SidebarNavLink
201-
to={to}
202-
activeTo={activeTo}
203-
analyticsKey={analyticsKey}
204-
group={group}
205-
>
206-
{children}
207-
</SidebarNavLink>
208-
</SecondaryHovercard>
201+
<SidebarNavLink
202+
to={to}
203+
activeTo={activeTo}
204+
analyticsKey={analyticsKey}
205+
group={group}
206+
>
207+
{children}
208+
</SidebarNavLink>
209209
</SidebarItem>
210210
);
211211
}
@@ -413,14 +413,15 @@ const ChonkNavLink = chonkStyled(Link, {
413413
}
414414
}
415415
416-
&:hover {
416+
&:hover,
417+
&[aria-selected='true'] {
417418
color: ${p => p.theme.tokens.content.muted};
418419
${NavLinkIconContainer} {
419420
background-color: ${p => p.theme.colors.gray100};
420421
}
421422
}
422423
423-
&[aria-selected='true'] {
424+
&[aria-current='page'] {
424425
color: ${p => p.theme.purple400};
425426
426427
&::before { opacity: 1; }
@@ -450,7 +451,8 @@ const StyledNavLink = styled(Link, {
450451
padding-bottom: ${space(1)};
451452
gap: ${space(0.5)};
452453
453-
&:hover {
454+
&:hover,
455+
&[aria-selected='true'] {
454456
${NavLinkIconContainer} {
455457
&::before {
456458
opacity: 0.06;
@@ -466,7 +468,7 @@ const StyledNavLink = styled(Link, {
466468
}
467469
}
468470
469-
&[aria-selected='true'] {
471+
&[aria-current='page'] {
470472
color: ${p.theme.purple400};
471473
472474
${NavLinkIconContainer} {

‎static/app/views/nav/primary/secondaryHovercard.tsx

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

0 commit comments

Comments
 (0)