Repository: getsentry/sentry
Pull request: ref(replays): tags is default tab for mobile replays
For mobile replays, all tabs except Tags are disabled, with a tooltip saying coming soon:
<img width="709" alt="SCR-20240327-jyko-2" src="https://github.com/getsentry/sentry/assets/56095982/711797bf-59a9-4010-9602-f87ca58105c6">


Attempting to route to some other `t_main` will result in the tags tab being shown by default:
<img width="1264" alt="SCR-20240327-jyha" src="https://github.com/getsentry/sentry/assets/56095982/59b61b1c-aae3-46ce-948d-95f526283dfc">

Closes https://github.com/getsentry/sentry/issues/67692

Problem statement:
[Replay Details] Make "Tags" tab default for mobile replays
We do not plan on having any additional functionality to the initial alpha release of mobile replays (aside from the playback), but we will have tags available, so let's show that tab by default. The other tabs should be disabled with a tooltip on hover explaining why.

Hints from issue discussion:

Commit message: add and fix tests

Patch to review:
diff --git a/static/app/components/links/listLink.tsx b/static/app/components/links/listLink.tsx
index 56733641b39606..86049fed7ce4fa 100644
--- a/static/app/components/links/listLink.tsx
+++ b/static/app/components/links/listLink.tsx
@@ -4,6 +4,7 @@ import classNames from 'classnames';
 import type {LocationDescriptor} from 'history';
 import * as qs from 'query-string';
 
+import {Tooltip} from 'sentry/components/tooltip';
 import useRouter from 'sentry/utils/useRouter';
 import {normalizeUrl} from 'sentry/utils/withDomainRequired';
 
@@ -19,6 +20,10 @@ type Props = LinkProps & {
    */
   activeClassName?: string;
   disabled?: boolean;
+  /**
+   * Tooltip to display if the link is disabled
+   */
+  disabledTooltip?: string;
   index?: boolean;
   /**
    * Should be should be supplied by the parent component
@@ -36,6 +41,7 @@ function ListLink({
   activeClassName = 'active',
   index = false,
   disabled = false,
+  disabledTooltip,
   ...props
 }: Props) {
   const router = useRouter();
@@ -50,9 +56,11 @@ function ListLink({
       className={classNames({[activeClassName]: active}, className)}
       disabled={disabled}
     >
-      <RouterLink {...props} onlyActiveOnIndex={index} to={disabled ? '' : target}>
-        {children}
-      </RouterLink>
+      <Tooltip title={disabled ? disabledTooltip : null}>
+        <RouterLink {...props} onlyActiveOnIndex={index} to={disabled ? '' : target}>
+          {children}
+        </RouterLink>
+      </Tooltip>
     </StyledLi>
   );
 }
diff --git a/static/app/components/replays/breadcrumbs/replayTimelineEvents.tsx b/static/app/components/replays/breadcrumbs/replayTimelineEvents.tsx
index 5fdffd3367fb84..e912cee004b201 100644
--- a/static/app/components/replays/breadcrumbs/replayTimelineEvents.tsx
+++ b/static/app/components/replays/breadcrumbs/replayTimelineEvents.tsx
@@ -74,7 +74,7 @@ function Event({
 }) {
   const theme = useTheme();
   const {onMouseEnter, onMouseLeave, onClickTimestamp} = useCrumbHandlers();
-  const {setActiveTab} = useActiveReplayTab();
+  const {setActiveTab} = useActiveReplayTab({});
 
   const buttons = frames.map((frame, i) => (
     <BreadcrumbItem
diff --git a/static/app/utils/replays/hooks/useActiveReplayTab.spec.tsx b/static/app/utils/replays/hooks/useActiveReplayTab.spec.tsx
index 280128a2d88ff6..7b209692798bde 100644
--- a/static/app/utils/replays/hooks/useActiveReplayTab.spec.tsx
+++ b/static/app/utils/replays/hooks/useActiveReplayTab.spec.tsx
@@ -39,21 +39,35 @@ describe('useActiveReplayTab', () => {
   });
 
   it('should use Breadcrumbs as a default', () => {
-    const {result} = reactHooks.renderHook(useActiveReplayTab);
+    const {result} = reactHooks.renderHook(useActiveReplayTab, {
+      initialProps: {},
+    });
 
     expect(result.current.getActiveTab()).toBe(TabKey.BREADCRUMBS);
   });
 
+  it('should use Tags as a default for video replays', () => {
+    const {result} = reactHooks.renderHook(useActiveReplayTab, {
+      initialProps: {isVideoReplay: true},
+    });
+
+    expect(result.current.getActiveTab()).toBe(TabKey.TAGS);
+  });
+
   it('should use Breadcrumbs as a default, when there is a click search in the url', () => {
     mockLocation('click.tag:button');
 
-    const {result} = reactHooks.renderHook(useActiveReplayTab);
+    const {result} = reactHooks.renderHook(useActiveReplayTab, {
+      initialProps: {},
+    });
 
     expect(result.current.getActiveTab()).toBe(TabKey.BREADCRUMBS);
   });
 
   it('should allow case-insensitive tab names', () => {
-    const {result} = reactHooks.renderHook(useActiveReplayTab);
+    const {result} = reactHooks.renderHook(useActiveReplayTab, {
+      initialProps: {},
+    });
     expect(result.current.getActiveTab()).toBe(TabKey.BREADCRUMBS);
 
     result.current.setActiveTab('nEtWoRk');
@@ -64,7 +78,9 @@ describe('useActiveReplayTab', () => {
   });
 
   it('should set the default tab if the name is invalid', () => {
-    const {result} = reactHooks.renderHook(useActiveReplayTab);
+    const {result} = reactHooks.renderHook(useActiveReplayTab, {
+      initialProps: {},
+    });
     expect(result.current.getActiveTab()).toBe(TabKey.BREADCRUMBS);
 
     result.current.setActiveTab('foo bar');
@@ -74,12 +90,27 @@ describe('useActiveReplayTab', () => {
     });
   });
 
+  it('should set the default tab if the name is invalid for video replays', () => {
+    const {result} = reactHooks.renderHook(useActiveReplayTab, {
+      initialProps: {isVideoReplay: true},
+    });
+    expect(result.current.getActiveTab()).toBe(TabKey.TAGS);
+
+    result.current.setActiveTab('foo bar');
+    expect(mockPush).toHaveBeenLastCalledWith({
+      pathname: '',
+      query: {t_main: TabKey.TAGS},
+    });
+  });
+
   it('should disallow PERF by default', () => {
     mockOrganizationFixture({
       features: [],
     });
 
-    const {result} = reactHooks.renderHook(useActiveReplayTab);
+    const {result} = reactHooks.renderHook(useActiveReplayTab, {
+      initialProps: {},
+    });
     expect(result.current.getActiveTab()).toBe(TabKey.BREADCRUMBS);
 
     result.current.setActiveTab(TabKey.PERF);
@@ -93,7 +124,9 @@ describe('useActiveReplayTab', () => {
     mockOrganizationFixture({
       features: ['session-replay-trace-table'],
     });
-    const {result} = reactHooks.renderHook(useActiveReplayTab);
+    const {result} = reactHooks.renderHook(useActiveReplayTab, {
+      initialProps: {},
+    });
     expect(result.current.getActiveTab()).toBe(TabKey.BREADCRUMBS);
 
     result.current.setActiveTab(TabKey.PERF);
diff --git a/static/app/utils/replays/hooks/useActiveReplayTab.tsx b/static/app/utils/replays/hooks/useActiveReplayTab.tsx
index 76950784f812f5..8b881e4bfc0736 100644
--- a/static/app/utils/replays/hooks/useActiveReplayTab.tsx
+++ b/static/app/utils/replays/hooks/useActiveReplayTab.tsx
@@ -26,8 +26,8 @@ function isReplayTab(tab: string, organization: Organization): tab is TabKey {
   return Object.values<string>(TabKey).includes(tab);
 }
 
-function useActiveReplayTab() {
-  const defaultTab = TabKey.BREADCRUMBS;
+function useActiveReplayTab({isVideoReplay}: {isVideoReplay?: boolean}) {
+  const defaultTab = isVideoReplay ? TabKey.TAGS : TabKey.BREADCRUMBS;
   const organization = useOrganization();
   const {getParamValue, setParamValue} = useUrlParams('t_main', defaultTab);
 
diff --git a/static/app/views/replays/detail/layout/focusArea.tsx b/static/app/views/replays/detail/layout/focusArea.tsx
index 7e3ee0edc3527f..a5e4b295373865 100644
--- a/static/app/views/replays/detail/layout/focusArea.tsx
+++ b/static/app/views/replays/detail/layout/focusArea.tsx
@@ -9,8 +9,12 @@ import PerfTable from 'sentry/views/replays/detail/perfTable/index';
 import TagPanel from 'sentry/views/replays/detail/tagPanel';
 import Trace from 'sentry/views/replays/detail/trace/index';
 
-export default function FocusArea() {
-  const {getActiveTab} = useActiveReplayTab();
+export default function FocusArea({isVideoReplay}: {isVideoReplay?: boolean}) {
+  const {getActiveTab} = useActiveReplayTab({isVideoReplay});
+
+  if (isVideoReplay) {
+    return <TagPanel />;
+  }
 
   switch (getActiveTab()) {
     case TabKey.A11Y:
diff --git a/static/app/views/replays/detail/layout/focusTabs.tsx b/static/app/views/replays/detail/layout/focusTabs.tsx
index 22309916904d52..00de0794b8f8b0 100644
--- a/static/app/views/replays/detail/layout/focusTabs.tsx
+++ b/static/app/views/replays/detail/layout/focusTabs.tsx
@@ -68,17 +68,24 @@ type Props = {
 function FocusTabs({className, isVideoReplay}: Props) {
   const organization = useOrganization();
   const {pathname, query} = useLocation();
-  const {getActiveTab, setActiveTab} = useActiveReplayTab();
+  const {getActiveTab, setActiveTab} = useActiveReplayTab({isVideoReplay});
   const activeTab = getActiveTab();
+  const supportedVideoTabs = [TabKey.TAGS];
+
+  const unsupportedVideoTab = tab => {
+    return isVideoReplay && !supportedVideoTabs.includes(tab);
+  };
 
   return (
     <ScrollableTabs className={className} underlined>
       {Object.entries(getReplayTabs({organization, isVideoReplay})).map(([tab, label]) =>
         label ? (
           <ListLink
+            disabledTooltip={t('This feature is coming soon')}
+            disabled={unsupportedVideoTab(tab)}
             data-test-id={`replay-details-${tab}-btn`}
             key={tab}
-            isActive={() => tab === activeTab}
+            isActive={() => (unsupportedVideoTab(tab) ? false : tab === activeTab)}
             to={`${pathname}?${queryString.stringify({...query, t_main: tab})}`}
             onClick={e => {
               e.preventDefault();
diff --git a/static/app/views/replays/detail/layout/index.tsx b/static/app/views/replays/detail/layout/index.tsx
index b5a449a5d8f6cc..ec264e518303be 100644
--- a/static/app/views/replays/detail/layout/index.tsx
+++ b/static/app/views/replays/detail/layout/index.tsx
@@ -62,7 +62,7 @@ function ReplayLayout({isVideoReplay = false}: {isVideoReplay?: boolean}) {
   const focusArea = (
     <FluidPanel title={<SmallMarginFocusTabs isVideoReplay={isVideoReplay} />}>
       <ErrorBoundary mini>
-        <FocusArea />
+        <FocusArea isVideoReplay={isVideoReplay} />
       </ErrorBoundary>
     </FluidPanel>
   );


Code review comments:
diff_hunk: @@ -50,9 +56,11 @@ function ListLink({
       className={classNames({[activeClassName]: active}, className)}
       disabled={disabled}
     >
-      <RouterLink {...props} onlyActiveOnIndex={index} to={disabled ? '' : target}>
-        {children}
-      </RouterLink>
+      <Tooltip title={disabled ? disabledTooltip : null}>
path: static/app/components/links/listLink.tsx
text: @user1:
Can we move this to `focusTabs` and wrap the label with Tooltip? Otherwise ListLink can become bloated with the number of props (imagine if we needed to customize the props on Tooltip).

@author:
done!