fix(insights): Closing mobile span details panel breaks screen (#83571)

By default the `useSamplesDrawer()` removes the `transaction` query parameter when being closed. This unfortunately doesn't work well for mobile screens detail pages as the underlying screen already operates on the `transaction` query param (e.g. `/insights/mobile/mobile-screens/details?tab=app_start&transaction=TransactionActivity`). Once the panel is closed the underlying screen looses it's state / turns into an empty state: https://github.com/user-attachments/assets/db2c8fb3-6615-4752-a1fe-23ee4ac02011 This PR fixes this by providing a way to override the default close behavior. It also fixes the panel links when the Mobile Screens feature is enabled.
getsentry · Jan 16, 2025 · e7bf296 · e7bf296
1 parent 8d4ef7a
commit e7bf296
Show file tree

Hide file tree

Showing 7 changed files with 90 additions and 80 deletions.
diff --git a/static/app/views/insights/common/utils/useSamplesDrawer.tsx b/static/app/views/insights/common/utils/useSamplesDrawer.tsx
@@ -15,36 +15,42 @@ interface UseSamplesDrawerProps {
   Component: React.ReactNode;
   moduleName: ModuleName;
   requiredParams: [string, ...string[]];
+  onClose?: () => void;
 }
 
 export function useSamplesDrawer({
   Component,
   moduleName,
   requiredParams,
+  onClose = () => undefined,
 }: UseSamplesDrawerProps): void {
   const organization = useOrganization();
   const {openDrawer, closeDrawer, isDrawerOpen} = useDrawer();
   const navigate = useNavigate();
   const location = useLocation();
 
-  const onClose = useCallback(() => {
-    navigate({
-      query: {
-        ...location.query,
-        transaction: undefined,
-        transactionMethod: undefined,
-        spanGroup: undefined,
-        spanOp: undefined,
-        query: undefined,
-        responseCodeClass: undefined,
-        panel: undefined,
-        statusClass: undefined,
-        spanSearchQuery: undefined,
-        traceStatus: undefined,
-        retryCount: undefined,
-      },
-    });
-  }, [navigate, location.query]);
+  const onCloseAction = useCallback(() => {
+    if (onClose) {
+      onClose();
+    } else {
+      navigate({
+        query: {
+          ...location.query,
+          transaction: undefined,
+          transactionMethod: undefined,
+          spanGroup: undefined,
+          spanOp: undefined,
+          query: undefined,
+          responseCodeClass: undefined,
+          panel: undefined,
+          statusClass: undefined,
+          spanSearchQuery: undefined,
+          traceStatus: undefined,
+          retryCount: undefined,
+        },
+      });
+    }
+  }, [navigate, onClose, location.query]);
 
   const shouldCloseOnLocationChange = useCallback(
     (newLocation: Location) => {
@@ -65,15 +71,15 @@ export function useSamplesDrawer({
 
     openDrawer(() => <FullHeightWrapper>{Component}</FullHeightWrapper>, {
       ariaLabel: t('Samples'),
-      onClose,
+      onClose: onCloseAction,
       transitionProps: {stiffness: 1000},
       shouldCloseOnLocationChange,
       shouldCloseOnInteractOutside: () => false,
     });
   }, [
     openDrawer,
     isDrawerOpen,
-    onClose,
+    onCloseAction,
     shouldCloseOnLocationChange,
     Component,
     organization,

diff --git a/static/app/views/insights/mobile/appStarts/components/tables/spanOperationTable.tsx b/static/app/views/insights/mobile/appStarts/components/tables/spanOperationTable.tsx
@@ -42,7 +42,12 @@ import {
 import useCrossPlatformProject from 'sentry/views/insights/mobile/common/queries/useCrossPlatformProject';
 import {useTableQuery} from 'sentry/views/insights/mobile/screenload/components/tables/screensTable';
 import {MobileCursors} from 'sentry/views/insights/mobile/screenload/constants';
-import {SpanMetricsField, type SubregionCode} from 'sentry/views/insights/types';
+import {isModuleEnabled} from 'sentry/views/insights/pages/utils';
+import {
+  ModuleName,
+  SpanMetricsField,
+  type SubregionCode,
+} from 'sentry/views/insights/types';
 
 const {SPAN_SELF_TIME, SPAN_DESCRIPTION, SPAN_GROUP, SPAN_OP, PROJECT_ID} =
   SpanMetricsField;
@@ -58,11 +63,18 @@ export function SpanOperationTable({
   primaryRelease,
   secondaryRelease,
 }: Props) {
-  const moduleURL = useModuleURL('app_start');
+  const organization = useOrganization();
+  const isMobileScreensEnabled = isModuleEnabled(ModuleName.MOBILE_SCREENS, organization);
+  const moduleURL = useModuleURL(
+    isMobileScreensEnabled ? ModuleName.MOBILE_SCREENS : ModuleName.APP_START
+  );
+  const baseURL = isMobileScreensEnabled
+    ? `${moduleURL}/details/`
+    : `${moduleURL}/spans/`;
+
   const navigate = useNavigate();
   const location = useLocation();
   const {selection} = usePageFilters();
-  const organization = useOrganization();
   const {isProjectCrossPlatform, selectedPlatform} = useCrossPlatformProject();
   const cursor = decodeScalar(location.query?.[MobileCursors.SPANS_TABLE]);
 
@@ -162,7 +174,6 @@ export function SpanOperationTable({
     if (column.key === SPAN_DESCRIPTION) {
       const label = row[SpanMetricsField.SPAN_DESCRIPTION];
 
-      const pathname = `${moduleURL}/spans/`;
       const query = {
         ...location.query,
         transaction,
@@ -174,7 +185,7 @@ export function SpanOperationTable({
 
       return (
         <OverflowEllipsisTextContainer>
-          <Link to={`${pathname}?${qs.stringify(query)}`}>{label}</Link>
+          <Link to={`${baseURL}?${qs.stringify(query)}`}>{label}</Link>
         </OverflowEllipsisTextContainer>
       );
     }

diff --git a/static/app/views/insights/mobile/appStarts/views/screenSummaryPage.tsx b/static/app/views/insights/mobile/appStarts/views/screenSummaryPage.tsx
@@ -109,34 +109,29 @@ export function ScreenSummaryContentPage() {
   }, [location, appStartType, navigate]);
 
   useSamplesDrawer({
-    Component: (
-      <SpanSamplesPanel
-        groupId={spanGroup}
-        moduleName={ModuleName.APP_START}
-        onClose={() => {
-          navigate(
-            {
-              pathname: location.pathname,
-              query: omit(
-                location.query,
-                'spanGroup',
-                'transactionMethod',
-                'spanDescription',
-                'spanOp'
-              ),
-            },
-            {replace: true}
-          );
-        }}
-      />
-    ),
+    Component: <SpanSamplesPanel groupId={spanGroup} moduleName={ModuleName.APP_START} />,
     moduleName: ModuleName.APP_START,
     requiredParams: [
       'transaction',
       'spanGroup',
       'spanOp',
       SpanMetricsField.APP_START_TYPE,
     ],
+    onClose: () => {
+      navigate(
+        {
+          pathname: location.pathname,
+          query: omit(
+            location.query,
+            'spanGroup',
+            'transactionMethod',
+            'spanDescription',
+            'spanOp'
+          ),
+        },
+        {replace: true}
+      );
+    },
   });
 
   return (

diff --git a/static/app/views/insights/mobile/common/components/spanSamplesPanel.tsx b/static/app/views/insights/mobile/common/components/spanSamplesPanel.tsx
@@ -23,7 +23,6 @@ import {getTransactionSummaryBaseUrl} from 'sentry/views/performance/transaction
 type Props = {
   groupId: string;
   moduleName: ModuleName;
-  onClose?: () => void;
   transactionRoute?: string;
 };
 

diff --git a/static/app/views/insights/mobile/screenload/components/tables/screenLoadSpansTable.tsx b/static/app/views/insights/mobile/screenload/components/tables/screenLoadSpansTable.tsx
@@ -44,7 +44,8 @@ import {
 import {useTableQuery} from 'sentry/views/insights/mobile/screenload/components/tables/screensTable';
 import {MobileCursors} from 'sentry/views/insights/mobile/screenload/constants';
 import {MODULE_DOC_LINK} from 'sentry/views/insights/mobile/screenload/settings';
-import {SpanMetricsField} from 'sentry/views/insights/types';
+import {isModuleEnabled} from 'sentry/views/insights/pages/utils';
+import {ModuleName, SpanMetricsField} from 'sentry/views/insights/types';
 
 const {SPAN_SELF_TIME, SPAN_DESCRIPTION, SPAN_GROUP, SPAN_OP, PROJECT_ID} =
   SpanMetricsField;
@@ -60,11 +61,18 @@ export function ScreenLoadSpansTable({
   primaryRelease,
   secondaryRelease,
 }: Props) {
-  const moduleURL = useModuleURL('screen_load');
+  const organization = useOrganization();
+  const isMobileScreensEnabled = isModuleEnabled(ModuleName.MOBILE_SCREENS, organization);
+  const moduleURL = useModuleURL(
+    isMobileScreensEnabled ? ModuleName.MOBILE_SCREENS : ModuleName.SCREEN_LOAD
+  );
+  const baseURL = isMobileScreensEnabled
+    ? `${moduleURL}/details/`
+    : `${moduleURL}/spans/`;
+
   const navigate = useNavigate();
   const location = useLocation();
   const {selection} = usePageFilters();
-  const organization = useOrganization();
   const cursor = decodeScalar(location.query?.[MobileCursors.SPANS_TABLE]);
   const {isProjectCrossPlatform, selectedPlatform} = useCrossPlatformProject();
 
@@ -158,7 +166,6 @@ export function ScreenLoadSpansTable({
     if (column.key === SPAN_DESCRIPTION) {
       const label = row[SpanMetricsField.SPAN_DESCRIPTION];
 
-      const pathname = `${moduleURL}/spans/`;
       const query = {
         ...location.query,
         transaction,
@@ -168,7 +175,7 @@ export function ScreenLoadSpansTable({
 
       return (
         <OverflowEllipsisTextContainer>
-          <Link to={`${pathname}?${qs.stringify(query)}`}>{label}</Link>
+          <Link to={`${baseURL}?${qs.stringify(query)}`}>{label}</Link>
         </OverflowEllipsisTextContainer>
       );
     }

diff --git a/static/app/views/insights/mobile/screenload/views/screenLoadSpansPage.tsx b/static/app/views/insights/mobile/screenload/views/screenLoadSpansPage.tsx
@@ -91,19 +91,16 @@ export function ScreenLoadSpansContent() {
 
   useSamplesDrawer({
     Component: (
-      <SpanSamplesPanel
-        groupId={spanGroup}
-        moduleName={ModuleName.SCREEN_LOAD}
-        onClose={() => {
-          router.replace({
-            pathname: router.location.pathname,
-            query: omit(router.location.query, 'spanGroup', 'transactionMethod'),
-          });
-        }}
-      />
+      <SpanSamplesPanel groupId={spanGroup} moduleName={ModuleName.SCREEN_LOAD} />
     ),
     moduleName: ModuleName.SCREEN_LOAD,
     requiredParams: ['transaction', 'spanGroup'],
+    onClose: () => {
+      router.replace({
+        pathname: router.location.pathname,
+        query: omit(router.location.query, 'spanGroup', 'transactionMethod'),
+      });
+    },
   });
 
   return (

diff --git a/static/app/views/insights/mobile/ui/views/screenSummaryPage.tsx b/static/app/views/insights/mobile/ui/views/screenSummaryPage.tsx
@@ -71,26 +71,21 @@ export function ScreenSummaryContent() {
   const {transaction: transactionName, spanGroup} = location.query;
 
   useSamplesDrawer({
-    Component: (
-      <SpanSamplesPanel
-        groupId={spanGroup}
-        moduleName={ModuleName.OTHER}
-        onClose={() => {
-          router.replace({
-            pathname: router.location.pathname,
-            query: omit(
-              router.location.query,
-              'spanGroup',
-              'transactionMethod',
-              'spanDescription',
-              'spanOp'
-            ),
-          });
-        }}
-      />
-    ),
+    Component: <SpanSamplesPanel groupId={spanGroup} moduleName={ModuleName.OTHER} />,
     moduleName: ModuleName.OTHER,
     requiredParams: ['spanGroup', 'spanOp'],
+    onClose: () => {
+      router.replace({
+        pathname: router.location.pathname,
+        query: omit(
+          router.location.query,
+          'spanGroup',
+          'transactionMethod',
+          'spanDescription',
+          'spanOp'
+        ),
+      });
+    },
   });
 
   return (