From d64def00c2ada058adb78b95d86976b89ffde7b0 Mon Sep 17 00:00:00 2001 From: Emma Segal-Grossman Date: Tue, 21 Nov 2023 16:51:08 -0500 Subject: [PATCH] Move execution time formatting into its own util (#1386) Refactors and rewrites the humanize time functions used on the dashboard, and swaps out these new functions in a couple of places. Examples of these functions' behaviours can be found in the tests for them. Screenshot 2023-11-16 at 8 07 14 PM Screenshot 2023-11-16 at 8 07 45 PM Screenshot 2023-11-16 at 8 21 13 PM Also fixes inconsistent tooltip text alignment on the dashboard :) --- frontend/src/pages/org/crawl-detail.ts | 4 +- frontend/src/pages/org/dashboard.ts | 53 +++------ .../src/utils/executionTimeFormatter.test.ts | 76 ++++++++++++ frontend/src/utils/executionTimeFormatter.ts | 109 ++++++++++++++++++ 4 files changed, 200 insertions(+), 42 deletions(-) create mode 100644 frontend/src/utils/executionTimeFormatter.test.ts create mode 100644 frontend/src/utils/executionTimeFormatter.ts diff --git a/frontend/src/pages/org/crawl-detail.ts b/frontend/src/pages/org/crawl-detail.ts index 4b305013..aa6284f5 100644 --- a/frontend/src/pages/org/crawl-detail.ts +++ b/frontend/src/pages/org/crawl-detail.ts @@ -4,7 +4,6 @@ import { when } from "lit/directives/when.js"; import { ifDefined } from "lit/directives/if-defined.js"; import { classMap } from "lit/directives/class-map.js"; import { msg, localized, str } from "@lit/localize"; -import humanizeDuration from "pretty-ms"; import type { PageChangeEvent } from "../../components/pagination"; import { RelativeDuration } from "../../components/relative-duration"; @@ -14,6 +13,7 @@ import { isActive } from "../../utils/crawler"; import { CopyButton } from "../../components/copy-button"; import type { Crawl, Seed } from "./types"; import { APIPaginatedList } from "../../types/api"; +import { humanizeExecutionSeconds } from "../../utils/executionTimeFormatter"; const SECTIONS = [ "overview", @@ -644,7 +644,7 @@ export class CrawlDetail extends LiteElement { ${this.crawl!.finished ? html`${humanizeDuration( + >${humanizeExecutionSeconds( this.crawl!.crawlExecSeconds * 1000 )}` diff --git a/frontend/src/pages/org/dashboard.ts b/frontend/src/pages/org/dashboard.ts index ff2e58ea..b38f95fc 100644 --- a/frontend/src/pages/org/dashboard.ts +++ b/frontend/src/pages/org/dashboard.ts @@ -4,13 +4,15 @@ import { when } from "lit/directives/when.js"; import { ifDefined } from "lit/directives/if-defined.js"; import { msg, localized, str } from "@lit/localize"; import type { SlSelectEvent } from "@shoelace-style/shoelace"; -import humanizeMilliseconds from "pretty-ms"; import LiteElement, { html } from "../../utils/LiteElement"; import type { AuthState } from "../../utils/AuthService"; import type { OrgData } from "../../utils/orgs"; import type { SelectNewDialogEvent } from "./index"; -import { getLocale } from "../../utils/localization"; +import { + humanizeExecutionSeconds, + humanizeSeconds, +} from "../../utils/executionTimeFormatter"; type Metrics = { storageUsedBytes: number; @@ -66,30 +68,6 @@ export class Dashboard extends LiteElement { } } - private humanizeExecutionSeconds = (seconds: number) => { - const minutes = Math.ceil(seconds / 60); - - const locale = getLocale(); - const compactFormatter = new Intl.NumberFormat(locale, { - notation: "compact", - style: "unit", - unit: "minute", - unitDisplay: "long", - }); - - const fullFormatter = new Intl.NumberFormat(locale, { - style: "unit", - unit: "minute", - unitDisplay: "long", - maximumFractionDigits: 0, - }); - - return html` - ${compactFormatter.format(minutes)} - (${humanizeMilliseconds(seconds * 1000)})`; - }; - render() { const hasQuota = Boolean(this.metrics?.storageQuotaBytes); const quotaReached = @@ -347,7 +325,7 @@ export class Dashboard extends LiteElement { ) )}
- +
${msg("Available")}
@@ -415,7 +393,7 @@ export class Dashboard extends LiteElement {
${label}
- ${humanizeMilliseconds(value * 1000)} | + ${humanizeExecutionSeconds(value)} | ${this.renderPercentage(value / quotaSeconds)}
@@ -438,9 +416,7 @@ export class Dashboard extends LiteElement { hasQuota ? html` - ${this.humanizeExecutionSeconds( - quotaSeconds - usageSeconds - )} + ${humanizeExecutionSeconds(quotaSeconds - usageSeconds)} ${msg("Available")} ` @@ -464,14 +440,11 @@ export class Dashboard extends LiteElement { ) )}
- +
${msg("Monthly Execution Time Available")}
- ${this.humanizeExecutionSeconds( - quotaSeconds - usageSeconds - )} - | + ${humanizeExecutionSeconds(quotaSeconds - usageSeconds)} | ${this.renderPercentage( (quotaSeconds - usageSeconds) / quotaSeconds )} @@ -481,10 +454,10 @@ export class Dashboard extends LiteElement {
- ${this.humanizeExecutionSeconds(usageSeconds)} + ${humanizeExecutionSeconds(usageSeconds, "short")} - ${this.humanizeExecutionSeconds(quotaSeconds)} + ${humanizeExecutionSeconds(quotaSeconds, "short")}
@@ -603,8 +576,8 @@ export class Dashboard extends LiteElement { > `, - value ? this.humanizeExecutionSeconds(value) : "--", - humanizeMilliseconds(crawlTime * 1000 || 0), + value ? humanizeExecutionSeconds(value) : "--", + humanizeSeconds(crawlTime || 0), ]; }); return html` diff --git a/frontend/src/utils/executionTimeFormatter.test.ts b/frontend/src/utils/executionTimeFormatter.test.ts new file mode 100644 index 00000000..d9d56984 --- /dev/null +++ b/frontend/src/utils/executionTimeFormatter.test.ts @@ -0,0 +1,76 @@ +import { + humanizeSeconds, + humanizeExecutionSeconds, +} from "./executionTimeFormatter"; +import { expect, fixture } from "@open-wc/testing"; + +describe("formatHours", () => { + it("returns a time in hours and minutes when given a time over an hour", () => { + expect(humanizeSeconds(12_345, "en-US")).to.equal("3h 26m"); + }); + it("returns 1m when given a time under a minute", () => { + expect(humanizeSeconds(24, "en-US")).to.equal("1m"); + }); + it("returns 0m and seconds when given a time under a minute with seconds on", () => { + expect(humanizeSeconds(24, "en-US", true)).to.equal("0m 24s"); + }); + it("returns minutes when given a time under an hour", () => { + expect(humanizeSeconds(1_234, "en-US")).to.equal("21m"); + }); + it("returns just hours when given a time exactly in hours", () => { + expect(humanizeSeconds(3_600, "en-US")).to.equal("1h"); + expect(humanizeSeconds(44_442_000, "en-US")).to.equal("12,345h"); + }); + it("handles different locales correctly", () => { + expect(humanizeSeconds(44_442_000_000, "en-IN")).to.equal("1,23,45,000h"); + expect(humanizeSeconds(44_442_000_000, "pt-BR")).to.equal("12.345.000 h"); + expect(humanizeSeconds(44_442_000_000, "de-DE")).to.equal( + "12.345.000 Std." + ); + expect(humanizeSeconds(44_442_000_000, "ar-EG")).to.equal("١٢٬٣٤٥٬٠٠٠ س"); + }); + it("formats zero time as expected", () => { + expect(humanizeSeconds(0, "en-US")).to.equal("0m"); + }); + it("formats zero time as expected", () => { + expect(humanizeSeconds(0, "en-US", true)).to.equal("0s"); + }); + it("formats negative time as expected", () => { + expect(() => humanizeSeconds(-100, "en-US")).to.throw( + "humanizeSeconds in unimplemented for negative times" + ); + }); +}); + +describe("humanizeExecutionSeconds", () => { + it("formats a given time in billable minutes", async () => { + const parentNode = document.createElement("div"); + const el = await fixture(humanizeExecutionSeconds(1_234_567_890), { + parentNode, + }); + expect(el.getAttribute("title")).to.equal("20,576,132 minutes"); + expect(el.textContent?.trim()).to.equal("21M minutes"); + expect(parentNode.innerText).to.equal("21M minutes\u00a0(342,935h 32m)"); + }); + + it("shows a short version when set", async () => { + const parentNode = document.createElement("div"); + const el = await fixture(humanizeExecutionSeconds(1_234_567_890, "short"), { + parentNode, + }); + expect(el.getAttribute("title")).to.equal( + "20,576,132 minutes\u00a0(342,935h 32m)" + ); + expect(el.textContent?.trim()).to.equal("21M minutes"); + expect(parentNode.innerText).to.equal("21M minutes"); + }); + it("skips the details when given a time less than an hour that is exactly in minutes", async () => { + const parentNode = document.createElement("div"); + const el = await fixture(humanizeExecutionSeconds(3_540), { + parentNode, + }); + expect(el.getAttribute("title")).to.equal("59 minutes"); + expect(el.textContent?.trim()).to.equal("59 minutes"); + expect(parentNode.innerText).to.equal("59 minutes"); + }); +}); diff --git a/frontend/src/utils/executionTimeFormatter.ts b/frontend/src/utils/executionTimeFormatter.ts new file mode 100644 index 00000000..599e78b9 --- /dev/null +++ b/frontend/src/utils/executionTimeFormatter.ts @@ -0,0 +1,109 @@ +import { html, nothing } from "lit"; +import { getLocale } from "./localization"; + +/** + * Returns either `nothing`, or hours-minutes-seconds wrapped in parens. + * Biases towards minutes: + * - When the time is exactly on an hour boundary, just shows hours + * - e.g. `3h` + * - When the time isn't on an hour boundary but is on a minute broundary, just shows hours (if applicable) and minutes + * - e.g. `3h 2m` or `32m` + * - When the time is less than a minute, shows minutes and seconds + * - e.g. `0m 43s` + */ +export function humanizeSeconds( + seconds: number, + locale?: string, + displaySeconds = false +) { + if (seconds < 0) { + throw new Error("humanizeSeconds in unimplemented for negative times"); + } + const hours = Math.floor(seconds / 3600); + seconds -= hours * 3600; + // If displaying seconds, round minutes down, otherwise round up + const minutes = displaySeconds + ? Math.floor(seconds / 60) + : Math.ceil(seconds / 60); + seconds -= minutes * 60; + + const hourFormatter = new Intl.NumberFormat(locale, { + style: "unit", + unit: "hour", + unitDisplay: "narrow", + }); + + const minuteFormatter = new Intl.NumberFormat(locale, { + style: "unit", + unit: "minute", + unitDisplay: "narrow", + }); + + const secondFormatter = new Intl.NumberFormat(locale, { + style: "unit", + unit: "second", + unitDisplay: "narrow", + }); + + return [ + hours !== 0 && hourFormatter.format(hours), + (minutes !== 0 || seconds !== 0 || (!displaySeconds && hours === 0)) && + minuteFormatter.format(minutes), + displaySeconds && + (seconds !== 0 || (hours === 0 && minutes === 0)) && + secondFormatter.format(seconds), + ] + .filter(Boolean) + .join(" "); +} + +/** + * Formats execution seconds, either just as minutes (when `style` is `"short"`), or as minutes and hours-minutes-seconds (when `style` is undefined or `"full"`) + * @example humanizeExecutionSeconds(1_234_567_890) + * // 21M minutes (342,935h 31m 30s) + * + * @example humanizeExecutionSeconds(1_234_567_890, "short") + * // 21M minutes + */ +export const humanizeExecutionSeconds = ( + seconds: number, + style: "short" | "full" = "full", + displaySeconds = false +) => { + const locale = getLocale(); + const minutes = Math.ceil(seconds / 60); + + const compactMinuteFormatter = new Intl.NumberFormat(locale, { + notation: "compact", + style: "unit", + unit: "minute", + unitDisplay: "long", + }); + + const longMinuteFormatter = new Intl.NumberFormat(locale, { + style: "unit", + unit: "minute", + unitDisplay: "long", + maximumFractionDigits: 0, + }); + + const details = humanizeSeconds(seconds, locale, displaySeconds); + + // if the time is less than an hour and lines up exactly on the minute, don't render the details. + const formattedDetails = + minutes === Math.floor(seconds / 60) && seconds < 3600 + ? nothing + : `\u00a0(${details})`; + + switch (style) { + case "full": + return html` + ${compactMinuteFormatter.format(minutes)}${formattedDetails}`; + case "short": + return html`${compactMinuteFormatter.format(minutes)}`; + } +};