feat(insights): Use GlobalDrawer instead of detail panels (#82534)

Closes #81273 Replaces all Insights side panels with new `GlobalDrawer` implementations that match the drawers in Issues. The visual changes are minimal 1. Slightly different animations 2. A "Close" button on the top left of the drawer 3. Scroll bars on the left of the drawer 4. Tighter padding 5. Panel doesn't re-animate as often on clicking another transaction There are small feature changes! - no more docking bottom or right. Docking is to the right only - panels now have closing animations ## Code Changes `GlobalDrawer` is invoked with a hook, rather than rendered inline. So, I had to make some changes. **Before:** - rendering panels mostly unconditionally like `<CacheSamplesPanel />` - each sample panel checks the URL to see if it's supposed to be open - if open, renders itself, otherwise stays invisible - on close, updates the URL, which re-renders and hides This is problematic for a bunch of reasons: 1. When components can decide to not render themselves, the code flow is confusing. Parents should generally decide when to render children 2. When a component is rendered but hidden, it runs its side effects. This means that in our case, network requests were sometimes going out, even though the panel isn't visible **After:** - the panel is rendered using a `useEffect`, the effect detects a necessary URL parameter or state, and open the panel if necessary. All conditions that the panel used to check for itself are pulled up into the parent - the panel is rendered straight, it doesn't have any conditionals to check if it's supposed to be open - on close, the panel hides itself, and tells the parent to update the URL or state as necessary This is tidier, avoids component side-effects, and preserves closing animations! Plus, this means I can re-use a header component, make the analytics tracking consistent, etc. --- This only leaves a small handful of users of `DetailPanel`, mostly in the trace view.
getsentry · Jan 22, 2025 · fd3e54e · fd3e54e
1 parent ef2780c
commit fd3e54e
Show file tree

Hide file tree

Showing 30 changed files with 614 additions and 598 deletions.
diff --git a/static/app/views/insights/browser/resources/views/resourceSummaryPage.tsx b/static/app/views/insights/browser/resources/views/resourceSummaryPage.tsx
@@ -1,4 +1,4 @@
-import React from 'react';
+import React, {useEffect} from 'react';
 
 import * as Layout from 'sentry/components/layouts/thirds';
 import {t, tct} from 'sentry/locale';
@@ -23,6 +23,7 @@ import {ModuleBodyUpsellHook} from 'sentry/views/insights/common/components/modu
 import {ToolRibbon} from 'sentry/views/insights/common/components/ribbon';
 import {useSpanMetrics} from 'sentry/views/insights/common/queries/useDiscover';
 import {useModuleURL} from 'sentry/views/insights/common/utils/useModuleURL';
+import {useSamplesDrawer} from 'sentry/views/insights/common/utils/useSamplesDrawer';
 import SubregionSelector from 'sentry/views/insights/common/views/spans/selectors/subregionSelector';
 import {SampleList} from 'sentry/views/insights/common/views/spanSummaryPage/sampleList';
 import {FrontendHeader} from 'sentry/views/insights/pages/frontend/frontendPageHeader';
@@ -47,6 +48,27 @@ function ResourceSummary() {
   const {
     query: {transaction},
   } = useLocation();
+
+  const {openSamplesDrawer} = useSamplesDrawer({
+    Component: (
+      <SampleList
+        transactionRoute={webVitalsModuleURL}
+        subregions={filters[SpanMetricsField.USER_GEO_SUBREGION]}
+        groupId={groupId!}
+        moduleName={ModuleName.RESOURCE}
+        transactionName={transaction as string}
+        referrer={TraceViewSources.ASSETS_MODULE}
+      />
+    ),
+    moduleName: ModuleName.RESOURCE,
+  });
+
+  useEffect(() => {
+    if (transaction) {
+      openSamplesDrawer();
+    }
+  });
+
   const {data, isPending} = useSpanMetrics(
     {
       search: MutableSearch.fromQueryObject({
@@ -139,17 +161,6 @@ function ResourceSummary() {
               <ModuleLayout.Full>
                 <ResourceSummaryTable />
               </ModuleLayout.Full>
-
-              <ModuleLayout.Full>
-                <SampleList
-                  transactionRoute={webVitalsModuleURL}
-                  subregions={filters[SpanMetricsField.USER_GEO_SUBREGION]}
-                  groupId={groupId!}
-                  moduleName={ModuleName.RESOURCE}
-                  transactionName={transaction as string}
-                  referrer={TraceViewSources.ASSETS_MODULE}
-                />
-              </ModuleLayout.Full>
             </ModuleLayout.Layout>
           </Layout.Main>
         </Layout.Body>

diff --git a/static/app/views/insights/browser/webVitals/components/pageOverviewWebVitalsDetailPanel.tsx b/static/app/views/insights/browser/webVitals/components/pageOverviewWebVitalsDetailPanel.tsx
@@ -2,6 +2,7 @@ import {useMemo} from 'react';
 import styled from '@emotion/styled';
 
 import type {LineChartSeries} from 'sentry/components/charts/lineChart';
+import {DrawerHeader} from 'sentry/components/globalDrawer/components';
 import type {
   GridColumnHeader,
   GridColumnOrder,
@@ -38,7 +39,7 @@ import type {
 } from 'sentry/views/insights/browser/webVitals/types';
 import decodeBrowserTypes from 'sentry/views/insights/browser/webVitals/utils/queryParameterDecoders/browserType';
 import useProfileExists from 'sentry/views/insights/browser/webVitals/utils/useProfileExists';
-import DetailPanel from 'sentry/views/insights/common/components/detailPanel';
+import {SampleDrawerBody} from 'sentry/views/insights/common/components/sampleDrawerBody';
 import {SpanIndexedField, type SubregionCode} from 'sentry/views/insights/types';
 import {TraceViewSources} from 'sentry/views/performance/newTraceDetails/traceHeader/breadcrumbs';
 import {generateReplayLink} from 'sentry/views/performance/transactionSummary/utils';
@@ -77,9 +78,7 @@ const inpSort: GridColumnSortBy<keyof InteractionSpanSampleRowWithScore> = {
 
 export function PageOverviewWebVitalsDetailPanel({
   webVital,
-  onClose,
 }: {
-  onClose: () => void;
   webVital: WebVitals | null;
 }) {
   const location = useLocation();
@@ -367,7 +366,9 @@ export function PageOverviewWebVitalsDetailPanel({
 
   return (
     <PageAlertProvider>
-      <DetailPanel detailKey={webVital ?? undefined} onClose={onClose}>
+      <DrawerHeader />
+
+      <SampleDrawerBody>
         {webVital && (
           <WebVitalDetailHeader
             value={
@@ -410,7 +411,7 @@ export function PageOverviewWebVitalsDetailPanel({
           )}
         </TableContainer>
         <PageAlert />
-      </DetailPanel>
+      </SampleDrawerBody>
     </PageAlertProvider>
   );
 }

diff --git a/static/app/views/insights/browser/webVitals/components/webVitalDescription.tsx b/static/app/views/insights/browser/webVitals/components/webVitalDescription.tsx
@@ -273,7 +273,6 @@ const Value = styled('h2')`
 
 const WebVitalName = styled('h4')`
   margin-bottom: ${space(1)};
-  margin-top: 40px;
   max-width: 400px;
   ${p => p.theme.overflowEllipsis}
 `;

diff --git a/static/app/views/insights/browser/webVitals/components/webVitalsDetailPanel.spec.tsx b/static/app/views/insights/browser/webVitals/components/webVitalsDetailPanel.spec.tsx
@@ -58,7 +58,7 @@ describe('WebVitalsDetailPanel', function () {
   });
 
   it('renders correctly with empty results', async () => {
-    render(<WebVitalsDetailPanel onClose={() => undefined} webVital="lcp" />, {
+    render(<WebVitalsDetailPanel webVital="lcp" />, {
       organization,
     });
     await waitForElementToBeRemoved(() => screen.queryByTestId('loading-indicator'));

diff --git a/static/app/views/insights/browser/webVitals/components/webVitalsDetailPanel.tsx b/static/app/views/insights/browser/webVitals/components/webVitalsDetailPanel.tsx
@@ -2,6 +2,7 @@ import {useEffect, useMemo} from 'react';
 import styled from '@emotion/styled';
 
 import type {LineChartSeries} from 'sentry/components/charts/lineChart';
+import {DrawerHeader} from 'sentry/components/globalDrawer/components';
 import type {
   GridColumnHeader,
   GridColumnOrder,
@@ -34,7 +35,7 @@ import type {
   WebVitals,
 } from 'sentry/views/insights/browser/webVitals/types';
 import decodeBrowserTypes from 'sentry/views/insights/browser/webVitals/utils/queryParameterDecoders/browserType';
-import DetailPanel from 'sentry/views/insights/common/components/detailPanel';
+import {SampleDrawerBody} from 'sentry/views/insights/common/components/sampleDrawerBody';
 import {SpanIndexedField, type SubregionCode} from 'sentry/views/insights/types';
 
 type Column = GridColumnHeader;
@@ -51,13 +52,7 @@ const sort: GridColumnSortBy<keyof Row> = {key: 'count()', order: 'desc'};
 
 const MAX_ROWS = 10;
 
-export function WebVitalsDetailPanel({
-  webVital,
-  onClose,
-}: {
-  onClose: () => void;
-  webVital: WebVitals | null;
-}) {
+export function WebVitalsDetailPanel({webVital}: {webVital: WebVitals | null}) {
   const location = useLocation();
   const organization = useOrganization();
   const browserTypes = decodeBrowserTypes(location.query[SpanIndexedField.BROWSER_NAME]);
@@ -133,8 +128,6 @@ export function WebVitalsDetailPanel({
     seriesName: webVital ?? '',
   };
 
-  const detailKey = webVital;
-
   useEffect(() => {
     if (webVital !== null) {
       trackAnalytics('insight.vital.vital_sidebar_opened', {
@@ -243,7 +236,9 @@ export function WebVitalsDetailPanel({
 
   return (
     <PageAlertProvider>
-      <DetailPanel detailKey={detailKey ?? undefined} onClose={onClose}>
+      <DrawerHeader />
+
+      <SampleDrawerBody>
         {webVital && (
           <WebVitalDescription
             value={
@@ -274,7 +269,7 @@ export function WebVitalsDetailPanel({
           />
         </TableContainer>
         <PageAlert />
-      </DetailPanel>
+      </SampleDrawerBody>
     </PageAlertProvider>
   );
 }

diff --git a/static/app/views/insights/browser/webVitals/views/pageOverview.tsx b/static/app/views/insights/browser/webVitals/views/pageOverview.tsx
@@ -1,4 +1,4 @@
-import React, {Fragment, useMemo, useState} from 'react';
+import React, {Fragment, useEffect, useMemo, useState} from 'react';
 import styled from '@emotion/styled';
 import omit from 'lodash/omit';
 
@@ -32,6 +32,7 @@ import {ModulePageFilterBar} from 'sentry/views/insights/common/components/modul
 import {ModulePageProviders} from 'sentry/views/insights/common/components/modulePageProviders';
 import {ModuleBodyUpsellHook} from 'sentry/views/insights/common/components/moduleUpsellHookWrapper';
 import {useModuleURL} from 'sentry/views/insights/common/utils/useModuleURL';
+import {useWebVitalsDrawer} from 'sentry/views/insights/common/utils/useWebVitalsDrawer';
 import {FrontendHeader} from 'sentry/views/insights/pages/frontend/frontendPageHeader';
 import {useDomainViewFilters} from 'sentry/views/insights/pages/useFilters';
 import {
@@ -106,6 +107,25 @@ export function PageOverview() {
   const {data: projectScores, isPending: isProjectScoresLoading} =
     useProjectWebVitalsScoresQuery({transaction, browserTypes, subregions});
 
+  const {openVitalsDrawer} = useWebVitalsDrawer({
+    Component: <PageOverviewWebVitalsDetailPanel webVital={state.webVital} />,
+    webVital: state.webVital,
+    onClose: () => {
+      router.replace({
+        pathname: router.location.pathname,
+        query: omit(router.location.query, 'webVital'),
+      });
+
+      setState({...state, webVital: null});
+    },
+  });
+
+  useEffect(() => {
+    if (state.webVital) {
+      openVitalsDrawer();
+    }
+  });
+
   if (transaction === undefined) {
     // redirect user to webvitals landing page
     window.location.href = moduleURL;
@@ -239,16 +259,6 @@ export function PageOverview() {
             </Layout.Body>
           )}
         </ModuleBodyUpsellHook>
-        <PageOverviewWebVitalsDetailPanel
-          webVital={state.webVital}
-          onClose={() => {
-            router.replace({
-              pathname: router.location.pathname,
-              query: omit(router.location.query, 'webVital'),
-            });
-            setState({...state, webVital: null});
-          }}
-        />
       </Tabs>
     </React.Fragment>
   );

diff --git a/static/app/views/insights/browser/webVitals/views/webVitalsLandingPage.tsx b/static/app/views/insights/browser/webVitals/views/webVitalsLandingPage.tsx
@@ -1,6 +1,5 @@
-import React, {Fragment, useState} from 'react';
+import React, {Fragment, useEffect, useState} from 'react';
 import styled from '@emotion/styled';
-import omit from 'lodash/omit';
 
 import Alert from 'sentry/components/alert';
 import {Button} from 'sentry/components/button';
@@ -12,7 +11,6 @@ import {t, tct} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import {decodeList} from 'sentry/utils/queryString';
 import {useLocation} from 'sentry/utils/useLocation';
-import useRouter from 'sentry/utils/useRouter';
 import BrowserTypeSelector from 'sentry/views/insights/browser/webVitals/components/browserTypeSelector';
 import {PerformanceScoreChart} from 'sentry/views/insights/browser/webVitals/components/charts/performanceScoreChart';
 import {PagePerformanceTable} from 'sentry/views/insights/browser/webVitals/components/tables/pagePerformanceTable';
@@ -32,6 +30,7 @@ import {ModulePageFilterBar} from 'sentry/views/insights/common/components/modul
 import {ModulePageProviders} from 'sentry/views/insights/common/components/modulePageProviders';
 import {ModulesOnboarding} from 'sentry/views/insights/common/components/modulesOnboarding';
 import {ModuleBodyUpsellHook} from 'sentry/views/insights/common/components/moduleUpsellHookWrapper';
+import {useWebVitalsDrawer} from 'sentry/views/insights/common/utils/useWebVitalsDrawer';
 import {FrontendHeader} from 'sentry/views/insights/pages/frontend/frontendPageHeader';
 import {
   ModuleName,
@@ -44,8 +43,6 @@ const WEB_VITALS_COUNT = 5;
 export function WebVitalsLandingPage() {
   const location = useLocation();
 
-  const router = useRouter();
-
   const [state, setState] = useState<{webVital: WebVitals | null}>({
     webVital: (location.query.webVital as WebVitals) ?? null,
   });
@@ -67,6 +64,20 @@ export function WebVitalsLandingPage() {
       ? undefined
       : getWebVitalScoresFromTableDataRow(projectScores?.data?.[0]);
 
+  const {openVitalsDrawer} = useWebVitalsDrawer({
+    Component: <WebVitalsDetailPanel webVital={state.webVital} />,
+    webVital: state.webVital,
+    onClose: () => {
+      setState({webVital: null});
+    },
+  });
+
+  useEffect(() => {
+    if (state.webVital) {
+      openVitalsDrawer();
+    }
+  });
+
   return (
     <React.Fragment>
       <FrontendHeader
@@ -147,16 +158,6 @@ export function WebVitalsLandingPage() {
           </Layout.Main>
         </Layout.Body>
       </ModuleBodyUpsellHook>
-      <WebVitalsDetailPanel
-        webVital={state.webVital}
-        onClose={() => {
-          router.replace({
-            pathname: router.location.pathname,
-            query: omit(router.location.query, 'webVital'),
-          });
-          setState({...state, webVital: null});
-        }}
-      />
     </React.Fragment>
   );
 }