feat(insights): Use GlobalDrawer instead of details panel II (#83301)

Attempt #2. The [previous PR](#82534) had to be reverted because I didn't account for URL behaviour. This PR is very similar, but now opening and closing the span samples drawer is done in a special hook that watches for the required URL parameters. --- 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 **Before:** <img width="829" alt="Screenshot 2024-12-23 at 11 42 33 AM" src="https://github.com/user-attachments/assets/78e3631b-7694-42eb-b3f0-149f92307748" /> <img width="817" alt="Screenshot 2024-12-23 at 11 42 44 AM" src="https://github.com/user-attachments/assets/f24b2a0e-3adb-414d-b965-eeb5715f7a9e" /> <img width="838" alt="Screenshot 2024-12-23 at 11 43 07 AM" src="https://github.com/user-attachments/assets/9753b6da-a6fb-464b-bc3e-25f5126fa4e5" /> **After:** <img width="767" alt="Screenshot 2024-12-23 at 11 43 19 AM" src="https://github.com/user-attachments/assets/b500af97-7493-4f2f-9330-6190e31eaac1" /> <img width="774" alt="Screenshot 2024-12-23 at 11 43 55 AM" src="https://github.com/user-attachments/assets/332569ad-b715-4365-b000-188b64747944" /> <img width="563" alt="Screenshot 2024-12-23 at 11 44 44 AM" src="https://github.com/user-attachments/assets/338ec721-471e-4760-b028-6df850df0e8e" /> <img width="483" alt="Screenshot 2024-12-23 at 12 04 50 PM" src="https://github.com/user-attachments/assets/db20d3a8-9958-4993-8ea0-6781e1467a80" /> ## 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 passed as parameters to the hooks - 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 · 83d855c · 83d855c
1 parent d159d50
commit 83d855c
Show file tree

Hide file tree

Showing 30 changed files with 620 additions and 582 deletions.
diff --git a/static/app/views/insights/browser/resources/views/resourceSummaryPage.tsx b/static/app/views/insights/browser/resources/views/resourceSummaryPage.tsx
@@ -22,6 +22,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';
@@ -43,6 +44,20 @@ function ResourceSummary() {
   const {groupId} = useParams();
   const filters = useResourceModuleFilters();
   const selectedSpanOp = filters[SPAN_OP];
+
+  useSamplesDrawer({
+    Component: (
+      <SampleList
+        groupId={groupId!}
+        moduleName={ModuleName.RESOURCE}
+        transactionRoute={webVitalsModuleURL}
+        referrer={TraceViewSources.ASSETS_MODULE}
+      />
+    ),
+    moduleName: ModuleName.RESOURCE,
+    requiredParams: ['transaction'],
+  });
+
   const {data, isPending} = useSpanMetrics(
     {
       search: MutableSearch.fromQueryObject({
@@ -135,15 +150,6 @@ function ResourceSummary() {
               <ModuleLayout.Full>
                 <ResourceSummaryTable />
               </ModuleLayout.Full>
-
-              <ModuleLayout.Full>
-                <SampleList
-                  groupId={groupId!}
-                  moduleName={ModuleName.RESOURCE}
-                  transactionRoute={webVitalsModuleURL}
-                  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';
@@ -11,7 +10,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';
@@ -26,6 +24,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,
@@ -38,8 +37,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,
   });
@@ -61,6 +58,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 module={ModuleName.VITAL} />
@@ -130,16 +141,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>
   );
 }