From f9b9ffad0e49ee68b114bbd0c56e83525f7008bb Mon Sep 17 00:00:00 2001 From: Nihey Takizawa Date: Wed, 16 Sep 2026 11:41:10 -0300 Subject: [PATCH] Let users switch duration fields between seconds, minutes, and hours. Period, grace, and interval were stuck in raw seconds; the shared control now opens in the coarsest whole unit and converts when s/min/hr changes. --- frontend/app/(app)/dashboard/page.test.tsx | 27 ++++--- .../app/duration-slider-field.test.tsx | 55 +++++++++++++-- .../components/app/duration-slider-field.tsx | 70 +++++++++++++++---- frontend/components/app/edit-check-dialog.tsx | 60 +++++++--------- frontend/lib/duration-units.test.ts | 49 +++++++++++++ frontend/lib/duration-units.ts | 45 ++++++++++++ frontend/test/edit-check-dialog.test.tsx | 29 ++++---- 7 files changed, 258 insertions(+), 77 deletions(-) create mode 100644 frontend/lib/duration-units.test.ts create mode 100644 frontend/lib/duration-units.ts diff --git a/frontend/app/(app)/dashboard/page.test.tsx b/frontend/app/(app)/dashboard/page.test.tsx index 2dc4db4..7381f59 100644 --- a/frontend/app/(app)/dashboard/page.test.tsx +++ b/frontend/app/(app)/dashboard/page.test.tsx @@ -296,13 +296,18 @@ describe("DashboardPage", () => { const period = await screen.findByLabelText("Period"); const grace = screen.getByLabelText("Grace"); - expect(period).toHaveValue(300); - expect(grace).toHaveValue(60); + expect(period).toHaveValue(5); + expect(grace).toHaveValue(1); - fireEvent.change(period, { target: { value: "600" } }); - fireEvent.change(grace, { target: { value: "120" } }); - expect(period).toHaveValue(600); - expect(grace).toHaveValue(120); + expect(screen.getAllByRole("button", { name: "Minutes" })[0]).toHaveAttribute( + "aria-pressed", + "true", + ); + + fireEvent.change(period, { target: { value: "10" } }); + fireEvent.change(grace, { target: { value: "2" } }); + expect(period).toHaveValue(10); + expect(grace).toHaveValue(2); }); it("lets the user type an interval when creating an HTTP check", async () => { @@ -314,9 +319,9 @@ describe("DashboardPage", () => { fireEvent.click(await screen.findByRole("button", { name: /^http$/i })); const interval = await screen.findByLabelText("Interval"); - expect(interval).toHaveValue(60); - fireEvent.change(interval, { target: { value: "180" } }); - expect(interval).toHaveValue(180); + expect(interval).toHaveValue(1); + fireEvent.change(interval, { target: { value: "3" } }); + expect(interval).toHaveValue(3); }); it("uses the SIGNAL creator floor when a SOLO collaborator creates a check", async () => { @@ -329,7 +334,7 @@ describe("DashboardPage", () => { fireEvent.click(newCheckButtons[0]); await waitFor(() => - expect(screen.getByLabelText("Period")).toHaveAttribute("min", "60") + expect(screen.getByLabelText("Period")).toHaveAttribute("min", "1") ); }); @@ -343,7 +348,7 @@ describe("DashboardPage", () => { fireEvent.click(newCheckButtons[0]); await waitFor(() => - expect(screen.getByLabelText("Period")).toHaveAttribute("min", "300") + expect(screen.getByLabelText("Period")).toHaveAttribute("min", "5") ); }); diff --git a/frontend/components/app/duration-slider-field.test.tsx b/frontend/components/app/duration-slider-field.test.tsx index a70c2b7..0594fff 100644 --- a/frontend/components/app/duration-slider-field.test.tsx +++ b/frontend/components/app/duration-slider-field.test.tsx @@ -70,18 +70,59 @@ describe("DurationSliderField", () => { expect(screen.getByRole("status")).toHaveTextContent("1000"); }); - it("lets the user type a value above a high minimum before blur", () => { - render(); + it("does not offer minute or hour units for a timeout-scale range", () => { + render(); + + expect(screen.queryByRole("button", { name: "Minutes" })).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Hours" })).not.toBeInTheDocument(); + }); + + it("opens a 5-minute value in minutes and converts when the unit changes", () => { + render( + , + ); const input = screen.getByLabelText("Timeout"); + expect(input).toHaveValue(5); + expect(screen.getByRole("button", { name: "Minutes" })).toHaveAttribute( + "aria-pressed", + "true", + ); + + fireEvent.click(screen.getByRole("button", { name: "Seconds" })); expect(input).toHaveValue(300); + expect(screen.getByRole("status")).toHaveTextContent("300000"); + + fireEvent.click(screen.getByRole("button", { name: "Hours" })); + expect(input).toHaveValue(300 / 3600); + expect(screen.getByRole("status")).toHaveTextContent("300000"); + }); + + it("commits a value typed in minutes", () => { + render( + , + ); - fireEvent.change(input, { target: { value: "6" } }); - expect(input).toHaveValue(6); - fireEvent.change(input, { target: { value: "600" } }); - expect(input).toHaveValue(600); + const input = screen.getByLabelText("Timeout"); + fireEvent.change(input, { target: { value: "10" } }); + expect(screen.getByRole("status")).toHaveTextContent("600000"); + }); + + it("lets the user type a value above a high minimum before blur", () => { + render( + , + ); + + const input = screen.getByLabelText("Timeout"); + expect(input).toHaveValue(5); + + fireEvent.change(input, { target: { value: "1" } }); + expect(input).toHaveValue(1); + expect(screen.getByRole("status")).toHaveTextContent("300000"); + fireEvent.change(input, { target: { value: "10" } }); + expect(input).toHaveValue(10); fireEvent.blur(input); - expect(input).toHaveValue(600); + expect(input).toHaveValue(10); expect(screen.getByRole("status")).toHaveTextContent("600000"); }); }); diff --git a/frontend/components/app/duration-slider-field.tsx b/frontend/components/app/duration-slider-field.tsx index 8dee1e0..30473b3 100644 --- a/frontend/components/app/duration-slider-field.tsx +++ b/frontend/components/app/duration-slider-field.tsx @@ -1,9 +1,18 @@ "use client"; import { useState } from "react"; +import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; import { Label } from "@/components/ui/label"; import { Slider } from "@/components/ui/slider"; +import { + availableDurationUnits, + durationUnitToMs, + DURATION_UNIT_LABEL, + msToDurationUnit, + nicestDurationUnit, + type DurationUnit, +} from "@/lib/duration-units"; function clampMs(ms: number, minMs: number, maxMs: number, stepMs: number) { const snapped = Math.round(ms / stepMs) * stepMs; @@ -27,16 +36,29 @@ export function DurationSliderField({ maxMs: number; stepMs: number; }) { + const units = availableDurationUnits(maxMs); const [draft, setDraft] = useState(null); + const [unit, setUnit] = useState(() => + nicestDurationUnit(valueMs, units), + ); + const selectedUnit = units.includes(unit) ? unit : units[0]; + const displayValue = draft ?? msToDurationUnit(valueMs, selectedUnit); - function commitSeconds(raw: string) { - const seconds = parseFloat(raw); + function commit(raw: string) { + const amount = parseFloat(raw); setDraft(null); - if (!Number.isFinite(seconds)) { + if (!Number.isFinite(amount)) { onValueMsChange(clampMs(valueMs, minMs, maxMs, stepMs)); return; } - onValueMsChange(clampMs(Math.round(seconds * 1000), minMs, maxMs, stepMs)); + onValueMsChange( + clampMs( + Math.round(durationUnitToMs(amount, selectedUnit)), + minMs, + maxMs, + stepMs, + ), + ); } return ( @@ -47,26 +69,47 @@ export function DurationSliderField({ { const raw = event.target.value; setDraft(raw); - const seconds = parseFloat(raw); - if (!Number.isFinite(seconds)) { + const amount = parseFloat(raw); + if (!Number.isFinite(amount)) { return; } - const nextMs = Math.round(seconds * 1000); + const nextMs = Math.round(durationUnitToMs(amount, selectedUnit)); if (nextMs >= minMs && nextMs <= maxMs) { onValueMsChange(clampMs(nextMs, minMs, maxMs, stepMs)); } }} - onBlur={(event) => commitSeconds(event.target.value)} + onBlur={(event) => commit(event.target.value)} /> - s + {units.length === 1 ? ( + s + ) : ( +
+ {units.map((option) => ( + + ))} +
+ )} { setDraft(null); const next = Array.isArray(value) ? value[0] : value; diff --git a/frontend/components/app/edit-check-dialog.tsx b/frontend/components/app/edit-check-dialog.tsx index 1a96b5f..36bea05 100644 --- a/frontend/components/app/edit-check-dialog.tsx +++ b/frontend/components/app/edit-check-dialog.tsx @@ -346,17 +346,15 @@ export function EditCheckDialog({ open, onOpenChange, check, onSaved }: EditChec {scheduleType === "simple" ? ( -
- - setPeriodSeconds(e.target.value)} - required - /> -
+ setPeriodSeconds(String(Math.round(ms / 1000)))} + minMs={periodMinimum * 1000} + maxMs={86400 * 1000} + stepMs={1000} + /> ) : ( <>
@@ -393,17 +391,15 @@ export function EditCheckDialog({ open, onOpenChange, check, onSaved }: EditChec )} -
- - setGraceSeconds(e.target.value)} - required - /> -
+ setGraceSeconds(String(Math.round(ms / 1000)))} + minMs={0} + maxMs={3600 * 1000} + stepMs={1000} + /> )} @@ -421,17 +417,15 @@ export function EditCheckDialog({ open, onOpenChange, check, onSaved }: EditChec required />
-
- - setIntervalSeconds(e.target.value)} - required - /> -
+ setIntervalSeconds(String(Math.round(ms / 1000)))} + minMs={intervalMinimum * 1000} + maxMs={3600 * 1000} + stepMs={1000} + /> { + it("keeps timeout-scale ranges in seconds only", () => { + expect(availableDurationUnits(60_000)).toEqual(["s"]); + }); + + it("adds minutes when the range is longer than one minute", () => { + expect(availableDurationUnits(3_600_000)).toEqual(["s", "min", "hr"]); + }); + + it("adds minutes but not hours when the max is under an hour", () => { + expect(availableDurationUnits(3_599_000)).toEqual(["s", "min"]); + }); +}); + +describe("nicestDurationUnit", () => { + const all = ["s", "min", "hr"] as const; + + it("uses seconds for zero and for values that are not whole minutes", () => { + expect(nicestDurationUnit(0, [...all])).toBe("s"); + expect(nicestDurationUnit(45_000, [...all])).toBe("s"); + }); + + it("uses the coarsest whole unit", () => { + expect(nicestDurationUnit(5_000, [...all])).toBe("s"); + expect(nicestDurationUnit(60_000, [...all])).toBe("min"); + expect(nicestDurationUnit(300_000, [...all])).toBe("min"); + expect(nicestDurationUnit(3_600_000, [...all])).toBe("hr"); + }); + + it("does not pick a unit that is not offered", () => { + expect(nicestDurationUnit(3_600_000, ["s"])).toBe("s"); + }); +}); + +describe("duration unit conversion", () => { + it("converts between milliseconds and the selected unit", () => { + expect(msToDurationUnit(300_000, "min")).toBe(5); + expect(durationUnitToMs(5, "min")).toBe(300_000); + expect(durationUnitToMs(2, "hr")).toBe(7_200_000); + }); +}); diff --git a/frontend/lib/duration-units.ts b/frontend/lib/duration-units.ts new file mode 100644 index 0000000..5770fb0 --- /dev/null +++ b/frontend/lib/duration-units.ts @@ -0,0 +1,45 @@ +export const DURATION_UNITS = ["s", "min", "hr"] as const; +export type DurationUnit = (typeof DURATION_UNITS)[number]; + +export const DURATION_UNIT_MS: Record = { + s: 1_000, + min: 60_000, + hr: 3_600_000, +}; + +export const DURATION_UNIT_LABEL: Record = { + s: "Seconds", + min: "Minutes", + hr: "Hours", +}; + +export function availableDurationUnits(maxMs: number): DurationUnit[] { + const units: DurationUnit[] = ["s"]; + if (maxMs > 60_000) units.push("min"); + if (maxMs >= 3_600_000) units.push("hr"); + return units; +} + +export function nicestDurationUnit( + valueMs: number, + units: DurationUnit[], +): DurationUnit { + if (valueMs === 0) { + return units.includes("s") ? "s" : units[0]; + } + for (const unit of [...units].reverse()) { + const size = DURATION_UNIT_MS[unit]; + if (valueMs >= size && valueMs % size === 0) { + return unit; + } + } + return units[0]; +} + +export function msToDurationUnit(ms: number, unit: DurationUnit): number { + return ms / DURATION_UNIT_MS[unit]; +} + +export function durationUnitToMs(value: number, unit: DurationUnit): number { + return value * DURATION_UNIT_MS[unit]; +} diff --git a/frontend/test/edit-check-dialog.test.tsx b/frontend/test/edit-check-dialog.test.tsx index 849f7cd..fa6a803 100644 --- a/frontend/test/edit-check-dialog.test.tsx +++ b/frontend/test/edit-check-dialog.test.tsx @@ -1,5 +1,5 @@ import { beforeEach, describe, it, expect, vi } from "vitest"; -import { render, screen, fireEvent, waitFor } from "@testing-library/react"; +import { render, screen, fireEvent, waitFor, within } from "@testing-library/react"; import { MockedProvider } from "@apollo/client/testing/react"; import { ApolloClient, ApolloLink, InMemoryCache, Observable } from "@apollo/client"; import { ApolloProvider } from "@apollo/client/react"; @@ -191,15 +191,15 @@ describe("EditCheckDialog", () => { it("shows heartbeat timing fields for a heartbeat check", () => { renderDialog(HEARTBEAT); - expect(screen.getByLabelText(/period/i)).toBeInTheDocument(); - expect(screen.getByLabelText(/grace/i)).toBeInTheDocument(); + expect(screen.getByLabelText("Period")).toBeInTheDocument(); + expect(screen.getByLabelText("Grace")).toBeInTheDocument(); expect(screen.queryByLabelText(/target/i)).not.toBeInTheDocument(); }); it("shows target fields for an active check", () => { renderDialog(HTTP); expect(screen.getByLabelText(/target/i)).toHaveValue("https://example.com/health"); - expect(screen.getByLabelText(/interval/i)).toBeInTheDocument(); + expect(screen.getByLabelText("Interval")).toBeInTheDocument(); expect(screen.queryByLabelText(/period/i)).not.toBeInTheDocument(); }); @@ -208,11 +208,14 @@ describe("EditCheckDialog", () => { const input = screen.getByLabelText("Timeout"); expect(input).toHaveValue(5); - expect(screen.getByRole("slider", { hidden: true })).toHaveValue("5000"); + const timeoutSlider = within( + screen.getByRole("group", { name: "Timeout range" }), + ).getByRole("slider", { hidden: true }); + expect(timeoutSlider).toHaveValue("5000"); fireEvent.change(input, { target: { value: "10" } }); expect(input).toHaveValue(10); - expect(screen.getByRole("slider", { hidden: true })).toHaveValue("10000"); + expect(timeoutSlider).toHaveValue("10000"); }); it("submits typed timeout seconds as milliseconds", async () => { @@ -232,13 +235,13 @@ describe("EditCheckDialog", () => { it("uses the paid plan floor for heartbeat periods and active intervals", async () => { const { unmount } = renderDialog(HEARTBEAT); await waitFor(() => - expect(screen.getByLabelText(/period/i)).toHaveAttribute("min", "60") + expect(screen.getByLabelText("Period")).toHaveAttribute("min", "1") ); unmount(); renderDialog(HTTP); await waitFor(() => - expect(screen.getByLabelText(/interval/i)).toHaveAttribute("min", "60") + expect(screen.getByLabelText("Interval")).toHaveAttribute("min", "1") ); }); @@ -248,7 +251,7 @@ describe("EditCheckDialog", () => { await waitForPlan(); await waitFor(() => - expect(screen.getByLabelText(/period/i)).toHaveAttribute("min", "60") + expect(screen.getByLabelText("Period")).toHaveAttribute("min", "1") ); }); @@ -258,7 +261,7 @@ describe("EditCheckDialog", () => { await waitForPlan(); await waitFor(() => - expect(screen.getByLabelText(/interval/i)).toHaveAttribute("min", "300") + expect(screen.getByLabelText("Interval")).toHaveAttribute("min", "5") ); }); @@ -267,7 +270,7 @@ describe("EditCheckDialog", () => { renderDialog(HEARTBEAT, "SOLO"); await waitForPlan(); - expect(screen.getByLabelText(/period/i)).toHaveAttribute("min", "60"); + expect(screen.getByLabelText("Period")).toHaveAttribute("min", "1"); }); it("allows unrelated edits to an unchanged grandfathered cadence", async () => { @@ -276,7 +279,7 @@ describe("EditCheckDialog", () => { await waitForPlan(); await waitFor(() => - expect(screen.getByLabelText(/interval/i)).toHaveAttribute("min", "30") + expect(screen.getByLabelText("Interval")).toHaveAttribute("min", "30") ); fireEvent.change(screen.getByLabelText(/name/i), { target: { value: "Renamed health check" }, @@ -294,7 +297,7 @@ describe("EditCheckDialog", () => { const { captured } = renderDialogCapturingVariables(grandfathered); await waitForPlan(); - fireEvent.change(screen.getByLabelText(/interval/i), { + fireEvent.change(screen.getByLabelText("Interval"), { target: { value: "45" }, }); submitForm();