Browse Source

feat(replays): Allow more components to render w/o data loaded (#34566)

This allows more components to render (with placeholders) without any
data.

![image](https://user-images.githubusercontent.com/79684/168150198-b7035e8f-9c8e-41b4-8651-58f31da9eed8.png)
Billy Vong 2 years ago
parent
commit
7efaf75cb6

+ 2 - 1
static/app/components/replays/breadcrumbs/replayTimeline.tsx

@@ -4,6 +4,7 @@ import styled from '@emotion/styled';
 
 import {transformCrumbs} from 'sentry/components/events/interfaces/breadcrumbs/utils';
 import {Panel} from 'sentry/components/panels';
+import Placeholder from 'sentry/components/placeholder';
 import {
   MajorGridlines,
   MinorGridlines,
@@ -32,7 +33,7 @@ function ReplayTimeline({}: Props) {
   const {currentHoverTime, currentTime, duration, replay} = useReplayContext();
 
   if (!replay) {
-    return null;
+    return <Placeholder height="86px" bottomGutter={2} />;
   }
 
   const crumbs = replay.getRawCrumbs() || [];

+ 13 - 6
static/app/components/replays/replayContext.tsx

@@ -130,7 +130,7 @@ const ReplayPlayerContext = React.createContext<ReplayPlayerContextProps>({
 
 type Props = {
   children: React.ReactNode;
-  replay: ReplayReader;
+  replay: ReplayReader | null;
 
   /**
    * Time, in seconds, when the video should start
@@ -150,10 +150,12 @@ function useCurrentTime(callback: () => number) {
 }
 
 export function Provider({children, replay, initialTimeOffset = 0, value = {}}: Props) {
-  const events = replay.getRRWebEvents();
+  const events = replay?.getRRWebEvents();
 
   const theme = useTheme();
   const oldEvents = usePrevious(events);
+  // Note we have to check this outside of hooks, see `usePrevious` comments
+  const hasNewEvents = events !== oldEvents;
   const replayerRef = useRef<Replayer>(null);
   const [dimensions, setDimensions] = useState<Dimensions>({height: 0, width: 0});
   const [currentHoverTime, setCurrentHoverTime] = useState<undefined | number>();
@@ -188,11 +190,16 @@ export function Provider({children, replay, initialTimeOffset = 0, value = {}}:
       }
 
       if (replayerRef.current) {
-        if (events === oldEvents) {
+        if (!hasNewEvents) {
           // Already have a player for these events, the parent node must've re-rendered
           return;
         }
 
+        if (replayerRef.current.iframe.contentDocument?.body.childElementCount === 0) {
+          // If this is true, then no need to clear old iframe as nothing was rendered
+          return;
+        }
+
         // We have new events, need to clear out the old iframe because a new
         // `Replayer` instance is about to be created
         while (root.firstChild) {
@@ -231,7 +238,7 @@ export function Provider({children, replay, initialTimeOffset = 0, value = {}}:
       // @ts-expect-error
       replayerRef.current = inst;
     },
-    [events, oldEvents, theme.purple200]
+    [events, theme.purple200, hasNewEvents]
   );
 
   useEffect(() => {
@@ -340,8 +347,8 @@ export function Provider({children, replay, initialTimeOffset = 0, value = {}}:
     setBufferTime({target: -1, previous: -1});
   }
 
-  const event = replay.getEvent();
-  const duration = (event.endTimestamp - event.startTimestamp) * 1000;
+  const event = replay?.getEvent();
+  const duration = event ? (event.endTimestamp - event.startTimestamp) * 1000 : -1;
 
   return (
     <ReplayPlayerContext.Provider

+ 6 - 2
static/app/utils/replays/getCurrentUserAction.tsx

@@ -2,10 +2,14 @@ import {relativeTimeInMs} from 'sentry/components/replays/utils';
 import {Crumb} from 'sentry/types/breadcrumbs';
 
 export function getCurrentUserAction(
-  userActionCrumbs: Crumb[],
-  startTimestamp: number,
+  userActionCrumbs: Crumb[] | undefined,
+  startTimestamp: number | undefined,
   currentHoverTime: number
 ) {
+  if (!startTimestamp || !userActionCrumbs) {
+    return undefined;
+  }
+
   return userActionCrumbs.reduce((prev, curr) => {
     return Math.abs(
       relativeTimeInMs(curr.timestamp ?? '', startTimestamp) - currentHoverTime

+ 12 - 7
static/app/views/replays/detail/focusArea.tsx

@@ -1,6 +1,7 @@
 import {useMemo} from 'react';
 
 import EventEntry from 'sentry/components/events/eventEntry';
+import Placeholder from 'sentry/components/placeholder';
 import {useReplayContext} from 'sentry/components/replays/replayContext';
 import TagsTable from 'sentry/components/tagsTable';
 import type {Entry, Event} from 'sentry/types/event';
@@ -16,7 +17,7 @@ import Trace from './trace';
 import useActiveTabFromLocation from './useActiveTabFromLocation';
 
 type Props = {
-  replay: ReplayReader;
+  replay: ReplayReader | null;
 };
 
 function getBreadcrumbsByCategory(breadcrumbEntry: Entry, categories: string[]) {
@@ -32,18 +33,22 @@ function FocusArea({replay}: Props) {
     useReplayContext();
   const organization = useOrganization();
 
-  const event = replay.getEvent();
-  const spansEntry = replay.getEntryType(EntryType.SPANS);
-
   // Memoize this because re-renders will interfere with the mouse state of the
   // chart (e.g. on mouse over and out)
   const memorySpans = useMemo(() => {
-    return replay.getRawSpans().filter(replay.isMemorySpan);
+    return replay?.getRawSpans().filter(replay.isMemorySpan);
   }, [replay]);
 
+  if (!replay || !memorySpans) {
+    return <Placeholder height="150px" />;
+  }
+
+  const event = replay.getEvent();
+  const spansEntry = replay.getEntryType(EntryType.SPANS);
+
   switch (active) {
     case 'console':
-      const breadcrumbEntry = replay.getEntryType(EntryType.BREADCRUMBS);
+      const breadcrumbEntry = replay?.getEntryType(EntryType.BREADCRUMBS);
       const consoleMessages = getBreadcrumbsByCategory(breadcrumbEntry, [
         'console',
         'error',
@@ -102,7 +107,7 @@ function FocusArea({replay}: Props) {
           memorySpans={memorySpans}
           setCurrentTime={setCurrentTime}
           setCurrentHoverTime={setCurrentHoverTime}
-          startTimestamp={event.startTimestamp}
+          startTimestamp={event?.startTimestamp}
         />
       );
     default:

+ 61 - 37
static/app/views/replays/detail/userActionsNavigator.tsx

@@ -1,4 +1,4 @@
-import React, {useCallback, useEffect, useState} from 'react';
+import {Fragment, useCallback, useEffect, useState} from 'react';
 import styled from '@emotion/styled';
 
 import Type from 'sentry/components/events/interfaces/breadcrumbs/breadcrumb/type';
@@ -12,6 +12,7 @@ import {
   PanelHeader as BasePanelHeader,
   PanelItem,
 } from 'sentry/components/panels';
+import Placeholder from 'sentry/components/placeholder';
 import ActionCategory from 'sentry/components/replays/actionCategory';
 import PlayerRelativeTime from 'sentry/components/replays/playerRelativeTime';
 import {useReplayContext} from 'sentry/components/replays/replayContext';
@@ -22,9 +23,25 @@ import {Crumb, RawCrumb} from 'sentry/types/breadcrumbs';
 import {EventTransaction} from 'sentry/types/event';
 import {getCurrentUserAction} from 'sentry/utils/replays/getCurrentUserAction';
 
+function CrumbPlaceholder({number}: {number: number}) {
+  return (
+    <Fragment>
+      {[...Array(number)].map((_, i) => (
+        <PlaceholderMargin key={i} height="40px" />
+      ))}
+    </Fragment>
+  );
+}
+
 type Props = {
-  crumbs: RawCrumb[];
-  event: EventTransaction;
+  /**
+   * Raw breadcrumbs, `undefined` means it is still loading
+   */
+  crumbs: RawCrumb[] | undefined;
+  /**
+   * Root replay event, `undefined` means it is still loading
+   */
+  event: EventTransaction | undefined;
 };
 
 type ContainerProps = {
@@ -37,8 +54,9 @@ function UserActionsNavigator({event, crumbs}: Props) {
   const [currentUserAction, setCurrentUserAction] = useState<Crumb>();
   const [closestUserAction, setClosestUserAction] = useState<Crumb>();
 
-  const {startTimestamp} = event;
-  const userActionCrumbs = onlyUserActions(transformCrumbs(crumbs));
+  const {startTimestamp} = event || {};
+  const userActionCrumbs = crumbs && onlyUserActions(transformCrumbs(crumbs));
+  const isLoaded = userActionCrumbs && startTimestamp;
 
   const getClosestUserAction = useCallback(
     async (hovertime: number) => {
@@ -47,7 +65,10 @@ function UserActionsNavigator({event, crumbs}: Props) {
         startTimestamp,
         hovertime
       );
-      if (closestUserAction?.timestamp !== closestUserActionItem.timestamp) {
+      if (
+        closestUserActionItem &&
+        closestUserAction?.timestamp !== closestUserActionItem.timestamp
+      ) {
         setClosestUserAction(closestUserActionItem);
       }
     },
@@ -62,44 +83,42 @@ function UserActionsNavigator({event, crumbs}: Props) {
     getClosestUserAction(currentHoverTime);
   }, [getClosestUserAction, currentHoverTime]);
 
-  if (!event) {
-    return null;
-  }
-
   return (
     <Panel>
       <PanelHeader>{t('Event Chapters')}</PanelHeader>
 
       <PanelBody>
-        {userActionCrumbs.map(item => (
-          <PanelItemCenter
-            key={item.id}
-            onClick={() => {
-              setCurrentUserAction(item);
-              item.timestamp
-                ? setCurrentTime(relativeTimeInMs(item.timestamp, startTimestamp))
-                : '';
-            }}
-          >
-            <Container
-              isHovered={closestUserAction?.id === item.id}
-              isSelected={currentUserAction?.id === item.id}
+        {!isLoaded && <CrumbPlaceholder number={4} />}
+        {isLoaded &&
+          userActionCrumbs.map(item => (
+            <PanelItemCenter
+              key={item.id}
+              onClick={() => {
+                setCurrentUserAction(item);
+                item.timestamp
+                  ? setCurrentTime(relativeTimeInMs(item.timestamp, startTimestamp))
+                  : '';
+              }}
             >
-              <Wrapper>
-                <Type
-                  type={item.type}
-                  color={item.color}
-                  description={item.description}
+              <Container
+                isHovered={closestUserAction?.id === item.id}
+                isSelected={currentUserAction?.id === item.id}
+              >
+                <Wrapper>
+                  <Type
+                    type={item.type}
+                    color={item.color}
+                    description={item.description}
+                  />
+                  <ActionCategory category={item} />
+                </Wrapper>
+                <PlayerRelativeTime
+                  relativeTime={startTimestamp}
+                  timestamp={item.timestamp}
                 />
-                <ActionCategory category={item} />
-              </Wrapper>
-              <PlayerRelativeTime
-                relativeTime={startTimestamp}
-                timestamp={item.timestamp}
-              />
-            </Container>
-          </PanelItemCenter>
-        ))}
+              </Container>
+            </PanelItemCenter>
+          ))}
       </PanelBody>
     </Panel>
   );
@@ -160,4 +179,9 @@ const Wrapper = styled('div')`
   color: ${p => p.theme.gray500};
 `;
 
+const PlaceholderMargin = styled(Placeholder)`
+  margin: ${space(1)} ${space(1.5)};
+  width: auto;
+`;
+
 export default UserActionsNavigator;

+ 7 - 14
static/app/views/replays/details.tsx

@@ -4,7 +4,6 @@ import styled from '@emotion/styled';
 import DetailedError from 'sentry/components/errors/detailedError';
 import NotFound from 'sentry/components/errors/notFound';
 import * as Layout from 'sentry/components/layouts/thirds';
-import LoadingIndicator from 'sentry/components/loadingIndicator';
 import ReplayTimeline from 'sentry/components/replays/breadcrumbs/replayTimeline';
 import {Provider as ReplayContextProvider} from 'sentry/components/replays/replayContext';
 import ReplayView from 'sentry/components/replays/replayView';
@@ -37,14 +36,7 @@ function ReplayDetails() {
 
   const {ref: fullscreenRef, isFullscreen, toggle: toggleFullscreen} = useFullscreen();
 
-  if (fetching) {
-    return (
-      <DetailLayout orgId={orgId}>
-        <LoadingIndicator />
-      </DetailLayout>
-    );
-  }
-  if (!replay) {
+  if (!fetching && !replay) {
     // TODO(replay): Give the user more details when errors happen
     console.log({fetching, fetchError}); // eslint-disable-line no-console
     return (
@@ -55,7 +47,8 @@ function ReplayDetails() {
       </DetailLayout>
     );
   }
-  if (replay.getRRWebEvents().length < 2) {
+
+  if (!fetching && replay && replay.getRRWebEvents().length < 2) {
     return (
       <DetailLayout event={replay.getEvent()} orgId={orgId}>
         <DetailedError
@@ -80,9 +73,9 @@ function ReplayDetails() {
   return (
     <ReplayContextProvider replay={replay} initialTimeOffset={initialTimeOffset}>
       <DetailLayout
-        event={replay.getEvent()}
+        event={replay?.getEvent()}
         orgId={orgId}
-        crumbs={replay.getRawCrumbs()}
+        crumbs={replay?.getRawCrumbs()}
       >
         <Layout.Body>
           <Layout.Main ref={fullscreenRef}>
@@ -91,8 +84,8 @@ function ReplayDetails() {
 
           <Layout.Side>
             <UserActionsNavigator
-              crumbs={replay.getRawCrumbs()}
-              event={replay.getEvent()}
+              crumbs={replay?.getRawCrumbs()}
+              event={replay?.getEvent()}
             />
           </Layout.Side>