Browse Source

Revert "fix(breadcrumbs): Fix flicking on scroll from bottom to top - (#41055)"

This reverts commit a9e65027c2975d39e5f70c1e69fbd6d13e4a0dee.

Co-authored-by: priscilawebdev <29228205+priscilawebdev@users.noreply.github.com>
getsentry-bot 2 years ago
parent
commit
5c05c223a5

+ 10 - 2
static/app/components/events/eventEntries.tsx

@@ -90,7 +90,7 @@ function hasThreadOrExceptionMinifiedFrameData(definedEvent: Event, bestThread?:
 
 type ProGuardErrors = Array<Error>;
 
-type Props = {
+type Props = Pick<React.ComponentProps<typeof EventEntry>, 'route' | 'router'> & {
   api: Client;
   location: Location;
   /**
@@ -113,6 +113,8 @@ const EventEntries = ({
   event,
   group,
   className,
+  router,
+  route,
   isShare = false,
   showTagSummary = true,
 }: Props) => {
@@ -385,6 +387,8 @@ const EventEntries = ({
         projectSlug={projectSlug}
         group={group}
         organization={organization}
+        route={route}
+        router={router}
         isShare={isShare}
       />
       {hasContext && <EventContexts group={group} event={event} />}
@@ -474,11 +478,13 @@ function Entries({
   isShare,
   group,
   organization,
+  route,
+  router,
 }: {
   definedEvent: Event;
   projectSlug: string;
   isShare?: boolean;
-} & Pick<Props, 'group' | 'organization'>) {
+} & Pick<Props, 'group' | 'organization' | 'route' | 'router'>) {
   if (!Array.isArray(definedEvent.entries)) {
     return null;
   }
@@ -507,6 +513,8 @@ function Entries({
             organization={organization}
             event={definedEvent}
             entry={entry}
+            route={route}
+            router={router}
             isShare={isShare}
           />
         </ErrorBoundary>

+ 14 - 2
static/app/components/events/eventEntry.tsx

@@ -25,7 +25,7 @@ import {Entry, EntryType, Event, EventTransaction} from 'sentry/types/event';
 import {Resources} from './interfaces/performance/resources';
 import {getResourceDescription, getResourceLinks} from './interfaces/performance/utils';
 
-type Props = {
+type Props = Pick<React.ComponentProps<typeof Breadcrumbs>, 'route' | 'router'> & {
   entry: Entry;
   event: Event;
   organization: SharedViewOrganization | Organization;
@@ -34,7 +34,16 @@ type Props = {
   isShare?: boolean;
 };
 
-function EventEntry({entry, projectSlug, event, organization, group, isShare}: Props) {
+function EventEntry({
+  entry,
+  projectSlug,
+  event,
+  organization,
+  group,
+  isShare,
+  route,
+  router,
+}: Props) {
   const hasHierarchicalGrouping =
     !!organization.features?.includes('grouping-stacktrace-ui') &&
     !!(event.metadata.current_tree_label || event.metadata.finest_tree_label);
@@ -110,7 +119,10 @@ function EventEntry({entry, projectSlug, event, organization, group, isShare}: P
       return (
         <Breadcrumbs
           data={entry.data}
+          organization={organization as Organization}
           event={event}
+          router={router}
+          route={route}
           isShare={isShare}
           projectSlug={projectSlug}
         />

+ 28 - 0
static/app/components/events/interfaces/breadcrumbs/breadcrumb/category.tsx

@@ -0,0 +1,28 @@
+import {memo} from 'react';
+import styled from '@emotion/styled';
+
+import Highlight from 'sentry/components/highlight';
+import {t} from 'sentry/locale';
+import {defined} from 'sentry/utils';
+
+type Props = {
+  searchTerm: string;
+  category?: string | null;
+};
+
+const Category = memo(function Category({category, searchTerm}: Props) {
+  const title = !defined(category) ? t('generic') : category;
+  return (
+    <Wrapper title={title}>
+      <Highlight text={searchTerm}>{title}</Highlight>
+    </Wrapper>
+  );
+});
+
+export default Category;
+
+const Wrapper = styled('div')`
+  color: ${p => p.theme.textColor};
+  font-size: ${p => p.theme.fontSizeSmall};
+  font-weight: 700;
+`;

+ 16 - 3
static/app/components/events/interfaces/breadcrumbs/breadcrumb/data/index.tsx

@@ -7,7 +7,7 @@ import {Exception} from './exception';
 import {Http} from './http';
 import {LinkedEvent} from './linkedEvent';
 
-type Props = {
+type Props = Pick<React.ComponentProps<typeof LinkedEvent>, 'route' | 'router'> & {
   breadcrumb: RawCrumb;
   event: Event;
   organization: Organization;
@@ -15,13 +15,26 @@ type Props = {
   meta?: Record<any, any>;
 };
 
-export function Data({breadcrumb, event, organization, searchTerm, meta}: Props) {
+export function Data({
+  breadcrumb,
+  event,
+  organization,
+  searchTerm,
+  meta,
+  route,
+  router,
+}: Props) {
   const orgSlug = organization.slug;
 
   const linkedEvent =
     !!organization.features?.includes('breadcrumb-linked-event') &&
     breadcrumb.event_id ? (
-      <LinkedEvent orgSlug={orgSlug} eventId={breadcrumb.event_id} />
+      <LinkedEvent
+        orgSlug={orgSlug}
+        eventId={breadcrumb.event_id}
+        route={route}
+        router={router}
+      />
     ) : undefined;
 
   if (breadcrumb.type === BreadcrumbType.HTTP) {

+ 23 - 21
static/app/components/events/interfaces/breadcrumbs/breadcrumb/data/linkedEvent.tsx

@@ -1,4 +1,5 @@
-import {useCallback, useEffect, useState} from 'react';
+import {useEffect, useState} from 'react';
+import {InjectedRouter, PlainRoute} from 'react-router';
 import styled from '@emotion/styled';
 
 import {addErrorMessage} from 'sentry/actionCreators/indicator';
@@ -10,7 +11,6 @@ import space from 'sentry/styles/space';
 import {EventIdResponse, Group, Organization, Project} from 'sentry/types';
 import handleXhrErrorResponse from 'sentry/utils/handleXhrErrorResponse';
 import useApi from 'sentry/utils/useApi';
-import {useRouteContext} from 'sentry/utils/useRouteContext';
 import useSessionStorage from 'sentry/utils/useSessionStorage';
 
 type StoredLinkedEvent = {
@@ -23,23 +23,37 @@ type StoredLinkedEvent = {
 type Props = {
   eventId: string;
   orgSlug: Organization['slug'];
+  route: PlainRoute;
+  router: InjectedRouter;
 };
 
 const errorMessage = t(
   'An error occurred while fetching the data of the breadcrumb event link'
 );
 
-export function LinkedEvent({orgSlug, eventId}: Props) {
+export function LinkedEvent({orgSlug, eventId, route, router}: Props) {
   const [storedLinkedEvent, setStoredLinkedEvent, removeStoredLinkedEvent] =
     useSessionStorage<undefined | StoredLinkedEvent>(eventId);
 
   const [eventIdLookup, setEventIdLookup] = useState<undefined | EventIdResponse>();
   const [hasError, setHasError] = useState(false);
-  const routeContext = useRouteContext();
 
   const api = useApi();
 
-  const fetchEventById = useCallback(async () => {
+  useEffect(() => {
+    fetchEventById();
+    router.setRouteLeaveHook(route, onRouteLeave);
+  }, []);
+
+  useEffect(() => {
+    fetchIssueByGroupId();
+  }, [eventIdLookup]);
+
+  function onRouteLeave() {
+    removeStoredLinkedEvent();
+  }
+
+  async function fetchEventById() {
     if (storedLinkedEvent) {
       return;
     }
@@ -57,22 +71,14 @@ export function LinkedEvent({orgSlug, eventId}: Props) {
         return;
       }
 
+      addErrorMessage(errorMessage);
       handleXhrErrorResponse(errorMessage)(error);
 
       // do nothing. The link won't be displayed
     }
-  }, [storedLinkedEvent, api, eventId, orgSlug]);
-
-  const onRouteLeave = useCallback(() => {
-    removeStoredLinkedEvent();
-  }, [removeStoredLinkedEvent]);
-
-  useEffect(() => {
-    fetchEventById();
-    routeContext.router.setRouteLeaveHook(routeContext, onRouteLeave);
-  }, [fetchEventById, routeContext, onRouteLeave]);
+  }
 
-  const fetchIssueByGroupId = useCallback(async () => {
+  async function fetchIssueByGroupId() {
     if (!!storedLinkedEvent || !eventIdLookup) {
       return;
     }
@@ -97,11 +103,7 @@ export function LinkedEvent({orgSlug, eventId}: Props) {
 
       // do nothing. The link won't be displayed
     }
-  }, [api, orgSlug, eventIdLookup, storedLinkedEvent, setStoredLinkedEvent]);
-
-  useEffect(() => {
-    fetchIssueByGroupId();
-  }, [fetchIssueByGroupId]);
+  }
 
   if (hasError) {
     return null;

+ 23 - 32
static/app/components/events/interfaces/breadcrumbs/breadcrumb/index.tsx

@@ -1,24 +1,25 @@
+import {memo} from 'react';
 import {css} from '@emotion/react';
 import styled from '@emotion/styled';
 
-import Highlight from 'sentry/components/highlight';
-import {t} from 'sentry/locale';
 import space from 'sentry/styles/space';
+import {Organization} from 'sentry/types';
 import {BreadcrumbType, Crumb} from 'sentry/types/breadcrumbs';
 import {Event} from 'sentry/types/event';
-import {defined} from 'sentry/utils';
-import useOrganization from 'sentry/utils/useOrganization';
 
+import Category from './category';
 import {Data} from './data';
-import {Level} from './level';
+import Level from './level';
 import Time from './time';
-import {Type} from './type';
+import Type from './type';
 
-type Props = {
+type Props = Pick<React.ComponentProps<typeof Data>, 'route' | 'router'> & {
   breadcrumb: Crumb;
+  ['data-test-id']: string;
   displayRelativeTime: boolean;
   event: Event;
   onLoad: () => void;
+  organization: Organization;
   relativeTime: string;
   scrollbarSize: number;
   searchTerm: string;
@@ -27,7 +28,8 @@ type Props = {
   meta?: Record<any, any>;
 };
 
-export function Breadcrumb({
+export const Breadcrumb = memo(function Breadcrumb({
+  organization,
   event,
   breadcrumb,
   relativeTime,
@@ -36,53 +38,48 @@ export function Breadcrumb({
   onLoad,
   scrollbarSize,
   style,
+  route,
+  router,
   meta,
-  ...props
+  ['data-test-id']: dataTestId,
 }: Props) {
-  const organization = useOrganization();
+  const {type, description, color, level, category, timestamp} = breadcrumb;
   const error = breadcrumb.type === BreadcrumbType.ERROR;
-  const category = !defined(breadcrumb.category) ? t('generic') : breadcrumb.category;
 
   return (
     <Wrapper
-      {...props}
       style={style}
       error={error}
       onLoad={onLoad}
+      data-test-id={dataTestId}
       scrollbarSize={scrollbarSize}
     >
-      <Type
-        type={breadcrumb.type}
-        color={breadcrumb.color}
-        description={breadcrumb.description}
-        error={error}
-      />
-      <Category title={category}>
-        <Highlight text={searchTerm}>{category}</Highlight>
-      </Category>
+      <Type type={type} color={color} description={description} error={error} />
+      <Category category={category} searchTerm={searchTerm} />
       <Data
         event={event}
         organization={organization}
         breadcrumb={breadcrumb}
         searchTerm={searchTerm}
+        route={route}
+        router={router}
         meta={meta}
       />
       <div>
-        <Level level={breadcrumb.level} searchTerm={searchTerm} />
+        <Level level={level} searchTerm={searchTerm} />
       </div>
       <Time
-        timestamp={breadcrumb.timestamp}
+        timestamp={timestamp}
         relativeTime={relativeTime}
         displayRelativeTime={displayRelativeTime}
         searchTerm={searchTerm}
       />
     </Wrapper>
   );
-}
+});
 
 const Wrapper = styled('div')<{error: boolean; scrollbarSize: number}>`
   display: grid;
-  transform: scaleY(-1);
   grid-template-columns: 64px 140px 1fr 106px 100px ${p => p.scrollbarSize}px;
 
   > * {
@@ -131,7 +128,7 @@ const Wrapper = styled('div')<{error: boolean; scrollbarSize: number}>`
   word-break: break-all;
   white-space: pre-wrap;
   :not(:last-child) {
-    border-top: 1px solid ${p => (p.error ? p.theme.red300 : p.theme.innerBorder)};
+    border-bottom: 1px solid ${p => (p.error ? p.theme.red300 : p.theme.innerBorder)};
   }
 
   ${p =>
@@ -148,9 +145,3 @@ const Wrapper = styled('div')<{error: boolean; scrollbarSize: number}>`
       }
     `}
 `;
-
-const Category = styled('div')`
-  color: ${p => p.theme.textColor};
-  font-size: ${p => p.theme.fontSizeSmall};
-  font-weight: 700;
-`;

+ 5 - 2
static/app/components/events/interfaces/breadcrumbs/breadcrumb/level.tsx

@@ -1,3 +1,4 @@
+import {memo} from 'react';
 import styled from '@emotion/styled';
 
 import Highlight from 'sentry/components/highlight';
@@ -10,7 +11,7 @@ type Props = {
   searchTerm?: string;
 };
 
-export function Level({level, searchTerm = ''}: Props) {
+const Level = memo(function Level({level, searchTerm = ''}: Props) {
   switch (level) {
     case BreadcrumbLevelType.FATAL:
       return (
@@ -43,7 +44,9 @@ export function Level({level, searchTerm = ''}: Props) {
         </LevelTag>
       );
   }
-}
+});
+
+export default Level;
 
 const LevelTag = styled(Tag)`
   display: flex;

+ 8 - 2
static/app/components/events/interfaces/breadcrumbs/breadcrumb/time/index.tsx

@@ -1,3 +1,4 @@
+import {memo} from 'react';
 import styled from '@emotion/styled';
 
 import Highlight from 'sentry/components/highlight';
@@ -15,7 +16,12 @@ type Props = {
   timestamp?: string;
 };
 
-export function Time({timestamp, relativeTime, displayRelativeTime, searchTerm}: Props) {
+const Time = memo(function Time({
+  timestamp,
+  relativeTime,
+  displayRelativeTime,
+  searchTerm,
+}: Props) {
   if (!(defined(timestamp) && defined(relativeTime))) {
     return <div />;
   }
@@ -46,7 +52,7 @@ export function Time({timestamp, relativeTime, displayRelativeTime, searchTerm}:
       </Tooltip>
     </Wrapper>
   );
-}
+});
 
 export default Time;
 

+ 3 - 1
static/app/components/events/interfaces/breadcrumbs/breadcrumb/type/index.tsx

@@ -11,7 +11,7 @@ type Props = Required<Pick<SVGIconProps, 'color'>> &
     error?: boolean;
   };
 
-export function Type({type, color, description, error}: Props) {
+function Type({type, color, description, error}: Props) {
   return (
     <Wrapper error={error}>
       <Tooltip
@@ -28,6 +28,8 @@ export function Type({type, color, description, error}: Props) {
   );
 }
 
+export default Type;
+
 const Wrapper = styled('div')<Pick<Props, 'error'>>`
   display: flex;
   justify-content: center;

+ 8 - 5
static/app/components/events/interfaces/breadcrumbs/breadcrumbs.spec.tsx

@@ -1,3 +1,4 @@
+import {initializeOrg} from 'sentry-test/initializeOrg';
 import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary';
 import {textWithMarkupMatcher} from 'sentry-test/utils';
 
@@ -30,9 +31,13 @@ jest.mock('sentry/utils/replays/hooks/useReplayData', () => {
 
 describe('Breadcrumbs', () => {
   let props: React.ComponentProps<typeof Breadcrumbs>;
+  const {router} = initializeOrg();
 
   beforeEach(() => {
     props = {
+      route: {},
+      router,
+      organization: TestStubs.Organization(),
       projectSlug: 'project-slug',
       isShare: false,
       event: TestStubs.Event({entries: []}),
@@ -182,12 +187,10 @@ describe('Breadcrumbs', () => {
             entries: [],
             tags: [{key: 'replayId', value: '761104e184c64d439ee1014b72b4d83b'}],
           })}
-        />,
-        {
-          organization: TestStubs.Organization({
+          organization={TestStubs.Organization({
             features: ['session-replay-ui'],
-          }),
-        }
+          })}
+        />
       );
 
       await waitFor(() => {

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