Browse Source

ref(replays): remove StringWalker from index and details (#57304)

Closes  https://github.com/getsentry/sentry/issues/56868

<img width="1111" alt="SCR-20231002-kqkn"
src="https://github.com/getsentry/sentry/assets/56095982/20437509-0279-4c51-a19e-35b18d455670">
<img width="1136" alt="SCR-20231002-kqmm"
src="https://github.com/getsentry/sentry/assets/56095982/5a033560-f6b3-4954-a1cd-f387d4e33595">
Michelle Zhang 1 year ago
parent
commit
3adb7291f9

+ 0 - 62
static/app/components/replays/walker/chevronDividedList.spec.tsx

@@ -1,62 +0,0 @@
-import {render, screen} from 'sentry-test/reactTestingLibrary';
-
-import ChevronDividedList from './chevronDividedList';
-
-describe('ChevronDividedList', () => {
-  it('should accept zero items and show an empty <List>', () => {
-    const mockItems = [];
-    render(<ChevronDividedList items={mockItems} />);
-
-    expect(screen.queryByRole('listitem')).not.toBeInTheDocument();
-    expect(screen.queryByRole('separator')).not.toBeInTheDocument();
-  });
-
-  it('should accept one item and show it in the <List>', () => {
-    const mockItems = [<span key="1">first</span>];
-    render(<ChevronDividedList items={mockItems} />);
-
-    expect(screen.queryByRole('listitem')).toBeInTheDocument();
-    expect(screen.queryByRole('separator')).not.toBeInTheDocument();
-    expect(screen.getByText('first')).toBeInTheDocument();
-  });
-
-  it('should accept two items and show them both in the <List>', () => {
-    const mockItems = [<span key="1">first</span>, <span key="2">second</span>];
-    render(<ChevronDividedList items={mockItems} />);
-
-    expect(screen.queryAllByRole('listitem')).toHaveLength(2);
-    expect(screen.queryByRole('separator')).toBeInTheDocument();
-    expect(screen.getByText('first')).toBeInTheDocument();
-    expect(screen.getByText('second')).toBeInTheDocument();
-  });
-
-  it('should accept three items and show them all in the <List>', () => {
-    const mockItems = [
-      <span key="1">first</span>,
-      <span key="2">second</span>,
-      <span key="3">third</span>,
-    ];
-    render(<ChevronDividedList items={mockItems} />);
-
-    expect(screen.queryAllByRole('listitem')).toHaveLength(3);
-    expect(screen.queryAllByRole('separator')).toHaveLength(2);
-    expect(screen.getByText('first')).toBeInTheDocument();
-    expect(screen.getByText('second')).toBeInTheDocument();
-    expect(screen.getByText('third')).toBeInTheDocument();
-  });
-
-  it('should accept many items and show them all in the <List>', () => {
-    const mockItems = [
-      <span key="1">first</span>,
-      <span key="2">second</span>,
-      <span key="3">third</span>,
-      <span key="4">fourth</span>,
-      <span key="5">fifth</span>,
-      <span key="6">sixth</span>,
-    ];
-    render(<ChevronDividedList items={mockItems} />);
-
-    expect(screen.queryAllByRole('listitem')).toHaveLength(6);
-    expect(screen.queryAllByRole('separator')).toHaveLength(5);
-  });
-});

+ 0 - 60
static/app/components/replays/walker/chevronDividedList.tsx

@@ -1,60 +0,0 @@
-import {ReactElement} from 'react';
-import styled from '@emotion/styled';
-
-import {IconChevron} from 'sentry/icons';
-import {space} from 'sentry/styles/space';
-
-type Props = {
-  items: ReactElement[];
-};
-
-function ChevronDividedList({items}: Props) {
-  return (
-    <List>
-      {items.flatMap((item, i) => {
-        const li = <Item key={`${i}-item`}>{item}</Item>;
-
-        return i === 0
-          ? li
-          : [
-              <Item key={`${i}-chev`} role="separator">
-                <Chevron>
-                  <IconChevron color="gray500" size="xs" direction="right" />
-                </Chevron>
-              </Item>,
-              li,
-            ];
-      })}
-    </List>
-  );
-}
-
-const List = styled('ul')`
-  padding: 0;
-  margin: 0;
-  list-style: none;
-  display: flex;
-  gap: ${space(0.75)};
-  flex-wrap: nowrap;
-  align-items: center;
-  overflow: hidden;
-`;
-
-const Item = styled('li')`
-  display: flex;
-  flex-direction: column;
-  overflow: hidden;
-  flex-shrink: 0;
-
-  &:first-child,
-  &:last-child {
-    flex-shrink: 1;
-  }
-`;
-
-const Chevron = styled('span')`
-  font-size: ${p => p.theme.fontSizeMedium};
-  line-height: 1;
-`;
-
-export default ChevronDividedList;

+ 0 - 55
static/app/components/replays/walker/frameWalker.spec.tsx

@@ -1,55 +0,0 @@
-import {render, screen} from 'sentry-test/reactTestingLibrary';
-
-import FrameWalker from 'sentry/components/replays/walker/frameWalker';
-import hydrateBreadcrumbs, {
-  replayInitBreadcrumb,
-} from 'sentry/utils/replays/hydrateBreadcrumbs';
-
-describe('FrameWalker', () => {
-  const replayRecord = TestStubs.ReplayRecord({});
-
-  it('should accept a list of crumbs and render a <ChevronDividedList />', () => {
-    const init = replayInitBreadcrumb(replayRecord);
-    const breadcrumbs = hydrateBreadcrumbs(replayRecord, [
-      TestStubs.Replay.NavFrame({
-        timestamp: new Date(),
-        data: {
-          from: '/',
-          to: '/report/1655300817078_https%3A%2F%2Fmaxcdn.bootstrapcdn.com%2Fbootstrap%2F3.3.7%2Fjs%2Fbootstrap.min.js',
-        },
-      }),
-      TestStubs.Replay.NavFrame({
-        timestamp: new Date(),
-        data: {
-          from: '/',
-          to: '/report/1655300817078_https%3A%2F%2Fmaxcdn.bootstrapcdn.com%2Fbootstrap%2F3.3.7%2Fjs%2Fbootstrap.min.js',
-        },
-      }),
-      TestStubs.Replay.NavFrame({
-        timestamp: new Date(),
-        data: {
-          from: '/',
-          to: '/report/1655300817078_https%3A%2F%2Fmaxcdn.bootstrapcdn.com%2Fbootstrap%2F3.3.7%2Fjs%2Fbootstrap.min.js',
-        },
-      }),
-      TestStubs.Replay.NavFrame({
-        timestamp: new Date(),
-        data: {
-          from: '/report/1655300817078_https%3A%2F%2Fmaxcdn.bootstrapcdn.com%2Fbootstrap%2F3.3.7%2Fjs%2Fbootstrap.min.js',
-          to: '/report/1669088273097_http%3A%2F%2Funderscorejs.org%2Funderscore-min.js',
-        },
-      }),
-    ]);
-    const frames = [init, ...breadcrumbs];
-
-    render(<FrameWalker frames={frames} replayRecord={replayRecord} />);
-
-    expect(screen.getByText('/')).toBeInTheDocument();
-    expect(screen.getByText('3 Pages')).toBeInTheDocument();
-    expect(
-      screen.getByText(
-        '/report/1669088273097_http%3A%2F%2Funderscorejs.org%2Funderscore-min.js'
-      )
-    ).toBeInTheDocument();
-  });
-});

+ 0 - 29
static/app/components/replays/walker/frameWalker.tsx

@@ -1,29 +0,0 @@
-import {memo} from 'react';
-
-import ChevronDividedList from 'sentry/components/replays/walker/chevronDividedList';
-import splitCrumbs from 'sentry/components/replays/walker/splitCrumbs';
-import useCrumbHandlers from 'sentry/utils/replays/hooks/useCrumbHandlers';
-import type {ReplayFrame} from 'sentry/utils/replays/types';
-import type {ReplayRecord} from 'sentry/views/replays/types';
-
-type Props = {
-  frames: ReplayFrame[];
-  replayRecord: ReplayRecord;
-};
-
-const FrameWalker = memo(function FrameWalker({frames, replayRecord}: Props) {
-  const {onClickTimestamp} = useCrumbHandlers();
-  const startTimestampMs = replayRecord.started_at.getTime();
-
-  return (
-    <ChevronDividedList
-      items={splitCrumbs({
-        frames,
-        onClick: onClickTimestamp,
-        startTimestampMs,
-      })}
-    />
-  );
-});
-
-export default FrameWalker;

+ 0 - 144
static/app/components/replays/walker/splitCrumbs.spec.tsx

@@ -1,144 +0,0 @@
-import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary';
-
-import splitCrumbs from 'sentry/components/replays/walker/splitCrumbs';
-import hydrateBreadcrumbs, {
-  replayInitBreadcrumb,
-} from 'sentry/utils/replays/hydrateBreadcrumbs';
-
-const replayRecord = TestStubs.ReplayRecord({});
-
-const INIT_FRAME = replayInitBreadcrumb(replayRecord);
-
-const [BOOTSTRAP_FRAME, UNDERSCORE_FRAME] = hydrateBreadcrumbs(replayRecord, [
-  TestStubs.Replay.NavFrame({
-    timestamp: new Date(),
-    data: {
-      from: '/',
-      to: '/report/1655300817078_https%3A%2F%2Fmaxcdn.bootstrapcdn.com%2Fbootstrap%2F3.3.7%2Fjs%2Fbootstrap.min.js',
-    },
-  }),
-  TestStubs.Replay.NavFrame({
-    timestamp: new Date(),
-    data: {
-      from: '/report/1655300817078_https%3A%2F%2Fmaxcdn.bootstrapcdn.com%2Fbootstrap%2F3.3.7%2Fjs%2Fbootstrap.min.js',
-      to: '/report/1669088273097_http%3A%2F%2Funderscorejs.org%2Funderscore-min.js',
-    },
-  }),
-]);
-
-describe('splitCrumbs', () => {
-  const onClick = null;
-  const startTimestampMs = 0;
-
-  it('should accept an empty list, and print that there are zero pages', () => {
-    const frames = [];
-
-    const results = splitCrumbs({
-      frames,
-      onClick,
-      startTimestampMs,
-    });
-    expect(results).toHaveLength(1);
-
-    render(results[0]);
-    expect(screen.getByText('0 Pages')).toBeInTheDocument();
-  });
-
-  it('should accept one crumb and return that single segment', () => {
-    const frames = [INIT_FRAME];
-
-    const results = splitCrumbs({
-      frames,
-      onClick,
-      startTimestampMs,
-    });
-    expect(results).toHaveLength(1);
-
-    render(results[0]);
-    expect(screen.getByText('/')).toBeInTheDocument();
-  });
-
-  it('should accept three crumbs and return them all as individual segments', () => {
-    const frames = [INIT_FRAME, BOOTSTRAP_FRAME, UNDERSCORE_FRAME];
-
-    const results = splitCrumbs({
-      frames,
-      onClick,
-      startTimestampMs,
-    });
-    expect(results).toHaveLength(3);
-
-    render(results[0]);
-    expect(screen.getByText('/')).toBeInTheDocument();
-
-    render(results[1]);
-    expect(
-      screen.getByText(
-        '/report/1655300817078_https%3A%2F%2Fmaxcdn.bootstrapcdn.com%2Fbootstrap%2F3.3.7%2Fjs%2Fbootstrap.min.js'
-      )
-    ).toBeInTheDocument();
-
-    render(results[2]);
-    expect(
-      screen.getByText(
-        '/report/1669088273097_http%3A%2F%2Funderscorejs.org%2Funderscore-min.js'
-      )
-    ).toBeInTheDocument();
-  });
-
-  it('should accept more than three crumbs and summarize the middle ones', () => {
-    const frames = [
-      INIT_FRAME,
-      BOOTSTRAP_FRAME,
-      BOOTSTRAP_FRAME,
-      BOOTSTRAP_FRAME,
-      UNDERSCORE_FRAME,
-    ];
-
-    const results = splitCrumbs({
-      frames,
-      onClick,
-      startTimestampMs,
-    });
-    expect(results).toHaveLength(3);
-
-    render(results[0]);
-    expect(screen.getByText('/')).toBeInTheDocument();
-
-    render(results[1]);
-    expect(screen.getByText('3 Pages')).toBeInTheDocument();
-
-    render(results[2]);
-    expect(
-      screen.getByText(
-        '/report/1669088273097_http%3A%2F%2Funderscorejs.org%2Funderscore-min.js'
-      )
-    ).toBeInTheDocument();
-  });
-
-  it('should show the summarized items on hover', async () => {
-    const frames = [
-      INIT_FRAME,
-      BOOTSTRAP_FRAME,
-      BOOTSTRAP_FRAME,
-      BOOTSTRAP_FRAME,
-      UNDERSCORE_FRAME,
-    ];
-
-    const results = splitCrumbs({
-      frames,
-      onClick,
-      startTimestampMs,
-    });
-    expect(results).toHaveLength(3);
-
-    render(results[1]);
-    expect(screen.getByText('3 Pages')).toBeInTheDocument();
-    expect(screen.queryByRole('listitem')).not.toBeInTheDocument();
-
-    await userEvent.hover(screen.getByText('3 Pages'));
-    await waitFor(() => {
-      expect(screen.getAllByRole('listitem')).toHaveLength(3);
-    });
-  });
-});

+ 0 - 136
static/app/components/replays/walker/splitCrumbs.tsx

@@ -1,136 +0,0 @@
-import styled from '@emotion/styled';
-
-import {Hovercard} from 'sentry/components/hovercard';
-import BreadcrumbItem from 'sentry/components/replays/breadcrumbs/breadcrumbItem';
-import TextOverflow from 'sentry/components/textOverflow';
-import {tn} from 'sentry/locale';
-import {space} from 'sentry/styles/space';
-import getFrameDetails from 'sentry/utils/replays/getFrameDetails';
-import useCrumbHandlers from 'sentry/utils/replays/hooks/useCrumbHandlers';
-import type {ReplayFrame} from 'sentry/utils/replays/types';
-
-type MaybeOnClickHandler = null | ((frame: ReplayFrame) => void);
-
-function splitCrumbs({
-  frames,
-  onClick,
-  startTimestampMs,
-}: {
-  frames: ReplayFrame[];
-  onClick: MaybeOnClickHandler;
-  startTimestampMs: number;
-}) {
-  const firstFrame = frames.slice(0, 1);
-  const summarizedFrames = frames.slice(1, -1);
-  const lastFrame = frames.slice(-1);
-
-  if (frames.length === 0) {
-    // This one shouldn't overflow, but by including <TextOverflow> css stays
-    // consistent with the other Segment types
-    return [
-      <Span key="summary">
-        <TextOverflow>{tn('%s Page', '%s Pages', 0)}</TextOverflow>
-      </Span>,
-    ];
-  }
-
-  if (frames.length > 3) {
-    return [
-      <SummarySegment
-        key="first"
-        frames={firstFrame}
-        startTimestampMs={startTimestampMs}
-        onClick={onClick}
-      />,
-      <SummarySegment
-        key="summary"
-        frames={summarizedFrames}
-        startTimestampMs={startTimestampMs}
-        onClick={onClick}
-      />,
-      <SummarySegment
-        key="last"
-        frames={lastFrame}
-        startTimestampMs={startTimestampMs}
-        onClick={onClick}
-      />,
-    ];
-  }
-
-  return frames.map((frame, i) => (
-    <SummarySegment
-      key={i}
-      frames={[frame]}
-      startTimestampMs={startTimestampMs}
-      onClick={onClick}
-    />
-  ));
-}
-
-function SummarySegment({
-  frames,
-  onClick,
-  startTimestampMs,
-}: {
-  frames: ReplayFrame[];
-  onClick: MaybeOnClickHandler;
-  startTimestampMs: number;
-}) {
-  const {onMouseEnter, onMouseLeave} = useCrumbHandlers();
-
-  const summaryItems = (
-    <ScrollingList>
-      {frames.map((frame, i) => (
-        <li key={i}>
-          <BreadcrumbItem
-            frame={frame}
-            onClick={onClick}
-            onMouseEnter={onMouseEnter}
-            onMouseLeave={onMouseLeave}
-            startTimestampMs={startTimestampMs}
-          />
-        </li>
-      ))}
-    </ScrollingList>
-  );
-
-  const label =
-    frames.length === 1
-      ? getFrameDetails(frames[0]).description
-      : tn('%s Page', '%s Pages', frames.length);
-  return (
-    <Span>
-      <HalfPaddingHovercard body={summaryItems} position="right">
-        <TextOverflow>{label}</TextOverflow>
-      </HalfPaddingHovercard>
-    </Span>
-  );
-}
-
-const ScrollingList = styled('ul')`
-  padding: 0;
-  margin: 0;
-  list-style: none;
-  max-height: calc(100vh - 32px);
-  overflow: scroll;
-`;
-
-const Span = styled('span')`
-  color: ${p => p.theme.gray500};
-  font-size: ${p => p.theme.fontSizeMedium};
-  line-height: 0;
-`;
-
-const HalfPaddingHovercard = styled(
-  ({children, bodyClassName, ...props}: React.ComponentProps<typeof Hovercard>) => (
-    <Hovercard bodyClassName={(bodyClassName ?? '') + ' half-padding'} {...props}>
-      {children}
-    </Hovercard>
-  )
-)`
-  .half-padding {
-    padding: ${space(0.5)};
-  }
-`;
-
-export default splitCrumbs;

+ 0 - 24
static/app/components/replays/walker/stringWalker.spec.tsx

@@ -1,24 +0,0 @@
-import {render, screen} from 'sentry-test/reactTestingLibrary';
-
-import StringWalker from 'sentry/components/replays/walker/stringWalker';
-
-describe('StringWalker', () => {
-  it('should accept a list of strings and render a <ChevronDividedList />', () => {
-    const urls = [
-      'https://sourcemaps.io/',
-      '/report/1655300817078_https%3A%2F%2Fmaxcdn.bootstrapcdn.com%2Fbootstrap%2F3.3.7%2Fjs%2Fbootstrap.min.js',
-      '/report/1669088273097_http%3A%2F%2Funderscorejs.org%2Funderscore-min.js',
-      '/report/1669088971516_https%3A%2F%2Fcdn.ravenjs.com%2F3.17.0%2Fraven.min.js',
-    ];
-
-    render(<StringWalker urls={urls} />);
-
-    expect(screen.getByText('/')).toBeInTheDocument();
-    expect(screen.getByText('2 Pages')).toBeInTheDocument();
-    expect(
-      screen.getByText(
-        '/report/1669088971516_https%3A%2F%2Fcdn.ravenjs.com%2F3.17.0%2Fraven.min.js'
-      )
-    ).toBeInTheDocument();
-  });
-});

+ 0 - 40
static/app/components/replays/walker/stringWalker.tsx

@@ -1,40 +0,0 @@
-import {memo} from 'react';
-
-import ChevronDividedList from 'sentry/components/replays/walker/chevronDividedList';
-import splitCrumbs from 'sentry/components/replays/walker/splitCrumbs';
-import {BreadcrumbType} from 'sentry/types/breadcrumbs';
-import type {ReplayFrame} from 'sentry/utils/replays/types';
-
-type StringProps = {
-  urls: string[];
-};
-
-const StringWalker = memo(function StringWalker({urls}: StringProps) {
-  return (
-    <ChevronDividedList
-      items={splitCrumbs({
-        frames: urls.map(urlToFrame),
-        startTimestampMs: 0,
-        onClick: null,
-      })}
-    />
-  );
-});
-
-export default StringWalker;
-
-function urlToFrame(url: string): ReplayFrame {
-  const now = new Date();
-  return {
-    category: 'navigation',
-    data: {
-      from: '',
-      to: url,
-    },
-    message: url,
-    timestamp: now,
-    type: BreadcrumbType.NAVIGATION,
-    offsetMs: 0,
-    timestampMs: now.getTime() / 1000,
-  };
-}

+ 0 - 1
static/app/views/replays/deadRageClick/exampleReplaysList.tsx

@@ -91,7 +91,6 @@ export default function ExampleReplaysList({
               eventView={eventView}
               organization={organization}
               referrer={referrer}
-              showUrl={false}
               referrer_table="selector-widget"
               isWidget
             />

+ 2 - 27
static/app/views/replays/detail/page.tsx

@@ -10,12 +10,9 @@ import FeedbackButton from 'sentry/components/replays/header/feedbackButton';
 import HeaderPlaceholder from 'sentry/components/replays/header/headerPlaceholder';
 import ReplayMetaData from 'sentry/components/replays/header/replayMetaData';
 import ShareButton from 'sentry/components/replays/shareButton';
-import FrameWalker from 'sentry/components/replays/walker/frameWalker';
-import StringWalker from 'sentry/components/replays/walker/stringWalker';
 import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
-import type {ReplayFrame} from 'sentry/utils/replays/types';
 import type {ReplayError, ReplayRecord} from 'sentry/views/replays/types';
 
 type Props = {
@@ -24,17 +21,9 @@ type Props = {
   projectSlug: string | null;
   replayErrors: ReplayError[];
   replayRecord: undefined | ReplayRecord;
-  frames?: ReplayFrame[];
 };
 
-function Page({
-  children,
-  frames,
-  orgSlug,
-  replayRecord,
-  projectSlug,
-  replayErrors,
-}: Props) {
+function Page({children, orgSlug, replayRecord, projectSlug, replayErrors}: Props) {
   const title = replayRecord
     ? `${replayRecord.id} — Session Replay — ${orgSlug}`
     : `Session Replay — ${orgSlug}`;
@@ -71,15 +60,7 @@ function Page({
             id: replayRecord.user.id || '',
           }}
           // this is the subheading for the avatar, so displayEmail in this case is a misnomer
-          displayEmail={
-            <Cols>
-              {frames?.length ? (
-                <FrameWalker replayRecord={replayRecord} frames={frames} />
-              ) : (
-                <StringWalker urls={replayRecord.urls} />
-              )}
-            </Cols>
-          }
+          displayEmail={<div>{undefined}</div>}
         />
       ) : (
         <HeaderPlaceholder width="100%" height="58px" />
@@ -108,12 +89,6 @@ const Header = styled(Layout.Header)`
   }
 `;
 
-const Cols = styled('div')`
-  display: flex;
-  flex-direction: column;
-  gap: ${space(0.25)};
-`;
-
 // TODO(replay); This could make a lot of sense to put inside HeaderActions by default
 const ButtonActionsWrapper = styled(Layout.HeaderActions)`
   flex-direction: row;

Some files were not shown because too many files changed in this diff