Browse Source

ref(replay): Use formatDuration directly, or through `<Duration>` in replay (#76711)

This replaces `formatReplayDuration` with calls to the more general
`formatDuration` and creates a new helper `<Duration/>` to make things
easier inside a react tree.

Basically before, we had a weird function signature that wasn't as
obvious to use. Now we have the `duration: Duration, precision: Unit`
params that can be passed through the `<Duration/>` helper, or passed
through ` <TimestampButton>` so it's obvious at the callsite what the
output will be.

For formatting we don't need to check user preferences. The only
time-related preferences are: 24-hour time & local-vs-UTC time, neither
apply to this.

The `components/duration/duration.tsx` helper is, imo, improved over the
previous `components/duration.tsx` because the props are much clearer to
use. Removing the old one is going to be a next step.


List of places to check/changed:
- Replay Timeline 'current time' and 'duration', no UI change (sec
precision)
- Hover over the Timeline and the timestamp now shows ms precision,
before was second precision
- Replay Breadcrumbs tab, no UI change (sec precision)
- Replay Console tab, right column, no UI change (sec precision)
- Replay Network tab, right column, no UI change (ms precision)
- Replay Error tab, right column, no UI change (sec precision)
- Replay A11y tab, "Results as of ..." & "Run Validation ...", no UI
change (sec precision)
- Replay Memory tab, chart axes, no UI change (sec precision)
- Replay Memory tab, hover updated, shows ms precision, before was
second precision
- Replay List page, duration column, no UI change (sec precision)
Ryan Albrecht 6 months ago
parent
commit
8240028ee9

+ 29 - 0
static/app/components/duration/duration.spec.tsx

@@ -0,0 +1,29 @@
+import {render, screen} from 'sentry-test/reactTestingLibrary';
+
+import Duration from 'sentry/components/duration/duration';
+
+describe('Duration', () => {
+  it('should render the duration in the default format', () => {
+    render(<Duration duration={[83, 'sec']} precision="sec" />);
+
+    const time = screen.getByText('01:23');
+    expect(time).toBeInTheDocument();
+  });
+
+  it('should render the duration in the specified format', () => {
+    render(<Duration duration={[83_456, 'ms']} precision="ms" format="h:mm:ss.sss" />);
+
+    const time = screen.getByText('1:23.456');
+    expect(time).toBeInTheDocument();
+  });
+
+  it('should include `dateTime` & `title` attributes for accessibility', () => {
+    // See https://developer.mozilla.org/en-US/docs/Web/HTML/Element/time
+
+    render(<Duration duration={[83, 'sec']} precision="sec" />);
+
+    const time = screen.getByText('01:23');
+    expect(time).toHaveAttribute('datetime', 'PT1M23S');
+    expect(time).toHaveAttribute('title', '01:23.000');
+  });
+});

+ 27 - 0
static/app/components/duration/duration.stories.tsx

@@ -0,0 +1,27 @@
+import Duration from 'sentry/components/duration/duration';
+import storyBook from 'sentry/stories/storyBook';
+
+export default storyBook(Duration, story => {
+  story('Default format ("hh:mm:ss.sss")', () => (
+    <ul>
+      <li>
+        One millisecond = <Duration duration={[1, 'ms']} precision="ms" />
+      </li>
+      <li>
+        One second = <Duration duration={[1, 'sec']} precision="sec" />
+      </li>
+      <li>
+        One minute = <Duration duration={[1, 'min']} precision="min" />
+      </li>
+      <li>
+        One hour = <Duration duration={[1, 'hour']} precision="hour" />
+      </li>
+      <li>
+        One day = <Duration duration={[1, 'day']} precision="day" />
+      </li>
+      <li>
+        One week = <Duration duration={[1, 'week']} precision="week" />
+      </li>
+    </ul>
+  ));
+});

+ 45 - 0
static/app/components/duration/duration.tsx

@@ -0,0 +1,45 @@
+import type {HTMLAttributes} from 'react';
+import styled from '@emotion/styled';
+
+import formatDuration, {type Format} from 'sentry/utils/duration/formatDuration';
+import type {Duration as TDuration, Unit} from 'sentry/utils/duration/types';
+
+interface Props extends HTMLAttributes<HTMLTimeElement> {
+  /**
+   * The Duration that you want to render
+   */
+  duration: TDuration;
+
+  /**
+   * How granular to render the value. For example you can pass in something
+   * that has `ms` precision but only show the total number of seconds.
+   */
+  precision: Unit;
+
+  /**
+   * The style/format to render into.
+   *
+   * Default is `hh:mm:ss.sss` if the precision is `ms`
+   */
+  format?: Format;
+}
+
+const Duration = styled(({duration, format, precision, ...props}: Props) => {
+  // Style and precision should match, otherwise style will pad out missing or
+  // truncated values which we don't want in this component.
+  const style = format ?? (precision === 'ms' ? 'hh:mm:ss.sss' : 'hh:mm:ss');
+
+  return (
+    <time
+      dateTime={formatDuration({duration, precision: 'ms', style: 'ISO8601'})}
+      title={formatDuration({duration, precision: 'ms', style: 'hh:mm:ss.sss'})}
+      {...props}
+    >
+      {formatDuration({duration, precision, style})}
+    </time>
+  );
+})`
+  font-variant-numeric: tabular-nums;
+`;
+
+export default Duration;

+ 8 - 2
static/app/components/replays/player/scrubber.tsx

@@ -8,7 +8,7 @@ import * as Progress from 'sentry/components/replays/progress';
 import {useReplayContext} from 'sentry/components/replays/replayContext';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
-import formatReplayDuration from 'sentry/utils/duration/formatReplayDuration';
+import formatDuration from 'sentry/utils/duration/formatDuration';
 import divide from 'sentry/utils/number/divide';
 import toPercent from 'sentry/utils/number/toPercent';
 import useTimelineScale from 'sentry/utils/replays/hooks/useTimelineScale';
@@ -62,7 +62,13 @@ function Scrubber({className, showZoomIndicators = false}: Props) {
       <Meter>
         {currentHoverTime ? (
           <div>
-            <TimelineTooltip labelText={formatReplayDuration(currentHoverTime)} />
+            <TimelineTooltip
+              labelText={formatDuration({
+                duration: [currentHoverTime, 'ms'],
+                precision: 'ms',
+                style: 'hh:mm:ss.sss',
+              })}
+            />
             <MouseTrackingValue
               style={{
                 width: toPercent(hoverPlace),

+ 1 - 1
static/app/components/replays/replayContext.tsx

@@ -10,6 +10,7 @@ import type useInitialOffsetMs from 'sentry/utils/replays/hooks/useInitialTimeOf
 import {ReplayCurrentTimeContextProvider} from 'sentry/utils/replays/playback/providers/useCurrentHoverTime';
 import useReplayPrefs from 'sentry/utils/replays/playback/providers/useReplayPrefs';
 import type ReplayReader from 'sentry/utils/replays/replayReader';
+import type {Dimensions} from 'sentry/utils/replays/types';
 import useOrganization from 'sentry/utils/useOrganization';
 import usePrevious from 'sentry/utils/usePrevious';
 import useProjectFromId from 'sentry/utils/useProjectFromId';
@@ -18,7 +19,6 @@ import {useUser} from 'sentry/utils/useUser';
 
 import {CanvasReplayerPlugin} from './canvasReplayerPlugin';
 
-type Dimensions = {height: number; width: number};
 type RootElem = null | HTMLDivElement;
 
 type HighlightCallbacks = ReturnType<typeof useReplayHighlighting>;

+ 8 - 3
static/app/components/replays/timeAndScrubberGrid.tsx

@@ -3,6 +3,7 @@ import styled from '@emotion/styled';
 
 import {Button} from 'sentry/components/button';
 import ButtonBar from 'sentry/components/buttonBar';
+import Duration from 'sentry/components/duration/duration';
 import ReplayTimeline from 'sentry/components/replays/breadcrumbs/replayTimeline';
 import {PlayerScrubber} from 'sentry/components/replays/player/scrubber';
 import useScrubberMouseTracking from 'sentry/components/replays/player/useScrubberMouseTracking';
@@ -10,7 +11,6 @@ import {useReplayContext} from 'sentry/components/replays/replayContext';
 import {IconAdd, IconSubtract} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
-import formatReplayDuration from 'sentry/utils/duration/formatReplayDuration';
 import useTimelineScale, {
   TimelineScaleContextProvider,
 } from 'sentry/utils/replays/hooks/useTimelineScale';
@@ -67,8 +67,9 @@ export default function TimeAndScrubberGrid({
     <TimelineScaleContextProvider>
       <Grid id="replay-timeline-player" isCompact={isCompact}>
         <Numeric style={{gridArea: 'currentTime', paddingInline: space(1.5)}}>
-          {formatReplayDuration(currentTime)}
+          <Duration duration={[currentTime, 'ms']} precision="sec" />
         </Numeric>
+
         <div style={{gridArea: 'timeline'}}>
           <ReplayTimeline />
         </div>
@@ -79,7 +80,11 @@ export default function TimeAndScrubberGrid({
           <PlayerScrubber showZoomIndicators={showZoom} />
         </StyledScrubber>
         <Numeric style={{gridArea: 'duration', paddingInline: space(1.5)}}>
-          {durationMs ? formatReplayDuration(durationMs) : '--:--'}
+          {durationMs === undefined ? (
+            '--:--'
+          ) : (
+            <Duration duration={[durationMs, 'ms']} precision="sec" />
+          )}
         </Numeric>
       </Grid>
     </TimelineScaleContextProvider>

+ 1 - 1
static/app/utils/duration/formatDuration.tsx

@@ -1,7 +1,7 @@
 import {formatSecondsToClock} from 'sentry/utils/duration/formatSecondsToClock';
 import type {Duration, Unit} from 'sentry/utils/duration/types';
 
-type Format =
+export type Format =
   // example: `3,600`
   | 'count-locale'
   // example: `86400`

+ 0 - 11
static/app/utils/duration/formatReplayDuration.spec.tsx

@@ -1,11 +0,0 @@
-import formatReplayDuration from 'sentry/utils/duration/formatReplayDuration';
-
-describe('formatReplayDuration', () => {
-  it.each([
-    ['seconds', 15 * 1000, '00:15'],
-    ['minutes', 2.5 * 60 * 1000, '02:30'],
-    ['hours', 75 * 60 * 1000, '01:15:00'],
-  ])('should format a %s long duration into a string', (_desc, duration, expected) => {
-    expect(formatReplayDuration(duration)).toEqual(expected);
-  });
-});

+ 0 - 14
static/app/utils/duration/formatReplayDuration.tsx

@@ -1,14 +0,0 @@
-import {formatSecondsToClock} from 'sentry/utils/duration/formatSecondsToClock';
-
-export default function formatReplayDuration(ms: number, showMs?: boolean): string {
-  if (ms <= 0 || isNaN(ms)) {
-    if (showMs) {
-      return '00:00.000';
-    }
-
-    return '00:00';
-  }
-
-  const seconds = ms / 1000;
-  return formatSecondsToClock(showMs ? seconds : Math.floor(seconds));
-}

+ 5 - 0
static/app/utils/replays/types.tsx

@@ -17,6 +17,11 @@ import invariant from 'invariant';
 
 import type {HydratedA11yFrame} from 'sentry/utils/replays/hydrateA11yFrame';
 
+export type Dimensions = {
+  height: number;
+  width: number;
+};
+
 // Extracting WebVitalFrame types from TRawSpanFrame so we can document/support
 // the deprecated `nodeId` data field Moving forward, `nodeIds` is the accepted
 // field.

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