feat(nav): Open secondary navigation in overlay when collapsed

Author: malwilley

static/app/components/hovercard.tsx

@@ -1,7 +1,8 @@
-import {Fragment, useCallback, useRef} from 'react';
+import {createContext, Fragment, useCallback, useContext, useMemo, useRef} from 'react';
 import {createPortal} from 'react-dom';
 import {useTheme} from '@emotion/react';
 import styled from '@emotion/styled';
+import type {State} from '@popperjs/core';
 import {useResizeObserver} from '@react-aria/utils';
 import {AnimatePresence} from 'framer-motion';
 
@@ -16,6 +17,10 @@ interface HovercardProps extends Omit<UseHoverOverlayProps, 'isHoverable'> {
    * Classname to apply to the hovercard
    */
   children: React.ReactNode;
+  /**
+   * Whether to animate the hovercard in/out
+   */
+  animated?: boolean;
   /**
    * Element to display in the body
    */
@@ -47,11 +52,33 @@ type UseOverOverlayState = ReturnType<typeof useHoverOverlay>;
 interface HovercardContentProps
   extends Pick<
     HovercardProps,
-    'bodyClassName' | 'className' | 'header' | 'body' | 'tipColor' | 'tipBorderColor'
+    | 'animated'
+    | 'bodyClassName'
+    | 'className'
+    | 'header'
+    | 'body'
+    | 'tipColor'
+    | 'tipBorderColor'
   > {
   hoverOverlayState: Omit<UseOverOverlayState, 'isOpen' | 'wrapTrigger'>;
 }
 
+interface HovercardProviderValue {
+  isOpen: boolean;
+  reset: () => void;
+  update: (() => Promise<Partial<State>>) | null;
+}
+
+const HovercardContext = createContext<HovercardProviderValue>({
+  isOpen: false,
+  reset: () => {},
+  update: null,
+});
+
+export function useHovercardContext() {
+  return useContext(HovercardContext);
+}
+
 function useUpdateOverlayPositionOnContentChange({
   update,
 }: Pick<UseOverOverlayState, 'update'>) {
@@ -70,6 +97,7 @@ function useUpdateOverlayPositionOnContentChange({
 }
 
 function HovercardContent({
+  animated,
   body,
   bodyClassName,
   className,
@@ -84,7 +112,7 @@ function HovercardContent({
   return (
     <PositionWrapper zIndex={theme.zIndex.hovercard} {...overlayProps}>
       <StyledHovercard
-        animated
+        animated={animated}
         arrowProps={{
           ...arrowProps,
           size: 20,
@@ -114,6 +142,7 @@ function Hovercard({
   displayTimeout = 100,
   tipBorderColor = 'translucentBorder',
   tipColor = 'backgroundElevated',
+  animated = true,
   ...hoverOverlayProps
 }: HovercardProps): React.ReactElement {
   const {wrapTrigger, isOpen, ...hoverOverlayState} = useHoverOverlay({
@@ -124,16 +153,25 @@ function Hovercard({
     ...hoverOverlayProps,
   });
 
+  const contextValue = useMemo<HovercardProviderValue>(
+    () => ({
+      isOpen,
+      reset: hoverOverlayState.reset,
+      update: hoverOverlayState.update,
+    }),
+    [isOpen, hoverOverlayState.reset, hoverOverlayState.update]
+  );
   // Nothing to render if no header or body. Be consistent with wrapping the
   // children with the trigger in the case that the body / header is set while
   // the trigger is hovered.
   if (!body && !header) {
     return <Fragment>{wrapTrigger(children)}</Fragment>;
   }
 
-  const hovercardContent = isOpen && (
+  const hovercardContent = isOpen ? (
     <HovercardContent
       {...{
+        animated,
         body,
         bodyClassName,
         className,
@@ -143,13 +181,20 @@ function Hovercard({
         hoverOverlayState,
       }}
     />
-  );
+  ) : null;
 
   return (
-    <Fragment>
+    <HovercardContext.Provider value={contextValue}>
       {wrapTrigger(children)}
-      {createPortal(<AnimatePresence>{hovercardContent}</AnimatePresence>, document.body)}
-    </Fragment>
+      {createPortal(
+        animated ? (
+          <AnimatePresence>{hovercardContent}</AnimatePresence>
+        ) : (
+          hovercardContent
+        ),
+        document.body
+      )}
+    </HovercardContext.Provider>
   );
 }
 

static/app/utils/useHoverOverlay.tsx

@@ -53,6 +53,7 @@ function makeDefaultPopperModifiers(arrowElement: HTMLElement | null, offset: nu
       options: {
         padding: 12,
         altAxis: true,
+        boundary: document.body,
       },
     },
   ];

static/app/views/admin/adminLayout.tsx

@@ -5,6 +5,7 @@ import {t} from 'sentry/locale';
 import type {RouteComponentProps} from 'sentry/types/legacyReactRouter';
 import useOrganization from 'sentry/utils/useOrganization';
 import {prefersStackedNav} from 'sentry/views/nav/prefersStackedNav';
+import {PrimaryNavGroup} from 'sentry/views/nav/types';
 import {BreadcrumbProvider} from 'sentry/views/settings/components/settingsBreadcrumb/context';
 import SettingsLayout from 'sentry/views/settings/components/settingsLayout';
 import SettingsNavigation from 'sentry/views/settings/components/settingsNavigation';
@@ -40,6 +41,7 @@ export function AdminNavigation() {
           ],
         },
       ]}
+      primaryNavGroup={PrimaryNavGroup.ADMIN}
     />
   );
 }

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

@@ -78,6 +78,16 @@ describe('Nav', function () {
       body: {},
     });
 
+    MockApiClient.addMockResponse({
+      url: `/organizations/org-slug/explore/saved/`,
+      body: [],
+    });
+
+    MockApiClient.addMockResponse({
+      url: `/organizations/org-slug/dashboards/`,
+      body: [],
+    });
+
     ConfigStore.set('user', {
       ...ConfigStore.get('user'),
       options: {
@@ -243,58 +253,48 @@ describe('Nav', function () {
 
         await userEvent.click(screen.getByRole('button', {name: 'Collapse'}));
 
-        expect(screen.getByTestId('collapsed-secondary-sidebar')).toBeInTheDocument();
+        await waitFor(() => {
+          expect(
+            screen.queryByRole('navigation', {name: 'Secondary Navigation'})
+          ).not.toBeInTheDocument();
+        });
 
-        await userEvent.click(screen.getByRole('button', {name: 'Expand'}));
+        await userEvent.hover(screen.getByRole('link', {name: 'Issues'}));
 
         expect(
-          screen.queryByTestId('collapsed-secondary-sidebar')
-        ).not.toBeInTheDocument();
+          await screen.findByRole('navigation', {name: 'Secondary Navigation'})
+        ).toBeInTheDocument();
+
+        await userEvent.click(screen.getByRole('button', {name: 'Expand'}));
       });
 
       it('remembers collapsed state', async function () {
         localStorage.setItem(NAV_SIDEBAR_COLLAPSED_LOCAL_STORAGE_KEY, 'true');
 
         renderNav();
 
-        expect(
-          await screen.findByTestId('collapsed-secondary-sidebar')
-        ).toBeInTheDocument();
-        expect(screen.getByRole('button', {name: 'Expand'})).toBeInTheDocument();
+        await waitFor(() => {
+          expect(
+            screen.queryByRole('navigation', {name: 'Secondary Navigation'})
+          ).not.toBeInTheDocument();
+        });
       });
 
-      it('expands on hover', async function () {
+      it('closes secondary nav overlay when navigating to a new route', async function () {
         localStorage.setItem(NAV_SIDEBAR_COLLAPSED_LOCAL_STORAGE_KEY, 'true');
 
         renderNav();
 
-        expect(
-          await screen.findByTestId('collapsed-secondary-sidebar')
-        ).toBeInTheDocument();
+        await userEvent.hover(screen.getByRole('link', {name: 'Explore'}));
 
-        expect(screen.getByTestId('collapsed-secondary-sidebar')).toHaveAttribute(
-          'data-visible',
-          'false'
-        );
+        await screen.findByRole('navigation', {name: 'Secondary Navigation'});
 
-        // Moving pointer over the primary navigation should expand the sidebar
-        await userEvent.hover(
-          screen.getByRole('navigation', {name: 'Primary Navigation'})
-        );
-        expect(screen.getByTestId('collapsed-secondary-sidebar')).toHaveAttribute(
-          'data-visible',
-          'true'
-        );
+        await userEvent.click(screen.getByRole('link', {name: 'Traces'}));
 
-        // Moving pointer away should hide the sidebar
-        await userEvent.unhover(
-          screen.getByRole('navigation', {name: 'Primary Navigation'})
-        );
         await waitFor(() => {
-          expect(screen.getByTestId('collapsed-secondary-sidebar')).toHaveAttribute(
-            'data-visible',
-            'false'
-          );
+          expect(
+            screen.queryByRole('navigation', {name: 'Secondary Navigation'})
+          ).not.toBeInTheDocument();
         });
       });
     });

static/app/views/nav/index.tsx

@@ -4,18 +4,21 @@ import styled from '@emotion/styled';
 
 import {useNavContext} from 'sentry/views/nav/context';
 import MobileTopbar from 'sentry/views/nav/mobileTopbar';
+import {SecondaryNav} from 'sentry/views/nav/secondary/secondary';
 import {SecondaryNavContent} from 'sentry/views/nav/secondary/secondaryNavContent';
 import {Sidebar} from 'sentry/views/nav/sidebar';
 import {
   NavigationTourProvider,
   useStackedNavigationTour,
 } from 'sentry/views/nav/tour/tour';
 import {NavLayout} from 'sentry/views/nav/types';
+import {useActiveNavGroup} from 'sentry/views/nav/useActiveNavGroup';
 
 function NavContent() {
   const {layout, navParentRef} = useNavContext();
   const {currentStepId, endTour} = useStackedNavigationTour();
   const tourIsActive = currentStepId !== null;
+  const activeNavGroup = useActiveNavGroup();
 
   // The tour only works with the sidebar layout, so if we change to the mobile
   // layout in the middle of the tour, it needs to end.
@@ -32,7 +35,9 @@ function NavContent() {
       isMobile={layout === NavLayout.MOBILE}
     >
       {layout === NavLayout.SIDEBAR ? <Sidebar /> : <MobileTopbar />}
-      <SecondaryNavContent />
+      <SecondaryNav>
+        <SecondaryNavContent group={activeNavGroup} />
+      </SecondaryNav>
     </NavContainer>
   );
 }