Browse Source

ref(replay): Deduplicate components used in Replay Details>Error & Network tabs (#50884)

This is following up on a comment from
https://github.com/getsentry/sentry/pull/50701
Ryan Albrecht 1 year ago
parent
commit
6fa6337afd

+ 74 - 0
static/app/components/replays/virtualizedGrid/bodyCell.tsx

@@ -0,0 +1,74 @@
+import {Theme} from '@emotion/react';
+import styled from '@emotion/styled';
+
+import {space} from 'sentry/styles/space';
+import TimestampButton from 'sentry/views/replays/detail/timestampButton';
+
+const cellBackground = (p: CellProps & {theme: Theme}) => {
+  if (p.isSelected) {
+    return `background-color: ${p.theme.textColor};`;
+  }
+  if (p.hasOccurred === undefined && !p.isStatusError) {
+    return `background-color: inherit;`;
+  }
+  const color = p.isStatusError ? p.theme.alert.error.backgroundLight : 'inherit';
+  return `background-color: ${color};`;
+};
+
+const cellColor = (p: CellProps & {theme: Theme}) => {
+  if (p.isSelected) {
+    const colors = p.isStatusError
+      ? [p.theme.alert.error.background]
+      : [p.theme.background];
+    return `color: ${colors[0]};`;
+  }
+  const colors = p.isStatusError
+    ? [p.theme.alert.error.borderHover, p.theme.alert.error.iconColor]
+    : ['inherit', p.theme.gray300];
+
+  return `color: ${p.hasOccurred !== false ? colors[0] : colors[1]};`;
+};
+
+type CellProps = {
+  align?: 'flex-start' | 'flex-end';
+  className?: string;
+  gap?: Parameters<typeof space>[0];
+  hasOccurred?: boolean;
+  isSelected?: boolean;
+  isStatusError?: boolean;
+  numeric?: boolean;
+  onClick?: undefined | (() => void);
+};
+
+export const Cell = styled('div')<CellProps>`
+  display: flex;
+  gap: ${p => space(p.gap ?? 0)};
+  align-items: center;
+  font-size: ${p => p.theme.fontSizeSmall};
+  cursor: ${p => (p.onClick ? 'pointer' : 'inherit')};
+
+  ${cellBackground}
+  ${cellColor}
+
+  ${p =>
+    p.numeric &&
+    `
+    font-variant-numeric: tabular-nums;
+    justify-content: ${p.align ?? 'flex-end'};
+  `};
+`;
+
+export const Text = styled('div')`
+  text-overflow: ellipsis;
+  white-space: nowrap;
+  overflow: hidden;
+  padding: ${space(0.75)} ${space(1.5)};
+`;
+
+export const AvatarWrapper = styled('div')`
+  align-self: center;
+`;
+
+export const StyledTimestampButton = styled(TimestampButton)`
+  padding-inline: ${space(1.5)};
+`;

+ 81 - 0
static/app/components/replays/virtualizedGrid/headerCell.tsx

@@ -0,0 +1,81 @@
+import {CSSProperties, forwardRef, ReactNode} from 'react';
+import styled from '@emotion/styled';
+
+import {Tooltip} from 'sentry/components/tooltip';
+import {IconArrow, IconInfo} from 'sentry/icons';
+import {space} from 'sentry/styles/space';
+import type {Crumb} from 'sentry/types/breadcrumbs';
+import type {NetworkSpan} from 'sentry/views/replays/types';
+
+interface SortCrumbs {
+  asc: boolean;
+  by: keyof Crumb | string;
+  getValue: (row: Crumb) => any;
+}
+interface SortSpans {
+  asc: boolean;
+  by: keyof NetworkSpan | string;
+  getValue: (row: NetworkSpan) => any;
+}
+
+type Props = {
+  field: string;
+  handleSort: (fieldName: string) => void;
+  label: string;
+  sortConfig: SortCrumbs | SortSpans;
+  style: CSSProperties;
+  tooltipTitle: undefined | ReactNode;
+};
+
+const StyledIconInfo = styled(IconInfo)`
+  display: block;
+`;
+
+function CatchClicks({children}: {children: ReactNode}) {
+  return <div onClick={e => e.stopPropagation()}>{children}</div>;
+}
+
+const HeaderCell = forwardRef<HTMLButtonElement, Props>(
+  ({field, handleSort, label, sortConfig, style, tooltipTitle}: Props, ref) => (
+    <HeaderButton style={style} onClick={() => handleSort(field)} ref={ref}>
+      {label}
+      {tooltipTitle ? (
+        <Tooltip isHoverable title={<CatchClicks>{tooltipTitle}</CatchClicks>}>
+          <StyledIconInfo size="xs" />
+        </Tooltip>
+      ) : null}
+      <IconArrow
+        color="gray300"
+        size="xs"
+        direction={sortConfig.by === field && !sortConfig.asc ? 'down' : 'up'}
+        style={{visibility: sortConfig.by === field ? 'visible' : 'hidden'}}
+      />
+    </HeaderButton>
+  )
+);
+
+const HeaderButton = styled('button')`
+  border: 0;
+  border-bottom: 1px solid ${p => p.theme.border};
+  background: ${p => p.theme.backgroundSecondary};
+  color: ${p => p.theme.subText};
+
+  font-size: ${p => p.theme.fontSizeSmall};
+  font-weight: 600;
+  line-height: 16px;
+  text-align: unset;
+  text-transform: uppercase;
+  white-space: nowrap;
+
+  width: 100%;
+  display: flex;
+  align-items: center;
+  justify-content: space-between;
+  padding: ${space(0.5)} ${space(1)} ${space(0.5)} ${space(1.5)};
+
+  svg {
+    margin-left: ${space(0.25)};
+  }
+`;
+
+export default HeaderCell;

+ 11 - 50
static/app/views/replays/detail/errorList/errorHeaderCell.tsx

@@ -1,10 +1,8 @@
-import {ComponentProps, CSSProperties, forwardRef, ReactNode} from 'react';
-import styled from '@emotion/styled';
+import {ComponentProps, CSSProperties, forwardRef} from 'react';
 
+import HeaderCell from 'sentry/components/replays/virtualizedGrid/headerCell';
 import {Tooltip} from 'sentry/components/tooltip';
-import {IconArrow, IconInfo} from 'sentry/icons';
 import {t} from 'sentry/locale';
-import {space} from 'sentry/styles/space';
 import useSortErrors from 'sentry/views/replays/detail/errorList/useSortErrors';
 
 type SortConfig = ReturnType<typeof useSortErrors>['sortConfig'];
@@ -15,10 +13,6 @@ type Props = {
   style: CSSProperties;
 };
 
-const SizeInfoIcon = styled(IconInfo)`
-  display: block;
-`;
-
 const COLUMNS: {
   field: SortConfig['by'];
   label: string;
@@ -32,54 +26,21 @@ const COLUMNS: {
 
 export const COLUMN_COUNT = COLUMNS.length;
 
-function CatchClicks({children}: {children: ReactNode}) {
-  return <div onClick={e => e.stopPropagation()}>{children}</div>;
-}
-
 const ErrorHeaderCell = forwardRef<HTMLButtonElement, Props>(
   ({handleSort, index, sortConfig, style}: Props, ref) => {
     const {field, label, tooltipTitle} = COLUMNS[index];
     return (
-      <HeaderButton style={style} onClick={() => handleSort(field)} ref={ref}>
-        {label}
-        {tooltipTitle ? (
-          <Tooltip isHoverable title={<CatchClicks>{tooltipTitle}</CatchClicks>}>
-            <SizeInfoIcon size="xs" />
-          </Tooltip>
-        ) : null}
-        <IconArrow
-          color="gray300"
-          size="xs"
-          direction={sortConfig.by === field && !sortConfig.asc ? 'down' : 'up'}
-          style={{visibility: sortConfig.by === field ? 'visible' : 'hidden'}}
-        />
-      </HeaderButton>
+      <HeaderCell
+        ref={ref}
+        handleSort={handleSort}
+        field={field}
+        label={label}
+        tooltipTitle={tooltipTitle}
+        sortConfig={sortConfig}
+        style={style}
+      />
     );
   }
 );
 
-const HeaderButton = styled('button')`
-  border: 0;
-  border-bottom: 1px solid ${p => p.theme.border};
-  background: ${p => p.theme.backgroundSecondary};
-  color: ${p => p.theme.subText};
-
-  font-size: ${p => p.theme.fontSizeSmall};
-  font-weight: 600;
-  line-height: 16px;
-  text-align: unset;
-  text-transform: uppercase;
-  white-space: nowrap;
-
-  width: 100%;
-  display: flex;
-  align-items: center;
-  justify-content: space-between;
-  padding: ${space(0.5)} ${space(1)} ${space(0.5)} ${space(1.5)};
-
-  svg {
-    margin-left: ${space(0.25)};
-  }
-`;
-
 export default ErrorHeaderCell;

+ 20 - 69
static/app/views/replays/detail/errorList/errorTableCell.tsx

@@ -1,11 +1,15 @@
-import {CSSProperties, forwardRef, useMemo} from 'react';
-import styled from '@emotion/styled';
+import {ComponentProps, CSSProperties, forwardRef, useMemo} from 'react';
 import classNames from 'classnames';
 
 import Avatar from 'sentry/components/avatar';
 import Link from 'sentry/components/links/link';
 import {relativeTimeInMs} from 'sentry/components/replays/utils';
-import {space} from 'sentry/styles/space';
+import {
+  AvatarWrapper,
+  Cell,
+  StyledTimestampButton,
+  Text,
+} from 'sentry/components/replays/virtualizedGrid/bodyCell';
 import type {Crumb} from 'sentry/types/breadcrumbs';
 import {getShortEventId} from 'sentry/utils/events';
 import useOrganization from 'sentry/utils/useOrganization';
@@ -14,7 +18,6 @@ import {normalizeUrl} from 'sentry/utils/withDomainRequired';
 import {QuickContextHoverWrapper} from 'sentry/views/discover/table/quickContext/quickContextWrapper';
 import {ContextType} from 'sentry/views/discover/table/quickContext/utils';
 import useSortErrors from 'sentry/views/replays/detail/errorList/useSortErrors';
-import TimestampButton from 'sentry/views/replays/detail/timestampButton';
 
 const EMPTY_CELL = '--';
 
@@ -32,15 +35,6 @@ type Props = {
   style: CSSProperties;
 };
 
-type CellProps = {
-  hasOccurred: boolean | undefined;
-  align?: 'flex-start' | 'flex-end';
-  className?: string;
-  gap?: Parameters<typeof space>[0];
-  numeric?: boolean;
-  onClick?: undefined | (() => void);
-};
-
 const ErrorTableCell = forwardRef<HTMLDivElement, Props>(
   (
     {
@@ -120,7 +114,7 @@ const ErrorTableCell = forwardRef<HTMLDivElement, Props>(
       onMouseLeave: () => onMouseLeave(crumb),
       ref,
       style,
-    } as CellProps;
+    } as ComponentProps<typeof Cell>;
 
     const renderFns = [
       () => (
@@ -136,16 +130,18 @@ const ErrorTableCell = forwardRef<HTMLDivElement, Props>(
       ),
       () => (
         <Cell {...columnProps}>
-          <QuickContextHoverWrapper
-            dataRow={{
-              id: eventId,
-              'project.name': projectSlug,
-            }}
-            contextType={ContextType.EVENT}
-            organization={organization}
-          >
-            <Text>{title ?? EMPTY_CELL}</Text>
-          </QuickContextHoverWrapper>
+          <Text>
+            <QuickContextHoverWrapper
+              dataRow={{
+                id: eventId,
+                'project.name': projectSlug,
+              }}
+              contextType={ContextType.EVENT}
+              organization={organization}
+            >
+              {title ?? EMPTY_CELL}
+            </QuickContextHoverWrapper>
+          </Text>
         </Cell>
       ),
       () => (
@@ -187,49 +183,4 @@ const ErrorTableCell = forwardRef<HTMLDivElement, Props>(
   }
 );
 
-const cellBackground = p => {
-  if (p.hasOccurred === undefined && !p.isStatusError) {
-    const color = p.isHovered ? p.theme.hover : 'inherit';
-    return `background-color: ${color};`;
-  }
-  return `background-color: inherit;`;
-};
-
-const cellColor = p => {
-  return `color: ${p.hasOccurred !== false ? 'inherit' : p.theme.gray300};`;
-};
-
-const Cell = styled('div')<CellProps>`
-  display: flex;
-  gap: ${p => space(p.gap ?? 0)};
-  align-items: center;
-  font-size: ${p => p.theme.fontSizeSmall};
-  cursor: ${p => (p.onClick ? 'pointer' : 'inherit')};
-
-  ${cellBackground}
-  ${cellColor}
-
-  ${p =>
-    p.numeric &&
-    `
-    font-variant-numeric: tabular-nums;
-    justify-content: ${p.align ?? 'flex-end'};
-  `};
-`;
-
-const Text = styled('div')`
-  text-overflow: ellipsis;
-  white-space: nowrap;
-  overflow: hidden;
-  padding: ${space(0.75)} ${space(1.5)};
-`;
-
-const AvatarWrapper = styled('div')`
-  align-self: center;
-`;
-
-const StyledTimestampButton = styled(TimestampButton)`
-  padding-inline: ${space(1.5)};
-`;
-
 export default ErrorTableCell;

+ 11 - 49
static/app/views/replays/detail/network/networkHeaderCell.tsx

@@ -1,11 +1,9 @@
-import {ComponentProps, CSSProperties, forwardRef, ReactNode} from 'react';
-import styled from '@emotion/styled';
+import {ComponentProps, CSSProperties, forwardRef} from 'react';
 
 import ExternalLink from 'sentry/components/links/externalLink';
+import HeaderCell from 'sentry/components/replays/virtualizedGrid/headerCell';
 import {Tooltip} from 'sentry/components/tooltip';
-import {IconArrow, IconInfo} from 'sentry/icons';
 import {t, tct} from 'sentry/locale';
-import {space} from 'sentry/styles/space';
 import useSortNetwork from 'sentry/views/replays/detail/network/useSortNetwork';
 
 type SortConfig = ReturnType<typeof useSortNetwork>['sortConfig'];
@@ -16,10 +14,6 @@ type Props = {
   style: CSSProperties;
 };
 
-const SizeInfoIcon = styled(IconInfo)`
-  display: block;
-`;
-
 const COLUMNS: {
   field: SortConfig['by'];
   label: string;
@@ -61,53 +55,21 @@ const COLUMNS: {
 
 export const COLUMN_COUNT = COLUMNS.length;
 
-function CatchClicks({children}: {children: ReactNode}) {
-  return <div onClick={e => e.stopPropagation()}>{children}</div>;
-}
-
 const NetworkHeaderCell = forwardRef<HTMLButtonElement, Props>(
   ({handleSort, index, sortConfig, style}: Props, ref) => {
     const {field, label, tooltipTitle} = COLUMNS[index];
     return (
-      <HeaderButton style={style} onClick={() => handleSort(field)} ref={ref}>
-        {label}
-        {tooltipTitle ? (
-          <Tooltip isHoverable title={<CatchClicks>{tooltipTitle}</CatchClicks>}>
-            <SizeInfoIcon size="xs" />
-          </Tooltip>
-        ) : null}
-        <IconArrow
-          color="gray300"
-          size="xs"
-          direction={sortConfig.by === field && !sortConfig.asc ? 'down' : 'up'}
-          style={{visibility: sortConfig.by === field ? 'visible' : 'hidden'}}
-        />
-      </HeaderButton>
+      <HeaderCell
+        ref={ref}
+        handleSort={handleSort}
+        field={field}
+        label={label}
+        tooltipTitle={tooltipTitle}
+        sortConfig={sortConfig}
+        style={style}
+      />
     );
   }
 );
 
-const HeaderButton = styled('button')`
-  border: 0;
-  border-bottom: 1px solid ${p => p.theme.border};
-  background: ${p => p.theme.backgroundSecondary};
-  color: ${p => p.theme.subText};
-
-  font-size: ${p => p.theme.fontSizeSmall};
-  font-weight: 600;
-  line-height: 16px;
-  text-align: unset;
-  text-transform: uppercase;
-
-  width: 100%;
-  display: flex;
-  align-items: center;
-  justify-content: space-between;
-  padding: ${space(0.5)} ${space(1)} ${space(0.5)} ${space(1.5)};
-
-  svg {
-    margin-left: ${space(0.25)};
-  }
-`;
-
 export default NetworkHeaderCell;

+ 9 - 70
static/app/views/replays/detail/network/networkTableCell.tsx

@@ -1,14 +1,16 @@
-import {CSSProperties, forwardRef, MouseEvent, useMemo} from 'react';
-import styled from '@emotion/styled';
+import {ComponentProps, CSSProperties, forwardRef, MouseEvent, useMemo} from 'react';
 import classNames from 'classnames';
 
 import FileSize from 'sentry/components/fileSize';
 import {relativeTimeInMs} from 'sentry/components/replays/utils';
+import {
+  Cell,
+  StyledTimestampButton,
+  Text,
+} from 'sentry/components/replays/virtualizedGrid/bodyCell';
 import {Tooltip} from 'sentry/components/tooltip';
-import {space} from 'sentry/styles/space';
 import useUrlParams from 'sentry/utils/useUrlParams';
 import useSortNetwork from 'sentry/views/replays/detail/network/useSortNetwork';
-import TimestampButton from 'sentry/views/replays/detail/timestampButton';
 import {operationName} from 'sentry/views/replays/detail/utils';
 import type {NetworkSpan} from 'sentry/views/replays/types';
 
@@ -29,15 +31,6 @@ type Props = {
   style: CSSProperties;
 };
 
-type CellProps = {
-  hasOccurred: boolean | undefined;
-  isDetailsOpen: boolean;
-  isStatusError: boolean;
-  className?: string;
-  numeric?: boolean;
-  onClick?: undefined | (() => void);
-};
-
 const NetworkTableCell = forwardRef<HTMLDivElement, Props>(
   (
     {
@@ -60,7 +53,7 @@ const NetworkTableCell = forwardRef<HTMLDivElement, Props>(
     const dataIndex = rowIndex - 1;
 
     const {getParamValue} = useUrlParams('n_detail_row', '');
-    const isDetailsOpen = getParamValue() === String(dataIndex);
+    const isSelected = getParamValue() === String(dataIndex);
 
     const startMs = span.startTimestamp * 1000;
     const endMs = span.endTimestamp * 1000;
@@ -104,14 +97,14 @@ const NetworkTableCell = forwardRef<HTMLDivElement, Props>(
             : undefined,
       }),
       hasOccurred: isByTimestamp ? hasOccurred : undefined,
-      isDetailsOpen,
+      isSelected,
       isStatusError: typeof statusCode === 'number' && statusCode >= 400,
       onClick: () => onClickCell({dataIndex, rowIndex}),
       onMouseEnter: () => onMouseEnter(span),
       onMouseLeave: () => onMouseLeave(span),
       ref,
       style,
-    } as CellProps;
+    } as ComponentProps<typeof Cell>;
 
     const renderFns = [
       () => (
@@ -174,58 +167,4 @@ const NetworkTableCell = forwardRef<HTMLDivElement, Props>(
   }
 );
 
-const cellBackground = p => {
-  if (p.isDetailsOpen) {
-    return `background-color: ${p.theme.textColor};`;
-  }
-  if (p.hasOccurred === undefined && !p.isStatusError) {
-    const color = p.isHovered ? p.theme.hover : 'inherit';
-    return `background-color: ${color};`;
-  }
-  const color = p.isStatusError ? p.theme.alert.error.backgroundLight : 'inherit';
-  return `background-color: ${color};`;
-};
-
-const cellColor = p => {
-  if (p.isDetailsOpen) {
-    const colors = p.isStatusError
-      ? [p.theme.alert.error.background]
-      : [p.theme.background];
-    return `color: ${colors[0]};`;
-  }
-  const colors = p.isStatusError
-    ? [p.theme.alert.error.borderHover, p.theme.alert.error.iconColor]
-    : ['inherit', p.theme.gray300];
-
-  return `color: ${p.hasOccurred !== false ? colors[0] : colors[1]};`;
-};
-
-const Cell = styled('div')<CellProps>`
-  display: flex;
-  align-items: center;
-  font-size: ${p => p.theme.fontSizeSmall};
-  cursor: ${p => (p.onClick ? 'pointer' : 'inherit')};
-
-  ${cellBackground}
-  ${cellColor}
-
-  ${p =>
-    p.numeric &&
-    `
-    font-variant-numeric: tabular-nums;
-    justify-content: flex-end;
-  `};
-`;
-
-const Text = styled('div')`
-  text-overflow: ellipsis;
-  white-space: nowrap;
-  overflow: hidden;
-  padding: ${space(0.75)} ${space(1.5)};
-`;
-
-const StyledTimestampButton = styled(TimestampButton)`
-  padding-inline: ${space(1.5)};
-`;
-
 export default NetworkTableCell;