From 8f5abcbaa171118054e4377af8e6049b6659e4ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florian=20Qu=C3=A8ze?= Date: Wed, 11 Oct 2023 10:54:55 +0200 Subject: [PATCH] Display nicer time units in the timeline ruler for values that are better expressed in minutes or hours. --- src/components/timeline/Ruler.js | 52 +++++++++++++------ .../ActiveTabTimeline.test.js.snap | 6 +-- .../__snapshots__/Timeline.test.js.snap | 12 ++--- src/test/unit/marker-schema.test.js | 18 +++---- src/utils/format-numbers.js | 16 +++--- 5 files changed, 65 insertions(+), 39 deletions(-) diff --git a/src/components/timeline/Ruler.js b/src/components/timeline/Ruler.js index ff49536d70..b8135c2447 100644 --- a/src/components/timeline/Ruler.js +++ b/src/components/timeline/Ruler.js @@ -6,6 +6,7 @@ import React, { PureComponent } from 'react'; import { TIMELINE_RULER_HEIGHT } from 'firefox-profiler/app-logic/constants'; +import { formatTimestamp } from 'firefox-profiler/utils/format-numbers'; import './Ruler.css'; @@ -19,45 +20,66 @@ type Props = {| |}; export class TimelineRuler extends PureComponent { - _findNiceNumberGreaterOrEqualTo(uglyNumber: number) { + _findNiceNumberGreaterOrEqualTo(uglyNumber: Milliseconds) { + // Special case numbers in the seconds, minutes or hour ranges. + if (uglyNumber > 10000 && uglyNumber <= 48 * 3600 * 1000) { + for (const seconds of [15, 20, 30]) { + const number = seconds * 1000; + if (uglyNumber <= number) { + return number; + } + } + for (const minutes of [1, 2, 5, 10, 15, 20, 30]) { + const number = minutes * 60 * 1000; + if (uglyNumber <= number) { + return number; + } + } + for (const hours of [1, 2, 3, 4, 6, 8, 12, 24, 48]) { + const number = hours * 3600 * 1000; + if (uglyNumber <= number) { + return number; + } + } + } + // Write uglyNumber as a * 10^b, with 1 <= a < 10. // Return the lowest of 2 * 10^b, 5 * 10^b, 10 * 10^b that is greater or equal to uglyNumber. const b = Math.floor(Math.log10(uglyNumber)); if (uglyNumber <= 2 * Math.pow(10, b)) { - return { number: 2 * Math.pow(10, b), exponent: b }; + return 2 * Math.pow(10, b); } if (uglyNumber <= 5 * Math.pow(10, b)) { - return { number: 5 * Math.pow(10, b), exponent: b }; + return 5 * Math.pow(10, b); } - return { number: Math.pow(10, b + 1), exponent: b + 1 }; + return Math.pow(10, b + 1); } _getNotches() { if (this.props.width === 0) { - return { notches: [], decimalPlaces: 0 }; + return { notches: [], notchTime: 0 }; } const { zeroAt, rangeStart, rangeEnd, width } = this.props; const pixelsPerMilliSecond = width / (rangeEnd - rangeStart); const minimumNotchWidth = 55; // pixels - const { number: notchTime, exponent } = - this._findNiceNumberGreaterOrEqualTo( - minimumNotchWidth / pixelsPerMilliSecond - ); + const notchTime = this._findNiceNumberGreaterOrEqualTo( + minimumNotchWidth / pixelsPerMilliSecond + ); const firstNotchIndex = Math.ceil((rangeStart - zeroAt) / notchTime); const lastNotchIndex = Math.floor((rangeEnd - zeroAt) / notchTime); const notches = []; for (let i = firstNotchIndex; i <= lastNotchIndex; i++) { notches.push({ - time: (i * notchTime) / 1000, + time: i * notchTime, pos: (i * notchTime - (rangeStart - zeroAt)) * pixelsPerMilliSecond, }); } - return { notches, decimalPlaces: Math.max(0, -(exponent - 3)) }; + return { notches, notchTime }; } render() { - const { notches, decimalPlaces } = this._getNotches(); + const { notches, notchTime } = this._getNotches(); return (
{ key={i} style={{ left: `${pos}px` }} > - {`${time.toFixed( - decimalPlaces - )}s`} + + {formatTimestamp(time, 2, 2, notchTime)} + ))} diff --git a/src/test/components/__snapshots__/ActiveTabTimeline.test.js.snap b/src/test/components/__snapshots__/ActiveTabTimeline.test.js.snap index 097fc11f8c..55822df1a7 100644 --- a/src/test/components/__snapshots__/ActiveTabTimeline.test.js.snap +++ b/src/test/components/__snapshots__/ActiveTabTimeline.test.js.snap @@ -357,7 +357,7 @@ exports[`ActiveTabTimeline should be rendered properly from the Timeline compone - 0.0000s + 0s
  • - 0.0005s + 500μs
  • - 0.0010s + 1ms
    • diff --git a/src/test/components/__snapshots__/Timeline.test.js.snap b/src/test/components/__snapshots__/Timeline.test.js.snap index ea86c5e0a9..c3fb07efb4 100644 --- a/src/test/components/__snapshots__/Timeline.test.js.snap +++ b/src/test/components/__snapshots__/Timeline.test.js.snap @@ -40,7 +40,7 @@ exports[`TimelineSelection renders the vertical line indicating the time positio - 0.000s + 0s
  • - 0.002s + 2ms
  • - 0.004s + 4ms
  • - 0.006s + 6ms
  • - 0.008s + 8ms
  • - 0.010s + 10ms
    • diff --git a/src/test/unit/marker-schema.test.js b/src/test/unit/marker-schema.test.js index a3ddcb3b39..1de3a09964 100644 --- a/src/test/unit/marker-schema.test.js +++ b/src/test/unit/marker-schema.test.js @@ -299,21 +299,21 @@ describe('marker schema formatting', function () { "duration - 0s", "duration - 10ms", "duration - 12.346ms", - "duration - 2min3s", - "duration - 6h31min", + "duration - 2m3s", + "duration - 6h31m", "duration - 50s", - "duration - 33min20s", - "duration - 13h53min", + "duration - 33m20s", + "duration - 13h53m", "duration - 1d10h", "duration - 123.46ns", "duration - 59.499s", - "duration - 1min", - "duration - 1min", - "duration - 13min", - "duration - 59min59s", + "duration - 1m", + "duration - 1m", + "duration - 13m", + "duration - 59m59s", "duration - 1h", "duration - 1h", - "duration - 23h59min", + "duration - 23h59m", "duration - 1d", "duration - 1d", "time - 12.346ms", diff --git a/src/utils/format-numbers.js b/src/utils/format-numbers.js index f284158581..af0606ec56 100644 --- a/src/utils/format-numbers.js +++ b/src/utils/format-numbers.js @@ -243,7 +243,7 @@ export function formatSeconds( const msPerSecond = 1000; const timeInSeconds = time / msPerSecond; let result = ''; - if (precision !== 0 && precision < msPerSecond) { + if (precision < msPerSecond) { const exponent = Math.floor(Math.log10(precision / msPerSecond)); const digits = Math.max(0, -exponent); result = timeInSeconds.toFixed(digits); @@ -271,8 +271,8 @@ export function formatMinutes( const seconds = time % msPerMinute; return ( formatNumber((time - seconds) / msPerMinute, significantDigits, 0) + - 'min' + - (seconds > 0 && (maxFractionalDigits > 0 || precision < msPerMinute) + 'm' + + ((seconds > 0 && maxFractionalDigits > 0) || precision < msPerMinute ? formatSeconds(seconds, significantDigits, 0, precision) : '') ); @@ -293,7 +293,7 @@ export function formatHours( return ( formatNumber((time - minutes) / msPerHour, significantDigits, 0) + 'h' + - (minutes > 0 && (maxFractionalDigits > 0 || precision < msPerHour) + ((minutes > 0 && maxFractionalDigits > 0) || precision < msPerHour ? formatMinutes(minutes, significantDigits, 0, precision) : '') ); @@ -314,7 +314,7 @@ export function formatDays( return ( formatNumber((time - hours) / msPerDay, significantDigits, 0) + 'd' + - (hours > 0 && (maxFractionalDigits > 0 || precision < msPerDay) + ((hours > 0 && maxFractionalDigits > 0) || precision < msPerDay ? formatHours(hours, significantDigits, 0, precision) : '') ); @@ -330,7 +330,11 @@ export function formatTimestamp( if (precision !== Infinity) { // Round the values to display nicer numbers when the extra precision // isn't useful. (eg. show 3h52min10s instead of 3h52min14s) - precision = 10 ** Math.floor(Math.log10(precision)); + // Only do this for values < 10s as after that we use time units that are + // not decimal. + if (precision < 10000) { + precision = 10 ** Math.floor(Math.log10(precision)); + } if (time > precision) { time = Math.round(time / precision) * precision; }