From a80842ee7415b56bb60258d04f0fa65fe02896b8 Mon Sep 17 00:00:00 2001 From: Michelle Zhang <56095982+michellewzhang@users.noreply.github.com> Date: Wed, 27 Mar 2024 14:02:16 -0700 Subject: [PATCH] ref(replays): tags is default tab for mobile replays (#67802) For mobile replays, all tabs except Tags are disabled, with a tooltip saying coming soon: SCR-20240327-jyko-2 Attempting to route to some other `t_main` will result in the tags tab being shown by default: SCR-20240327-jyha Closes https://github.com/getsentry/sentry/issues/67692 --- static/app/components/links/listLink.tsx | 16 ++++--- .../breadcrumbs/replayTimelineEvents.tsx | 2 +- .../replays/hooks/useActiveReplayTab.spec.tsx | 45 ++++++++++++++++--- .../replays/hooks/useActiveReplayTab.tsx | 4 +- .../views/replays/detail/layout/focusArea.tsx | 8 +++- .../views/replays/detail/layout/focusTabs.tsx | 16 +++++-- .../app/views/replays/detail/layout/index.tsx | 2 +- 7 files changed, 72 insertions(+), 21 deletions(-) diff --git a/static/app/components/links/listLink.tsx b/static/app/components/links/listLink.tsx index 56733641b39606..e28c35c4e962f9 100644 --- a/static/app/components/links/listLink.tsx +++ b/static/app/components/links/listLink.tsx @@ -65,12 +65,16 @@ const StyledLi = styled('li', { ${p => p.disabled && ` - a { - color:${p.theme.disabled} !important; - pointer-events: none; - :hover { - color: ${p.theme.disabled} !important; + a { + color:${p.theme.disabled} !important; + :hover { + color: ${p.theme.disabled} !important; + } + cursor: default !important; } - } + + a:active { + pointer-events: none; + } `} `; 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) => ( { }); 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(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 ; + } 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..7f0af3a028278a 100644 --- a/static/app/views/replays/detail/layout/focusTabs.tsx +++ b/static/app/views/replays/detail/layout/focusTabs.tsx @@ -68,17 +68,23 @@ 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 ( {Object.entries(getReplayTabs({organization, isVideoReplay})).map(([tab, label]) => label ? ( tab === activeTab} + isActive={() => (unsupportedVideoTab(tab) ? false : tab === activeTab)} to={`${pathname}?${queryString.stringify({...query, t_main: tab})}`} onClick={e => { e.preventDefault(); @@ -90,7 +96,11 @@ function FocusTabs({className, isVideoReplay}: Props) { }); }} > - {label} + + {label} + ) : null )} 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 = ( }> - + );