ref(crumbs): Improve list perf (#19911)

package.json

@@ -46,7 +46,7 @@
     "@types/react-router": "^3.0.22",
     "@types/react-select": "3.0.8",
     "@types/react-sparklines": "^1.7.0",
-    "@types/react-virtualized": "^9.20.1",
+    "@types/react-virtualized": "^9.21.10",
     "@types/reflexbox": "^4.0.0",
     "@types/reflux": "0.4.1",
     "@types/scroll-to-element": "^2.0.0",
@@ -111,7 +111,7 @@
     "react-select": "^3.0.8",
     "react-select-legacy": "npm:react-select-legacy@1",
     "react-sparklines": "1.7.0",
-    "react-virtualized": "^9.20.1",
+    "react-virtualized": "^9.21.2",
     "reflexbox": "^4.0.6",
     "reflux": "0.4.1",
     "regenerator-runtime": "^0.13.3",

src/sentry/static/sentry/app/components/events/interfaces/breadcrumbsV2/data/summary.tsx

@@ -3,7 +3,6 @@ import styled from '@emotion/styled';
 
 import Highlight from 'app/components/highlight';
 import {getMeta} from 'app/components/events/meta/metaProxy';
-import overflowEllipsis from 'app/styles/overflowEllipsis';
 import {defined} from 'app/utils';
 
 import getBreadcrumbCustomRendererValue from '../../breadcrumbs/getBreadcrumbCustomRendererValue';
@@ -86,16 +85,10 @@ const StyledPre = styled('pre')`
   word-break: break-all;
   margin: 0;
   font-size: ${p => p.theme.fontSizeSmall};
-  @media (max-width: ${p => p.theme.breakpoints[0]}) {
-    ${overflowEllipsis};
-  }
 `;
 
 const StyledCode = styled('code')`
-  white-space: nowrap;
-  @media (min-width: ${p => p.theme.breakpoints[0]}) {
-    white-space: pre-wrap;
-  }
+  white-space: pre-wrap;
   line-height: 26px;
 `;
 

src/sentry/static/sentry/app/components/events/interfaces/breadcrumbsV2/index.tsx

@@ -21,7 +21,7 @@ import {
   BreadcrumbLevelType,
 } from './types';
 import transformCrumbs from './transformCrumbs';
-import Filter from './filter/filter';
+import Filter from './filter';
 import List from './list';
 import Level from './level';
 import Icon from './icon';
@@ -49,6 +49,7 @@ type State = {
   filteredBySearch: BreadcrumbsWithDetails;
   filterOptions: FilterOptions;
   displayRelativeTime: boolean;
+  relativeTime?: string;
 };
 
 class Breadcrumbs extends React.Component<Props, State> {
@@ -77,25 +78,9 @@ class Breadcrumbs extends React.Component<Props, State> {
   listRef = React.createRef<HTMLDivElement>();
 
   expandCollapsedCrumbs() {
-    this.setState(
-      prevState => ({
-        filteredBySearch: prevState.breadcrumbs,
-      }),
-      this.scrollToTheBottom
-    );
-  }
-
-  scrollToTheBottom() {
-    const element = this.listRef?.current;
-
-    if (!element) {
-      return;
-    }
-
-    element.scrollTo({
-      top: element.scrollHeight,
-      left: 0,
-    });
+    this.setState(prevState => ({
+      filteredBySearch: prevState.breadcrumbs,
+    }));
   }
 
   loadBreadcrumbs() {
@@ -108,14 +93,15 @@ class Breadcrumbs extends React.Component<Props, State> {
       breadcrumbs = [...breadcrumbs, virtualCrumb];
     }
 
-    const tranformedCrumbs = transformCrumbs(breadcrumbs);
-    const filterOptions = this.getFilterOptions(tranformedCrumbs);
+    const transformedCrumbs = transformCrumbs(breadcrumbs);
+    const filterOptions = this.getFilterOptions(transformedCrumbs);
 
     this.setState({
-      breadcrumbs: tranformedCrumbs,
-      filteredByFilter: tranformedCrumbs,
-      filteredBySearch: this.getCollapsedBreadcrumbs(tranformedCrumbs),
+      breadcrumbs: transformedCrumbs,
+      filteredByFilter: transformedCrumbs,
+      filteredBySearch: this.getCollapsedBreadcrumbs(transformedCrumbs),
       filterOptions,
+      relativeTime: transformedCrumbs[transformedCrumbs.length - 1]?.timestamp,
     });
   }
 
@@ -381,7 +367,13 @@ class Breadcrumbs extends React.Component<Props, State> {
 
   render() {
     const {type, event, orgId} = this.props;
-    const {filterOptions, searchTerm, filteredBySearch, displayRelativeTime} = this.state;
+    const {
+      filterOptions,
+      searchTerm,
+      filteredBySearch,
+      displayRelativeTime,
+      relativeTime,
+    } = this.state;
 
     return (
       <StyledEventDataSection
@@ -406,12 +398,12 @@ class Breadcrumbs extends React.Component<Props, State> {
         {filteredBySearch.length > 0 ? (
           <List
             breadcrumbs={filteredBySearch}
-            ref={this.listRef}
             event={event}
             orgId={orgId}
             onSwitchTimeFormat={this.handleSwitchTimeFormat}
             displayRelativeTime={displayRelativeTime}
             searchTerm={searchTerm}
+            relativeTime={relativeTime!} // relativeTime has to be always available, as the last item timestamp is the event created time
           />
         ) : (
           <StyledEmptyMessage

src/sentry/static/sentry/app/components/events/interfaces/breadcrumbsV2/list.tsx

@@ -1,57 +1,148 @@
 import React from 'react';
 import styled from '@emotion/styled';
+import {
+  List,
+  ListRowProps,
+  CellMeasurer,
+  CellMeasurerCache,
+  AutoSizer,
+} from 'react-virtualized';
 
-import space from 'app/styles/space';
-
+import {aroundContentStyle} from './styles';
 import ListHeader from './listHeader';
 import ListBody from './listBody';
-import {aroundContentStyle} from './styles';
+import {BreadcrumbsWithDetails} from './types';
+
+const LIST_MAX_HEIGHT = 400;
 
 type Props = {
   onSwitchTimeFormat: () => void;
-} & Omit<React.ComponentProps<typeof ListBody>, 'relativeTime'>;
-
-const List = React.forwardRef(
-  (
-    {
-      displayRelativeTime,
-      onSwitchTimeFormat,
-      orgId,
-      event,
-      breadcrumbs,
-      searchTerm,
-    }: Props,
-    ref: React.Ref<HTMLDivElement>
-  ) => (
-    <Grid ref={ref}>
-      <ListHeader
-        onSwitchTimeFormat={onSwitchTimeFormat}
-        displayRelativeTime={!!displayRelativeTime}
-      />
+  breadcrumbs: BreadcrumbsWithDetails;
+  relativeTime: string;
+} & Omit<React.ComponentProps<typeof ListBody>, 'breadcrumb' | 'isLastItem' | 'column'>;
+
+const cache = new CellMeasurerCache({
+  fixedWidth: true,
+  minHeight: 42,
+});
+
+class ListContainer extends React.Component<Props> {
+  componentDidMount() {
+    this.updateGrid();
+  }
+
+  componentDidUpdate() {
+    this.updateGrid();
+  }
+
+  listRef: List | null = null;
+
+  updateGrid = () => {
+    if (this.listRef) {
+      cache.clearAll();
+      this.listRef.forceUpdateGrid();
+    }
+  };
+
+  renderBody(breadcrumb: BreadcrumbsWithDetails[0], isLastItem = false) {
+    const {event, orgId, searchTerm, relativeTime, displayRelativeTime} = this.props;
+    return (
       <ListBody
+        orgId={orgId}
         searchTerm={searchTerm}
+        breadcrumb={breadcrumb}
         event={event}
-        orgId={orgId}
-        breadcrumbs={breadcrumbs}
-        relativeTime={breadcrumbs[breadcrumbs.length - 1]?.timestamp}
-        displayRelativeTime={!!displayRelativeTime}
+        relativeTime={relativeTime}
+        displayRelativeTime={displayRelativeTime}
+        isLastItem={isLastItem}
       />
-    </Grid>
-  )
-);
+    );
+  }
 
-export default List;
+  renderRow = ({index, key, parent, style}: ListRowProps) => {
+    const {breadcrumbs} = this.props;
+    const breadcrumb = breadcrumbs[index];
+    const isLastItem = breadcrumbs[breadcrumbs.length - 1].id === breadcrumb.id;
+    return (
+      <CellMeasurer
+        cache={cache}
+        columnIndex={0}
+        key={key}
+        parent={parent}
+        rowIndex={index}
+      >
+        {({measure}) =>
+          isLastItem ? (
+            <Row style={style} onLoad={measure} data-test-id="last-crumb">
+              {this.renderBody(breadcrumb, isLastItem)}
+            </Row>
+          ) : (
+            <Row style={style} onLoad={measure}>
+              {this.renderBody(breadcrumb)}
+            </Row>
+          )
+        }
+      </CellMeasurer>
+    );
+  };
 
-const Grid = styled('div')`
-  max-height: 500px;
-  overflow-y: auto;
-  display: grid;
-  > *:nth-last-child(5):before {
-    bottom: calc(100% - ${space(1)});
+  render() {
+    const {breadcrumbs, displayRelativeTime, onSwitchTimeFormat} = this.props;
+
+    // onResize is required in case the user rotates the device.
+    return (
+      <Wrapper>
+        <AutoSizer disableHeight onResize={this.updateGrid}>
+          {({width}) => (
+            <React.Fragment>
+              <Row width={width}>
+                <ListHeader
+                  displayRelativeTime={!!displayRelativeTime}
+                  onSwitchTimeFormat={onSwitchTimeFormat}
+                />
+              </Row>
+              <StyledList
+                ref={(el: List | null) => {
+                  this.listRef = el;
+                }}
+                deferredMeasurementCache={cache}
+                height={LIST_MAX_HEIGHT}
+                overscanRowCount={5}
+                rowCount={breadcrumbs.length}
+                rowHeight={cache.rowHeight}
+                rowRenderer={this.renderRow}
+                width={width}
+                // when the component mounts, it scrolls to the last item
+                scrollToIndex={breadcrumbs.length - 1}
+                scrollToAlignment="end"
+              />
+            </React.Fragment>
+          )}
+        </AutoSizer>
+      </Wrapper>
+    );
   }
-  grid-template-columns: max-content minmax(55px, 1fr) 6fr max-content 65px;
+}
+
+export default ListContainer;
+
+const Wrapper = styled('div')`
+  overflow: hidden;
+  ${aroundContentStyle}
+`;
+
+// it makes the list have a dynamic height; otherwise, in the case of filtered options, a list will be displayed with an empty space
+const StyledList = styled(List)<{height: number}>`
+  height: auto !important;
+  max-height: ${p => p.height}px;
+  overflow-y: auto !important;
+`;
+
+const Row = styled('div')<{width?: number}>`
+  display: grid;
+  grid-template-columns: 45px minmax(55px, 1fr) 6fr 86px 67px;
   @media (min-width: ${p => p.theme.breakpoints[0]}) {
-    grid-template-columns: max-content minmax(132px, 1fr) 6fr max-content max-content;
+    grid-template-columns: 63px minmax(132px, 1fr) 6fr 75px 85px;
   }
-  ${aroundContentStyle}
+  ${p => p.width && `width: ${p.width}px`};
 `;

src/sentry/static/sentry/app/components/events/interfaces/breadcrumbsV2/listBody.tsx

@@ -5,64 +5,68 @@ import {Event} from 'app/types';
 import Tooltip from 'app/components/tooltip';
 import space from 'app/styles/space';
 
-import Time from './time/time';
-import Data from './data/data';
+import Time from './time';
+import Data from './data';
 import Category from './category';
 import Icon from './icon';
 import Level from './level';
 import {GridCell, GridCellLeft} from './styles';
-import {Breadcrumb, BreadcrumbsWithDetails, BreadcrumbType} from './types';
+import {BreadcrumbsWithDetails, BreadcrumbType} from './types';
 
 type Props = {
-  breadcrumbs: BreadcrumbsWithDetails;
+  breadcrumb: BreadcrumbsWithDetails[0];
   event: Event;
   orgId: string | null;
   searchTerm: string;
-  relativeTime?: string;
-  displayRelativeTime?: boolean;
+  isLastItem: boolean;
+  relativeTime: string;
+  displayRelativeTime: boolean;
 };
 
 const ListBody = React.memo(
-  ({orgId, event, breadcrumbs, relativeTime, displayRelativeTime, searchTerm}: Props) => (
-    <React.Fragment>
-      {breadcrumbs.map(({color, icon, id, ...crumb}, idx) => {
-        const hasError = crumb.type === BreadcrumbType.ERROR;
-        const isLastItem = breadcrumbs.length - 1 === idx;
+  ({
+    orgId,
+    event,
+    breadcrumb,
+    relativeTime,
+    displayRelativeTime,
+    searchTerm,
+    isLastItem,
+  }: Props) => {
+    const hasError = breadcrumb.type === BreadcrumbType.ERROR;
 
-        return (
-          <React.Fragment key={id}>
-            <GridCellLeft hasError={hasError} isLastItem={isLastItem}>
-              <Tooltip title={crumb.description}>
-                <Icon icon={icon} color={color} />
-              </Tooltip>
-            </GridCellLeft>
-            <GridCellCategory hasError={hasError} isLastItem={isLastItem}>
-              <Category category={crumb?.category} searchTerm={searchTerm} />
-            </GridCellCategory>
-            <GridCell hasError={hasError} isLastItem={isLastItem}>
-              <Data
-                event={event}
-                orgId={orgId}
-                breadcrumb={crumb as Breadcrumb}
-                searchTerm={searchTerm}
-              />
-            </GridCell>
-            <GridCell hasError={hasError} isLastItem={isLastItem}>
-              <Level level={crumb.level} searchTerm={searchTerm} />
-            </GridCell>
-            <GridCell hasError={hasError} isLastItem={isLastItem}>
-              <Time
-                timestamp={crumb?.timestamp}
-                relativeTime={relativeTime}
-                displayRelativeTime={displayRelativeTime}
-                searchTerm={searchTerm}
-              />
-            </GridCell>
-          </React.Fragment>
-        );
-      })}
-    </React.Fragment>
-  )
+    return (
+      <React.Fragment>
+        <GridCellLeft hasError={hasError} isLastItem={isLastItem}>
+          <Tooltip title={breadcrumb.description}>
+            <Icon icon={breadcrumb.icon} color={breadcrumb.color} />
+          </Tooltip>
+        </GridCellLeft>
+        <GridCellCategory hasError={hasError} isLastItem={isLastItem}>
+          <Category category={breadcrumb?.category} searchTerm={searchTerm} />
+        </GridCellCategory>
+        <GridCell hasError={hasError} isLastItem={isLastItem}>
+          <Data
+            event={event}
+            orgId={orgId}
+            breadcrumb={breadcrumb}
+            searchTerm={searchTerm}
+          />
+        </GridCell>
+        <GridCell hasError={hasError} isLastItem={isLastItem}>
+          <Level level={breadcrumb.level} searchTerm={searchTerm} />
+        </GridCell>
+        <GridCell hasError={hasError} isLastItem={isLastItem}>
+          <Time
+            timestamp={breadcrumb?.timestamp}
+            relativeTime={relativeTime}
+            displayRelativeTime={displayRelativeTime}
+            searchTerm={searchTerm}
+          />
+        </GridCell>
+      </React.Fragment>
+    );
+  }
 );
 
 export default ListBody;

src/sentry/static/sentry/app/components/events/interfaces/breadcrumbsV2/styles.tsx

@@ -34,11 +34,11 @@ const GridCell = styled('div')<{
   hasError?: boolean;
   isLastItem?: boolean;
 }>`
+  height: 100%;
   position: relative;
+  white-space: pre-wrap;
+  word-break: break-all;
   border-bottom: 1px solid ${p => p.theme.borderLight};
-  margin-bottom: -1px;
-  text-overflow: ellipsis;
-  overflow: hidden;
   padding: ${space(1)};
   @media (min-width: ${p => p.theme.breakpoints[0]}) {
     padding: ${space(1)} ${space(2)};
@@ -47,9 +47,16 @@ const GridCell = styled('div')<{
     p.hasError &&
     `
       background: #fffcfb;
-      border-top: 1px solid #fa4747;
-      border-bottom: 1px solid #fa4747;
-      z-index: ${p.theme.zIndex.breadcrumbs.gridCellError};
+      border-bottom: 1px solid ${p.theme.red400};
+      :after {
+        content: '';
+        position: absolute;
+        top: -1px;
+        left: 0;
+        height: 1px;
+        width: 100%;
+        background: ${p.theme.red400};
+      }
     `}
   ${p => p.isLastItem && `border-bottom: none`};
 `;
@@ -65,7 +72,7 @@ const GridCellLeft = styled(GridCell)`
     top: 0;
     bottom: 0;
     left: 21px;
-    background: ${p => (p.hasError ? '#fa4747' : p.theme.gray300)};
+    background: ${p => (p.hasError ? p.theme.red400 : p.theme.gray300)};
     position: absolute;
     @media (min-width: ${p => p.theme.breakpoints[0]}) {
       left: 29px;
@@ -77,6 +84,7 @@ const aroundContentStyle = css`
   border: 1px solid ${theme.borderDark};
   border-radius: ${theme.borderRadius};
   box-shadow: ${theme.dropShadowLightest};
+  z-index: 1;
 `;
 
 export {GridCell, GridCellLeft, IconWrapper, aroundContentStyle};

tests/acceptance/test_issue_details.py

@@ -84,6 +84,7 @@ class IssueDetailsTest(AcceptanceTestCase, SnubaTestCase):
         with self.feature("organizations:breadcrumbs-v2"):
             event = self.create_sample_event(platform="cocoa")
             self.page.visit_issue(self.org.slug, event.group.id)
+            self.browser.wait_until_test_id("last-crumb")
             self.browser.snapshot("issue details cocoa - breadcrumbs-v2")
 
     def test_unity_event(self):
@@ -95,6 +96,7 @@ class IssueDetailsTest(AcceptanceTestCase, SnubaTestCase):
         with self.feature("organizations:breadcrumbs-v2"):
             event = self.create_sample_event(default="unity", platform="csharp")
             self.page.visit_issue(self.org.slug, event.group.id)
+            self.browser.wait_until_test_id("last-crumb")
             self.browser.snapshot("issue details unity - breadcrumbs v2")
 
     def test_android_event(self):