Browse Source

fix(replays): improve a11y of replay details (#64923)

Closes https://github.com/getsentry/sentry/issues/57285
Contributes to https://github.com/getsentry/sentry/issues/64383 (not
fully closed)

Several updates to the replay details to improve a11y and general
consistency:

## 1. Breadcrumbs tab has errors highlighted in red, to be consistent
with other tabs


https://github.com/getsentry/sentry/assets/56095982/1597dd45-de4f-4e34-ae2f-b28d20992c8c

## 2. Console tab text is black but background highlight & icon color
still persists
<img width="606" alt="SCR-20240208-nkcb"
src="https://github.com/getsentry/sentry/assets/56095982/6623e273-fbb7-4d2f-8da7-4d787f812b9d">

We also do not show a light grey color for events in the future; all
text is the same color.

## 3. Network tab: selected item is highlighted purple


https://github.com/getsentry/sentry/assets/56095982/06ede027-6d91-421c-98cd-2fa4e30b413f

Same as console tab, we also do not show a light grey color for events
in the future; all text is the same color.

## 4. A11y tab has background highlights. No more syntax highlighting or
scrollbar issues in the row itself because we're using a tooltip on
overflow now.
 

https://github.com/getsentry/sentry/assets/56095982/c98aab20-5b5e-4f53-9e4c-3dcc8af53ca7


## 5. Everything looks good in dark mode


https://github.com/getsentry/sentry/assets/56095982/c66bb2f8-0d82-464b-aa0c-a502e9d0b01d
Michelle Zhang 1 year ago
parent
commit
8db28eb47a

+ 3 - 2
static/app/components/replays/breadcrumbs/breadcrumbItem.tsx

@@ -72,6 +72,7 @@ function BreadcrumbItem({
 
   return (
     <CrumbItem
+      isErrorFrame={isErrorFrame(frame)}
       as={onClick && !forceSpan ? 'button' : 'span'}
       onClick={e => onClick?.(frame, e)}
       onMouseEnter={e => onMouseEnter(frame, e)}
@@ -250,7 +251,7 @@ const Description = styled(Tooltip)`
   color: ${p => p.theme.subText};
 `;
 
-const CrumbItem = styled(PanelItem)`
+const CrumbItem = styled(PanelItem)<{isErrorFrame?: boolean}>`
   display: grid;
   grid-template-columns: max-content auto;
   align-items: flex-start;
@@ -258,7 +259,7 @@ const CrumbItem = styled(PanelItem)`
   width: 100%;
 
   font-size: ${p => p.theme.fontSizeMedium};
-  background: transparent;
+  background: ${p => (p.isErrorFrame ? `${p.theme.red100}` : `transparent`)};
   padding: ${space(1)};
   text-align: left;
   border: none;

+ 3 - 12
static/app/components/replays/virtualizedGrid/bodyCell.tsx

@@ -6,7 +6,7 @@ import {space} from 'sentry/styles/space';
 
 const cellBackground = (p: CellProps & {theme: Theme}) => {
   if (p.isSelected) {
-    return `background-color: ${p.theme.black};`;
+    return `background-color: ${p.theme.purple300};`;
   }
   if (p.isStatusError) {
     return `background-color: ${p.theme.red100};`;
@@ -19,20 +19,11 @@ const cellBackground = (p: CellProps & {theme: Theme}) => {
 
 const cellColor = (p: CellProps & {theme: Theme}) => {
   if (p.isSelected) {
-    const color = p.isStatusError
-      ? p.theme.red300
-      : p.isStatusWarning
-        ? p.theme.yellow300
-        : p.theme.white;
+    const color = p.theme.white;
     return `color: ${color};`;
   }
-  const colors = p.isStatusError
-    ? [p.theme.red300, p.theme.red400]
-    : p.isStatusWarning
-      ? [p.theme.textColor, p.theme.subText]
-      : ['inherit', p.theme.subText];
 
-  return `color: ${p.hasOccurred !== false ? colors[0] : colors[1]};`;
+  return `color: inherit`;
 };
 
 type CellProps = {

+ 45 - 23
static/app/views/replays/detail/accessibility/accessibilityTableCell.tsx

@@ -1,29 +1,20 @@
-import type {ComponentProps, CSSProperties} from 'react';
+import type {ComponentProps, CSSProperties, ReactNode} from 'react';
 import {forwardRef} from 'react';
+import styled from '@emotion/styled';
 import classNames from 'classnames';
 
-import {
-  Cell,
-  CodeHighlightCell,
-  Text,
-} from 'sentry/components/replays/virtualizedGrid/bodyCell';
+import {Cell, Text} from 'sentry/components/replays/virtualizedGrid/bodyCell';
+import TextOverflow from 'sentry/components/textOverflow';
 import {Tooltip} from 'sentry/components/tooltip';
 import {IconFire, IconInfo, IconWarning} from 'sentry/icons';
+import {space} from 'sentry/styles/space';
 import type useCrumbHandlers from 'sentry/utils/replays/hooks/useCrumbHandlers';
 import type {HydratedA11yFrame} from 'sentry/utils/replays/hydrateA11yFrame';
-import type {Color} from 'sentry/utils/theme';
 import useUrlParams from 'sentry/utils/useUrlParams';
 import type useSortAccessibility from 'sentry/views/replays/detail/accessibility/useSortAccessibility';
 
 const EMPTY_CELL = '--';
 
-const IMPACT_ICON_MAPPING: Record<keyof HydratedA11yFrame['impact'], Color> = {
-  minor: <IconInfo size="xs" />,
-  moderate: <IconInfo size="xs" />,
-  serious: <IconWarning size="xs" color="yellow400" />,
-  critical: <IconFire size="xs" color="red400" />,
-};
-
 interface Props extends ReturnType<typeof useCrumbHandlers> {
   a11yIssue: HydratedA11yFrame;
   columnIndex: number;
@@ -57,6 +48,13 @@ const AccessibilityTableCell = forwardRef<HTMLDivElement, Props>(
     const {getParamValue} = useUrlParams('a_detail_row', '');
     const isSelected = getParamValue() === String(dataIndex);
 
+    const IMPACT_ICON_MAPPING: Record<keyof HydratedA11yFrame['impact'], ReactNode> = {
+      minor: <IconInfo size="xs" />,
+      moderate: <IconInfo size="xs" />,
+      serious: <IconWarning size="xs" color={isSelected ? 'white' : 'yellow400'} />,
+      critical: <IconFire size="xs" color={isSelected ? 'white' : 'red400'} />,
+    };
+
     const hasOccurred = currentTime >= a11yIssue.offsetMs;
     const isBeforeHover =
       currentHoverTime === undefined || currentHoverTime >= a11yIssue.offsetMs;
@@ -99,7 +97,7 @@ const AccessibilityTableCell = forwardRef<HTMLDivElement, Props>(
 
     const renderFns = [
       () => (
-        <Cell {...columnProps}>
+        <StyledCell {...columnProps} impact={a11yIssue.impact} isRowSelected={isSelected}>
           <Text>
             {a11yIssue.impact ? (
               <Tooltip title={a11yIssue.impact ?? EMPTY_CELL}>
@@ -109,19 +107,26 @@ const AccessibilityTableCell = forwardRef<HTMLDivElement, Props>(
               EMPTY_CELL
             )}
           </Text>
-        </Cell>
+        </StyledCell>
       ),
       () => (
-        <Cell {...columnProps}>
+        <StyledCell {...columnProps} impact={a11yIssue.impact} isRowSelected={isSelected}>
           <Text>{a11yIssue.id ?? EMPTY_CELL}</Text>
-        </Cell>
+        </StyledCell>
       ),
       () => (
-        <Cell {...columnProps}>
-          <CodeHighlightCell language="html" hideCopyButton data-render-inline>
-            {a11yIssue.element.element ?? EMPTY_CELL}
-          </CodeHighlightCell>
-        </Cell>
+        <StyledCell {...columnProps} impact={a11yIssue.impact} isRowSelected={isSelected}>
+          <Tooltip
+            title={a11yIssue.element.element ?? EMPTY_CELL}
+            isHoverable
+            showOnlyOnOverflow
+            overlayStyle={{maxWidth: '500px !important'}}
+          >
+            <StyledTextOverflow>
+              {a11yIssue.element.element ?? EMPTY_CELL}
+            </StyledTextOverflow>
+          </Tooltip>
+        </StyledCell>
       ),
     ];
 
@@ -130,3 +135,20 @@ const AccessibilityTableCell = forwardRef<HTMLDivElement, Props>(
 );
 
 export default AccessibilityTableCell;
+
+const StyledTextOverflow = styled(TextOverflow)`
+padding-right: ${space(1)};`;
+
+const StyledCell = styled(Cell)<{
+  impact: HydratedA11yFrame['impact'];
+  isRowSelected: boolean;
+}>`
+background: ${p =>
+  p.isSelected
+    ? p.theme.purple300
+    : p.impact === 'serious'
+      ? p.theme.yellow100
+      : p.impact === 'critical'
+        ? p.theme.red100
+        : 'transparent'}
+`;

+ 4 - 9
static/app/views/replays/detail/console/consoleLogRow.tsx

@@ -104,12 +104,7 @@ const ConsoleLog = styled('div')<{
   border-top: 1px solid transparent;
   border-bottom: 1px solid transparent;
 
-  color: ${p =>
-    ['warning', 'error'].includes(String(p.level))
-      ? p.theme.alert[String(p.level)].iconColor
-      : p.hasOccurred
-        ? 'inherit'
-        : p.theme.gray300};
+  color: ${p => p.theme.gray400};
 
   /*
   Show the timestamp button "Play" icon when we hover the row.
@@ -124,17 +119,17 @@ const ConsoleLog = styled('div')<{
 const ICONS = {
   [BreadcrumbLevelType.ERROR]: (
     <Tooltip title={BreadcrumbLevelType.ERROR}>
-      <IconClose size="xs" isCircled />
+      <IconClose size="xs" color="red400" isCircled />
     </Tooltip>
   ),
   [BreadcrumbLevelType.WARNING]: (
     <Tooltip title={BreadcrumbLevelType.WARNING}>
-      <IconWarning size="xs" />
+      <IconWarning color="yellow400" size="xs" />
     </Tooltip>
   ),
   [BreadcrumbLevelType.INFO]: (
     <Tooltip title={BreadcrumbLevelType.INFO}>
-      <IconInfo size="xs" />
+      <IconInfo color="gray400" size="xs" />
     </Tooltip>
   ),
 };