Browse Source

feat(replays): Use tabular-nums font variant for in Breadcrumbs and Timeline (#35795)

Using `font-variant-numeric: tabular-nums;` gives us numeric values that are a fixed-width which look better in the right column of breadcrumbs and on the timeline.
Also adjust the formatting so things like leading zeros are consistent in those two places.
Ryan Albrecht 2 years ago
parent
commit
ded61129b7

+ 5 - 1
static/app/components/replays/breadcrumbs/gridlines.tsx

@@ -45,7 +45,7 @@ export function MajorGridlines({duration, minWidth = 50, width}: Props) {
 
   return (
     <Gridlines cols={cols} lineStyle="solid" remaining={remaining}>
-      {i => <small>{formatTime((i + 1) * timespan)}</small>}
+      {i => <Label>{formatTime((i + 1) * timespan)}</Label>}
     </Gridlines>
   );
 }
@@ -55,3 +55,7 @@ export function MinorGridlines({duration, minWidth = 20, width}: Props) {
 
   return <Gridlines cols={cols} lineStyle="dotted" remaining={remaining} />;
 }
+
+const Label = styled('small')`
+  font-variant-numeric: tabular-nums;
+`;

+ 3 - 3
static/app/components/replays/playerRelativeTime.tsx

@@ -17,7 +17,7 @@ const PlayerRelativeTime = ({relativeTime, timestamp}: Props) => {
 
   return (
     <Tooltip
-      title={<DateTime date={timestamp} />}
+      title={<DateTime date={timestamp} seconds />}
       disabled={!timestamp}
       skipWrapper
       disableForVisualTest
@@ -31,8 +31,8 @@ const PlayerRelativeTime = ({relativeTime, timestamp}: Props) => {
 
 const Value = styled('p')`
   color: ${p => p.theme.subText};
-  font-size: 0.7em;
-  font-family: ${p => p.theme.text.familyMono};
+  font-size: 0.9em;
+  font-variant-numeric: tabular-nums;
 `;
 
 export default PlayerRelativeTime;

+ 4 - 5
static/app/components/replays/utils.tsx

@@ -17,7 +17,6 @@ function padZero(num: number, len = 2): string {
 const SECOND = 1000;
 const MINUTE = 60 * SECOND;
 const HOUR = 60 * MINUTE;
-const TIME_FORMAT = 'HH:mm:ss';
 
 /**
  * @param timestamp The timestamp that is our reference point. Can be anything that `moment` accepts such as `'2022-05-04T19:47:52.915000Z'` or `1651664872.915`
@@ -29,12 +28,12 @@ export function relativeTimeInMs(timestamp: moment.MomentInput, diffMs: number):
 }
 
 export function showPlayerTime(timestamp: string, relativeTime: number): string {
-  return moment.utc(relativeTimeInMs(timestamp, relativeTime)).format(TIME_FORMAT);
+  return formatTime(relativeTimeInMs(timestamp, relativeTime));
 }
 
 // TODO: move into 'sentry/utils/formatters'
 export function formatTime(ms: number): string {
-  if (ms <= 0) {
+  if (ms <= 0 || isNaN(ms)) {
     return '0:00';
   }
   const hour = Math.floor(ms / HOUR);
@@ -43,9 +42,9 @@ export function formatTime(ms: number): string {
   ms = ms % MINUTE;
   const second = Math.floor(ms / SECOND);
   if (hour) {
-    return `${hour}:${padZero(minute)}:${padZero(second)}`;
+    return `${padZero(hour)}:${padZero(minute)}:${padZero(second)}`;
   }
-  return `${minute}:${padZero(second)}`;
+  return `${padZero(minute)}:${padZero(second)}`;
 }
 
 /**

+ 1 - 1
static/app/views/replays/detail/breadcrumbs/breadcrumbItem.tsx

@@ -82,7 +82,7 @@ const Title = styled('span')`
 const Description = styled('span')`
   ${p => p.theme.overflowEllipsis};
   font-size: 0.7rem;
-  font-family: ${p => p.theme.text.familyMono};
+  font-variant-numeric: tabular-nums;
 `;
 
 type CrumbItemProps = {

+ 6 - 8
tests/js/spec/components/replays/utils.spec.tsx

@@ -33,9 +33,9 @@ function createCrumb({timestamp}: Pick<Crumb, 'timestamp'>): Crumb {
 
 describe('formatTime', () => {
   it.each([
-    ['seconds', 15 * 1000, '0:15'],
-    ['minutes', 2.5 * 60 * 1000, '2:30'],
-    ['hours', 75 * 60 * 1000, '1:15:00'],
+    ['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(formatTime(duration)).toEqual(expected);
   });
@@ -271,14 +271,12 @@ describe('flattenSpans', () => {
   describe('showPlayerTime', () => {
     it('returns time formatted for player', () => {
       expect(showPlayerTime('2022-05-11T23:04:27.576000Z', 1652309918.676)).toEqual(
-        '00:05:48'
+        '05:48'
       );
     });
 
-    it('returns invalid date if timestamp is malformed', () => {
-      expect(showPlayerTime('20223:04:27.576000Z', 1652309918.676)).toEqual(
-        'Invalid date'
-      );
+    it('returns 0:00 if timestamp is malformed', () => {
+      expect(showPlayerTime('20223:04:27.576000Z', 1652309918.676)).toEqual('0:00');
     });
   });