Browse Source

feat(frontend): Implement a GlobalDrawer (#73318)

This PR implements a global drawer, similar to current `GlobalModal` set
up. It uses render props to allow call sites to determine its contents.
The `DrawerPanel` itself lends a lot from
[`detailPanel.tsx`](https://github.com/getsentry/sentry/blob/master/static/app/views/insights/common/components/detailPanel.tsx)
bu without any of the docking features. The default appearance of drawer
is a header with a close button and a body which adds padding.

<img width="225" alt="image"
src="https://github.com/getsentry/sentry/assets/35509934/bdd20f8b-fc9e-41ea-9baa-b44f885397c5">

By default, it will close when:
- escape is pressed
- outside the drawer is clicked
- location changes
- closeDrawer is invoked
- default close button is pressed


https://github.com/getsentry/sentry/assets/35509934/d42a9a38-3f26-4a10-8f68-8ad33770b687

I am building this out to accomodate some new designs which rely on a
drawer for issue details page. Rather than having every call site also
have a rendered empty panel on the page (as is the case for current
starfish pages), they will now all be able to mutate the same global
drawer.


https://github.com/getsentry/sentry/assets/35509934/7e050b8b-7fd5-4cfa-957e-d9383d1a4e0a

---------

Co-authored-by: Evan Purkhiser <evanpurkhiser@gmail.com>
Co-authored-by: Jonas <jonas.badalic@sentry.io>
Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
Leander Rodrigues 8 months ago
parent
commit
4bb6ded6fc

+ 7 - 0
static/app/components/autoplayVideo.spec.tsx

@@ -11,6 +11,13 @@ jest.mock('react', () => {
     useRef: jest.fn(),
   };
 });
+// XXX: Mocking useRef throws an error for AnimatePrecense, so it must be mocked as well
+jest.mock('framer-motion', () => {
+  return {
+    ...jest.requireActual('framer-motion'),
+    AnimatePresence: jest.fn().mockImplementation(({children}) => <div>{children}</div>),
+  };
+});
 
 // Use a proxy to prevent <video ref={ref}/> from overriding our ref.current
 const makeProxyMock = (video: Partial<HTMLVideoElement>) => {

+ 2 - 2
static/app/components/deprecatedDropdownMenu.spec.tsx

@@ -134,10 +134,10 @@ describe('dropdownMenuDeprecated', function () {
     );
 
     // Make sure this is only called when menu is open
-    expect(addSpy).not.toHaveBeenCalled();
+    expect(addSpy).not.toHaveBeenCalledWith('click', expect.anything(), true);
 
     await userEvent.click(screen.getByRole('button'));
-    expect(addSpy).toHaveBeenCalled();
+    expect(addSpy).toHaveBeenCalledWith('click', expect.anything(), true);
     expect(removeSpy).not.toHaveBeenCalled();
 
     await userEvent.click(screen.getByRole('button'));

+ 79 - 0
static/app/components/globalDrawer/components.tsx

@@ -0,0 +1,79 @@
+import {forwardRef} from 'react';
+import styled from '@emotion/styled';
+
+import {Button} from 'sentry/components/button';
+import type {DrawerOptions} from 'sentry/components/globalDrawer';
+import SlideOverPanel, {type SlideOverPanelProps} from 'sentry/components/slideOverPanel';
+import {IconClose} from 'sentry/icons/iconClose';
+import {t} from 'sentry/locale';
+import {space} from 'sentry/styles/space';
+
+interface DrawerPanelProps {
+  children: React.ReactNode;
+  onClose: DrawerOptions['onClose'];
+  ariaLabel?: SlideOverPanelProps['ariaLabel'];
+}
+
+export const DrawerPanel = forwardRef(function _DrawerPanel(
+  {ariaLabel, children, onClose}: DrawerPanelProps,
+  ref: React.ForwardedRef<HTMLDivElement>
+) {
+  return (
+    <DrawerContainer>
+      <SlideOverPanel
+        ariaLabel={ariaLabel}
+        slidePosition="right"
+        collapsed={false}
+        ref={ref}
+      >
+        <DrawerHeader>
+          <CloseButton
+            priority="link"
+            size="xs"
+            borderless
+            aria-label={t('Close Drawer')}
+            icon={<IconClose />}
+            onClick={onClose}
+          >
+            {t('Close')}
+          </CloseButton>
+        </DrawerHeader>
+        {children}
+      </SlideOverPanel>
+    </DrawerContainer>
+  );
+});
+
+const CloseButton = styled(Button)`
+  color: ${p => p.theme.subText};
+  &:hover {
+    color: ${p => p.theme.textColor};
+  }
+`;
+
+const DrawerHeader = styled('header')`
+  justify-content: flex-start;
+  display: flex;
+  padding: ${space(1.5)};
+  border-bottom: 1px solid ${p => p.theme.border};
+  padding-left: 24px;
+`;
+
+export const DrawerBody = styled('section')`
+  padding: ${space(2)} 24px;
+  font-size: ${p => p.theme.fontSizeMedium};
+`;
+
+const DrawerContainer = styled('div')`
+  position: fixed;
+  inset: 0;
+  z-index: ${p => p.theme.zIndex.drawer};
+  pointer-events: none;
+`;
+
+export const DrawerComponents = {
+  DrawerBody,
+  DrawerPanel,
+};
+
+export default DrawerComponents;

+ 181 - 0
static/app/components/globalDrawer/index.spec.tsx

@@ -0,0 +1,181 @@
+import {
+  render,
+  screen,
+  userEvent,
+  waitForDrawerToHide,
+} from 'sentry-test/reactTestingLibrary';
+
+import type {DrawerConfig} from 'sentry/components/globalDrawer';
+import useDrawer from 'sentry/components/globalDrawer';
+
+function GlobalDrawerTestComponent({config}: {config: DrawerConfig}) {
+  const {openDrawer, closeDrawer} = useDrawer();
+  return (
+    <div data-test-id="drawer-test-outside">
+      <button
+        data-test-id="drawer-test-open"
+        onClick={() => openDrawer(config.renderer, config.options)}
+      >
+        Open
+      </button>
+      <button data-test-id="drawer-test-close" onClick={closeDrawer}>
+        Close
+      </button>
+    </div>
+  );
+}
+
+describe('GlobalDrawer', function () {
+  const ariaLabel = 'drawer-test-aria-label';
+  beforeEach(() => {
+    jest.resetAllMocks();
+  });
+
+  it('useDrawer hook can open and close the Drawer', async function () {
+    render(
+      <GlobalDrawerTestComponent
+        config={{
+          renderer: ({Body}) => (
+            <Body data-test-id="drawer-test-content">useDrawer hook</Body>
+          ),
+          options: {ariaLabel},
+        }}
+      />
+    );
+
+    expect(
+      screen.queryByRole('complementary', {name: ariaLabel})
+    ).not.toBeInTheDocument();
+
+    await userEvent.click(screen.getByTestId('drawer-test-open'));
+
+    expect(await screen.findByTestId('drawer-test-content')).toBeInTheDocument();
+    expect(screen.getByRole('complementary', {name: ariaLabel})).toBeInTheDocument();
+
+    await userEvent.click(screen.getByTestId('drawer-test-close'));
+    await waitForDrawerToHide(ariaLabel);
+
+    expect(screen.queryByTestId('drawer-test-content')).not.toBeInTheDocument();
+    expect(
+      screen.queryByRole('complementary', {name: ariaLabel})
+    ).not.toBeInTheDocument();
+  });
+
+  it('calls onClose handler when close button is clicked', async function () {
+    const closeSpy = jest.fn();
+
+    render(
+      <GlobalDrawerTestComponent
+        config={{
+          renderer: ({Body}) => (
+            <Body data-test-id="drawer-test-content">onClose button</Body>
+          ),
+          options: {onClose: closeSpy, ariaLabel},
+        }}
+      />
+    );
+
+    await userEvent.click(screen.getByTestId('drawer-test-open'));
+
+    expect(await screen.findByTestId('drawer-test-content')).toBeInTheDocument();
+
+    await userEvent.click(screen.getByRole('button', {name: 'Close Drawer'}));
+    await waitForDrawerToHide(ariaLabel);
+
+    expect(closeSpy).toHaveBeenCalled();
+    expect(screen.queryByTestId('drawer-test-content')).not.toBeInTheDocument();
+  });
+
+  it('calls onClose handler when clicking outside the drawer', async function () {
+    const closeSpy = jest.fn();
+
+    render(
+      <GlobalDrawerTestComponent
+        config={{
+          renderer: ({Body}) => (
+            <Body data-test-id="drawer-test-content">onClose outside click</Body>
+          ),
+          options: {onClose: closeSpy, ariaLabel},
+        }}
+      />
+    );
+
+    await userEvent.click(screen.getByTestId('drawer-test-open'));
+
+    expect(await screen.findByTestId('drawer-test-content')).toBeInTheDocument();
+
+    await userEvent.click(screen.getByTestId('drawer-test-outside'));
+    await waitForDrawerToHide(ariaLabel);
+
+    expect(closeSpy).toHaveBeenCalled();
+    expect(screen.queryByTestId('drawer-test-content')).not.toBeInTheDocument();
+  });
+
+  it('calls onClose handler when closeDrawer prop is called', async function () {
+    const closeSpy = jest.fn();
+
+    render(
+      <GlobalDrawerTestComponent
+        config={{
+          renderer: ({closeDrawer: cd}) => (
+            <button data-test-id="drawer-test-content" onClick={cd}>
+              onClose prop
+            </button>
+          ),
+          options: {onClose: closeSpy, ariaLabel},
+        }}
+      />
+    );
+
+    await userEvent.click(screen.getByTestId('drawer-test-open'));
+
+    const button = screen.getByTestId('drawer-test-content');
+    expect(button).toBeInTheDocument();
+
+    await userEvent.click(button);
+    await waitForDrawerToHide(ariaLabel);
+
+    expect(closeSpy).toHaveBeenCalled();
+    expect(button).not.toBeInTheDocument();
+  });
+
+  it('ignores some close events press when option is set', async function () {
+    const closeSpy = jest.fn();
+
+    render(
+      <GlobalDrawerTestComponent
+        config={{
+          renderer: ({Body}) => (
+            <Body data-test-id="drawer-test-content">ignore close events</Body>
+          ),
+          options: {
+            onClose: closeSpy,
+            closeOnEscapeKeypress: false,
+            closeOnOutsideClick: false,
+            ariaLabel,
+          },
+        }}
+      />
+    );
+
+    await userEvent.click(screen.getByTestId('drawer-test-open'));
+
+    const content = screen.getByTestId('drawer-test-content');
+
+    await userEvent.keyboard('{Escape}');
+
+    expect(closeSpy).not.toHaveBeenCalled();
+    expect(content).toBeInTheDocument();
+
+    await userEvent.click(screen.getByTestId('drawer-test-outside'));
+
+    expect(closeSpy).not.toHaveBeenCalled();
+    expect(content).toBeInTheDocument();
+
+    await userEvent.click(screen.getByRole('button', {name: 'Close Drawer'}));
+    await waitForDrawerToHide(ariaLabel);
+
+    expect(closeSpy).toHaveBeenCalled();
+    expect(content).not.toBeInTheDocument();
+  });
+});

+ 160 - 0
static/app/components/globalDrawer/index.tsx

@@ -0,0 +1,160 @@
+import {
+  createContext,
+  useCallback,
+  useContext,
+  useLayoutEffect,
+  useRef,
+  useState,
+} from 'react';
+import {AnimatePresence} from 'framer-motion';
+
+import ErrorBoundary from 'sentry/components/errorBoundary';
+import DrawerComponents from 'sentry/components/globalDrawer/components';
+import {t} from 'sentry/locale';
+import {useHotkeys} from 'sentry/utils/useHotkeys';
+import {useLocation} from 'sentry/utils/useLocation';
+import useOnClickOutside from 'sentry/utils/useOnClickOutside';
+
+export interface DrawerOptions {
+  /**
+   * Accessbility label for the drawer
+   */
+  ariaLabel: string;
+  /**
+   * If true (default), closes the drawer when an escape key is pressed
+   */
+  closeOnEscapeKeypress?: boolean;
+  /**
+   * If true (default), closes the drawer when anywhere else is clicked
+   */
+  closeOnOutsideClick?: boolean;
+  /**
+   * Callback for when the drawer closes
+   */
+  onClose?: () => void;
+  /**
+   * Callback for when the drawer opens
+   */
+  onOpen?: () => void;
+}
+
+interface DrawerRenderProps {
+  /**
+   * Body container for the drawer
+   */
+  Body: typeof DrawerComponents.DrawerBody;
+  /**
+   * Close the drawer
+   */
+  closeDrawer: () => void;
+}
+
+type DrawerRenderer = (renderProps: DrawerRenderProps) => React.ReactNode;
+
+export interface DrawerConfig {
+  options: DrawerOptions;
+  renderer: DrawerRenderer | null;
+}
+
+interface DrawerContextType {
+  closeDrawer: () => void;
+  openDrawer: (
+    renderer: DrawerConfig['renderer'],
+    options: DrawerConfig['options']
+  ) => void;
+}
+
+const DrawerContext = createContext<DrawerContextType>({
+  openDrawer: () => {},
+  closeDrawer: () => {},
+});
+
+export function GlobalDrawer({children}) {
+  const location = useLocation();
+  const [currentDrawerConfig, overwriteDrawerConfig] = useState<
+    DrawerConfig | undefined
+  >();
+  // If no config is set, the global drawer is closed.
+  const isDrawerOpen = !!currentDrawerConfig;
+  const openDrawer = useCallback<DrawerContextType['openDrawer']>(
+    (renderer, options) => overwriteDrawerConfig({renderer, options}),
+    []
+  );
+  const closeDrawer = useCallback<DrawerContextType['closeDrawer']>(
+    () => overwriteDrawerConfig(undefined),
+    []
+  );
+
+  const handleClose = useCallback(() => {
+    currentDrawerConfig?.options?.onClose?.();
+    closeDrawer();
+  }, [currentDrawerConfig, closeDrawer]);
+
+  // Close the drawer when the browser history changes.
+  useLayoutEffect(() => closeDrawer(), [location?.pathname, closeDrawer]);
+
+  // Close the drawer when clicking outside the panel and options allow it.
+  const panelRef = useRef<HTMLDivElement>(null);
+  const handleClickOutside = useCallback(() => {
+    if (currentDrawerConfig?.options?.closeOnOutsideClick ?? true) {
+      handleClose();
+    }
+  }, [currentDrawerConfig, handleClose]);
+  useOnClickOutside(panelRef, handleClickOutside);
+
+  // Close the drawer when escape is pressed and options allow it.
+  const handleEscapePress = useCallback(() => {
+    if (currentDrawerConfig?.options?.closeOnOutsideClick ?? true) {
+      handleClose();
+    }
+  }, [currentDrawerConfig, handleClose]);
+  useHotkeys([{match: 'Escape', callback: handleEscapePress}], [handleEscapePress]);
+
+  const renderedChild = currentDrawerConfig?.renderer
+    ? currentDrawerConfig.renderer({
+        Body: DrawerComponents.DrawerBody,
+        closeDrawer: handleClose,
+      })
+    : null;
+
+  return (
+    <DrawerContext.Provider value={{openDrawer, closeDrawer}}>
+      <ErrorBoundary mini message={t('There was a problem rendering the drawer.')}>
+        <AnimatePresence>
+          {isDrawerOpen && (
+            <DrawerComponents.DrawerPanel
+              ariaLabel={currentDrawerConfig.options.ariaLabel}
+              onClose={handleClose}
+              ref={panelRef}
+            >
+              {renderedChild}
+            </DrawerComponents.DrawerPanel>
+          )}
+        </AnimatePresence>
+      </ErrorBoundary>
+      {children}
+    </DrawerContext.Provider>
+  );
+}
+
+/**
+ * Returns helper functions to control the slide out drawer above the page content. For example:
+ * ```
+ * const {openDrawer, closeDrawer} = useDrawer()
+ * ```
+ *
+ * The `openDrawer` function accepts a renderer, and options. By default, the drawer will close
+ * on outside clicks, and 'Escape' key presses. For example:
+ * ```
+ * openDrawer((Body) => <Body><MyComponent /></Body>, {closeOnOutsideClick: false})
+ * ```
+ *
+ * The `closeDrawer` function accepts no parameters and closes the drawer, unmounting its contents.
+ * For example:
+ * ```
+ * openDrawer(() => <button onClick={closeDrawer}>Close!</button>)
+ * ```
+ */
+export default function useDrawer() {
+  return useContext(DrawerContext);
+}

+ 2 - 2
static/app/components/globalModal/index.tsx

@@ -182,8 +182,8 @@ function GlobalModal({onClose}: Props) {
   // Close the modal when the browser history changes.
   //
   // XXX: We're using useEffectAfterFirstRender primarily to support tests
-  // which render the GlobalModal after a modaul has already been registered in
-  // the modal store, meaning it would be closed immeidately.
+  // which render the GlobalModal after a modal has already been registered in
+  // the modal store, meaning it would be closed immediately.
   useEffectAfterFirstRender(() => actionCloseModal(), [location.pathname]);
 
   // Default to enabled backdrop

+ 24 - 22
static/app/views/insights/common/components/slideOverPanel.tsx → static/app/components/slideOverPanel.tsx

@@ -3,7 +3,7 @@ import {forwardRef, useEffect} from 'react';
 import isPropValid from '@emotion/is-prop-valid';
 import {css} from '@emotion/react';
 import styled from '@emotion/styled';
-import {AnimatePresence, motion} from 'framer-motion';
+import {motion} from 'framer-motion';
 
 import {space} from 'sentry/styles/space';
 
@@ -20,9 +20,10 @@ const COLLAPSED_STYLES = {
   right: {opacity: 0, x: PANEL_WIDTH, y: 0},
 };
 
-type SlideOverPanelProps = {
+export type SlideOverPanelProps = {
   children: React.ReactNode;
   collapsed: boolean;
+  ariaLabel?: string;
   onOpen?: () => void;
   slidePosition?: 'right' | 'bottom';
 };
@@ -30,7 +31,7 @@ type SlideOverPanelProps = {
 export default forwardRef(SlideOverPanel);
 
 function SlideOverPanel(
-  {collapsed, children, onOpen, slidePosition}: SlideOverPanelProps,
+  {ariaLabel, collapsed, children, onOpen, slidePosition}: SlideOverPanelProps,
   ref: ForwardedRef<HTMLDivElement>
 ) {
   useEffect(() => {
@@ -45,25 +46,24 @@ function SlideOverPanel(
     ? COLLAPSED_STYLES[slidePosition]
     : COLLAPSED_STYLES.right;
 
-  return (
-    <AnimatePresence>
-      {!collapsed && (
-        <_SlideOverPanel
-          ref={ref}
-          initial={collapsedStyle}
-          animate={openStyle}
-          exit={collapsedStyle}
-          slidePosition={slidePosition}
-          transition={{
-            type: 'spring',
-            stiffness: 500,
-            damping: 50,
-          }}
-        >
-          {children}
-        </_SlideOverPanel>
-      )}
-    </AnimatePresence>
+  return collapsed ? null : (
+    <_SlideOverPanel
+      ref={ref}
+      initial={collapsedStyle}
+      animate={openStyle}
+      exit={collapsedStyle}
+      slidePosition={slidePosition}
+      transition={{
+        type: 'spring',
+        stiffness: 500,
+        damping: 50,
+      }}
+      role="complementary"
+      aria-hidden={collapsed}
+      aria-label={ariaLabel ?? 'slide out drawer'}
+    >
+      {children}
+    </_SlideOverPanel>
   );
 }
 
@@ -82,6 +82,8 @@ const _SlideOverPanel = styled(motion.div, {
   left: ${space(2)};
 
   overflow: auto;
+  pointer-events: auto;
+  overscroll-behavior: contain;
 
   z-index: ${p => p.theme.zIndex.modal + 1};
 

+ 1 - 0
static/app/utils/theme.tsx

@@ -725,6 +725,7 @@ const commonTheme = {
 
     // If you change modal also update shared-components.less
     // as the z-index for bootstrap modals lives there.
+    drawer: 9999,
     modal: 10000,
     toast: 10001,
 

+ 9 - 6
static/app/views/app/index.tsx

@@ -11,6 +11,7 @@ import {openCommandPalette} from 'sentry/actionCreators/modal';
 import {fetchOrganizations} from 'sentry/actionCreators/organizations';
 import {initApiClientErrorHandling} from 'sentry/api';
 import ErrorBoundary from 'sentry/components/errorBoundary';
+import {GlobalDrawer} from 'sentry/components/globalDrawer';
 import GlobalModal from 'sentry/components/globalModal';
 import Hook from 'sentry/components/hook';
 import Indicators from 'sentry/components/indicators';
@@ -234,12 +235,14 @@ function App({children, params}: Props) {
     <Profiler id="App" onRender={onRenderCallback}>
       <OrganizationContextProvider>
         <AsyncSDKIntegrationContextProvider>
-          <MainContainer tabIndex={-1} ref={mainContainerRef}>
-            <GlobalModal onClose={handleModalClose} />
-            <SystemAlerts className="messages-container" />
-            <Indicators className="indicators-container" />
-            <ErrorBoundary>{renderBody()}</ErrorBoundary>
-          </MainContainer>
+          <GlobalDrawer>
+            <MainContainer tabIndex={-1} ref={mainContainerRef}>
+              <GlobalModal onClose={handleModalClose} />
+              <SystemAlerts className="messages-container" />
+              <Indicators className="indicators-container" />
+              <ErrorBoundary>{renderBody()}</ErrorBoundary>
+            </MainContainer>
+          </GlobalDrawer>
         </AsyncSDKIntegrationContextProvider>
       </OrganizationContextProvider>
     </Profiler>

+ 7 - 0
static/app/views/insights/common/components/chart.spec.tsx

@@ -14,6 +14,13 @@ jest.mock('react', () => {
     useRef: jest.fn(),
   };
 });
+// XXX: Mocking useRef throws an error for AnimatePrecense, so it must be mocked as well
+jest.mock('framer-motion', () => {
+  return {
+    ...jest.requireActual('framer-motion'),
+    AnimatePresence: jest.fn().mockImplementation(({children}) => <div>{children}</div>),
+  };
+});
 
 describe('Chart', function () {
   test('it shows an error panel if an error prop is supplied', function () {

Some files were not shown because too many files changed in this diff