diff --git a/app/components/form/fields/DateTimeRangePicker.spec.tsx b/app/components/form/fields/DateTimeRangePicker.spec.tsx index dd418db2de..048625610c 100644 --- a/app/components/form/fields/DateTimeRangePicker.spec.tsx +++ b/app/components/form/fields/DateTimeRangePicker.spec.tsx @@ -10,18 +10,27 @@ import { DateTimeRangePicker, dateForInput } from './DateTimeRangePicker' const now = new Date(2020, 1, 1) function renderLastDay() { - const onChange = vi.fn() + const setStartTime = vi.fn() + const setEndTime = vi.fn() render( ) - return { onChange } + return { setStartTime, setEndTime } } +beforeAll(() => { + vi.useFakeTimers() + vi.setSystemTime(now) + + return () => vi.useRealTimers() +}) + describe('useDateTimeRangePicker', () => { it.each([ ['lastHour', subHours(now, 1)], @@ -30,13 +39,13 @@ describe('useDateTimeRangePicker', () => { ['lastWeek', subDays(now, 7)], ['last30Days', subDays(now, 30)], ])('sets initial start and end', (preset, start) => { - const onChange = vi.fn() render( {}} + setEndTime={() => {}} /> ) @@ -52,36 +61,33 @@ it.each([ ['Last week', subDays(now, 7)], ['Last 30 days', subDays(now, 30)], ])('choosing a preset sets the times', (option, start) => { - vi.useFakeTimers() - vi.setSystemTime(now) - - const { onChange } = renderLastDay() + const { setStartTime, setEndTime } = renderLastDay() clickByRole('button', 'Choose a time range') clickByRole('option', option) - expect(onChange).toBeCalledWith(start, now) - - vi.useRealTimers() + expect(setStartTime).toBeCalledWith(start) + expect(setEndTime).toBeCalledWith(now) }) describe('custom mode', () => { it('enables datetime inputs', () => { - const { onChange } = renderLastDay() + const { setStartTime, setEndTime } = renderLastDay() expect(screen.getByLabelText('Start time')).toBeDisabled() clickByRole('button', 'Choose a time range') clickByRole('option', 'Custom...') - expect(onChange).not.toBeCalled() + expect(setStartTime).not.toBeCalled() + expect(setEndTime).not.toBeCalled() expect(screen.getByLabelText('Start time')).toBeEnabled() expect(screen.getByRole('button', { name: 'Reset' })).toBeDisabled() expect(screen.getByRole('button', { name: 'Load' })).toBeDisabled() }) it('clicking load after changing date changes range', async () => { - const { onChange } = renderLastDay() + const { setStartTime, setEndTime } = renderLastDay() expect(screen.getByLabelText('Start time')).toHaveValue(dateForInput(subDays(now, 1))) expect(screen.getByLabelText('End time')).toHaveValue(dateForInput(now)) @@ -98,15 +104,17 @@ describe('custom mode', () => { fireEvent.change(endInput, { target: { value: '2020-01-17T00:00' } }) // changing the input value without clicking Load doesn't do anything - expect(onChange).not.toBeCalled() + expect(setStartTime).not.toBeCalled() + expect(setEndTime).not.toBeCalled() - // clicking Load calls onChange + // clicking Load calls setTime with the new range clickByRole('button', 'Load') - expect(onChange).toBeCalledWith(new Date(2020, 0, 15), new Date(2020, 0, 17)) + expect(setStartTime).toBeCalledWith(new Date(2020, 0, 15)) + expect(setEndTime).toBeCalledWith(new Date(2020, 0, 17)) }) it('clicking reset after changing inputs resets inputs', async () => { - const { onChange } = renderLastDay() + const { setStartTime, setEndTime } = renderLastDay() expect(screen.getByLabelText('Start time')).toHaveValue(dateForInput(subDays(now, 1))) expect(screen.getByLabelText('End time')).toHaveValue(dateForInput(now)) @@ -127,8 +135,8 @@ describe('custom mode', () => { expect(startInput).toHaveValue('2020-01-31T00:00') expect(endInput).toHaveValue('2020-02-01T00:00') - // onChange is never called - expect(onChange).not.toBeCalled() + expect(setStartTime).not.toBeCalled() + expect(setEndTime).not.toBeCalled() }) it('shows error for invalid range', () => { diff --git a/app/components/form/fields/DateTimeRangePicker.tsx b/app/components/form/fields/DateTimeRangePicker.tsx index c3ecfb4a2a..e7b50752a5 100644 --- a/app/components/form/fields/DateTimeRangePicker.tsx +++ b/app/components/form/fields/DateTimeRangePicker.tsx @@ -1,5 +1,5 @@ import { format, subDays, subHours } from 'date-fns' -import { useMemo, useState } from 'react' +import { useCallback, useMemo, useState } from 'react' import { Listbox, useInterval } from '@oxide/ui' import { Button, TextInput } from '@oxide/ui' @@ -32,31 +32,25 @@ const computeStart: Record Date> = { // - list of presets is hard-coded // - initial preset can't be "custom" -type Props = { - initialPreset: RangeKey - /** - * if set and range is a relative preset, update the range to have `endTime` - * of now every X ms - */ - slideInterval?: number - startTime: Date - endTime: Date - onChange: (startTime: Date, endTime: Date) => void -} - -export function useDateTimeRangePickerState(initialPreset: RangeKey) { - // default endTime is now, i.e., mount time +/** + * Exposes `startTime` and `endTime` plus the whole set of picker UI controls as + * a JSX element to render. When we're using a relative preset like last N + * hours, automatically slide the window forward live by updating the range to + * have `endTime` of _now_ every `SLIDE_INTERVAL` ms. + */ +export function useDateTimeRangePicker(initialPreset: RangeKey) { const now = useMemo(() => new Date(), []) const [startTime, setStartTime] = useState(computeStart[initialPreset](now)) const [endTime, setEndTime] = useState(now) - function onChange(newStart: Date, newEnd: Date) { - setStartTime(newStart) - setEndTime(newEnd) - } + const props = { initialPreset, startTime, endTime, setStartTime, setEndTime } - return { startTime, endTime, onChange } + return { + startTime, + endTime, + dateTimeRangePicker: , + } } function validateRange(startTime: Date, endTime: Date): string | null { @@ -67,17 +61,24 @@ function validateRange(startTime: Date, endTime: Date): string | null { return null } -/** - * Exposes `startTime` and `endTime` plus the whole set of picker UI controls as - * a JSX element to render. - */ +/** Interval for sliding range forward when using a relative time preset */ +const SLIDE_INTERVAL = 10_000 + +type DateTimeRangePickerProps = { + initialPreset: RangeKey + startTime: Date + endTime: Date + setStartTime: (startTime: Date) => void + setEndTime: (endTime: Date) => void +} + export function DateTimeRangePicker({ initialPreset, - slideInterval, startTime, endTime, - onChange, -}: Props) { + setStartTime, + setEndTime, +}: DateTimeRangePickerProps) { const [preset, setPreset] = useState(initialPreset) // needs a separate pair of values because they can be edited without @@ -92,22 +93,29 @@ export function DateTimeRangePicker({ const enableInputs = preset === 'custom' - /** Set the input values and call the passed-on onChange with the new times */ - function setTimesForPreset(newPreset: RangeKey) { - const now = new Date() - const newStartTime = computeStart[newPreset](now) - onChange(newStartTime, now) - setStartTimeInput(newStartTime) - setEndTimeInput(now) - } - - useInterval( - () => { - if (preset !== 'custom') setTimesForPreset(preset) + // could handle this in a useEffect that looks at `preset`, but that would + // also run on initial render, which is silly. Instead explicitly call it on + // preset change and in useInterval. + const setRange = useCallback( + (preset: RangeKeyAll) => { + if (preset !== 'custom') { + const now = new Date() + const newStartTime = computeStart[preset](now) + setStartTime(newStartTime) + setEndTime(now) + setStartTimeInput(newStartTime) + setEndTimeInput(now) + } }, - slideInterval && preset !== 'custom' ? slideInterval : null + [setStartTime, setEndTime] ) + useInterval({ + fn: () => setRange(preset), + delay: preset !== 'custom' ? SLIDE_INTERVAL : null, + key: preset, // force a render which clears current interval + }) + return (
{ if (item) { - setPreset(item.value as RangeKeyAll) - if (item.value !== 'custom') setTimesForPreset(item.value as RangeKey) + const newPreset = item.value as RangeKeyAll + setPreset(newPreset) + setRange(newPreset) } }} /> @@ -170,7 +179,10 @@ export function DateTimeRangePicker({ {enableInputs && ( diff --git a/app/pages/SiloUtilizationPage.tsx b/app/pages/SiloUtilizationPage.tsx index 5f3c39fd8c..a0536e05ff 100644 --- a/app/pages/SiloUtilizationPage.tsx +++ b/app/pages/SiloUtilizationPage.tsx @@ -5,7 +5,7 @@ import { Divider, Listbox, PageHeader, PageTitle, Snapshots24Icon } from '@oxide import { bytesToGiB } from '@oxide/util' import { SystemMetric } from 'app/components/SystemMetric' -import { DateTimeRangePicker, useDateTimeRangePickerState } from 'app/components/form' +import { useDateTimeRangePicker } from 'app/components/form' const DEFAULT_SILO_ID = '001de000-5110-4000-8000-000000000000' const ALL_PROJECTS = '|ALL_PROJECTS|' @@ -32,12 +32,7 @@ export function SiloUtilizationPage() { { enabled: !!orgName } ) - const initialPreset = 'lastHour' - const { - startTime, - endTime, - onChange: onTimeChange, - } = useDateTimeRangePickerState(initialPreset) + const { startTime, endTime, dateTimeRangePicker } = useDateTimeRangePicker('lastHour') const orgItems = useMemo(() => { const items = orgs?.items.map(toListboxItem) || [] @@ -109,13 +104,7 @@ export function SiloUtilizationPage() { )} - + {dateTimeRangePicker} {/* TODO: this divider is supposed to go all the way across */} diff --git a/app/pages/project/instances/instance/tabs/MetricsTab.tsx b/app/pages/project/instances/instance/tabs/MetricsTab.tsx index e067c3cd3b..e71adb8b2b 100644 --- a/app/pages/project/instances/instance/tabs/MetricsTab.tsx +++ b/app/pages/project/instances/instance/tabs/MetricsTab.tsx @@ -6,7 +6,7 @@ import { useApiQuery } from '@oxide/api' import { Listbox, Spinner } from '@oxide/ui' import { TimeSeriesAreaChart } from 'app/components/TimeSeriesChart' -import { DateTimeRangePicker, useDateTimeRangePickerState } from 'app/components/form' +import { useDateTimeRangePicker } from 'app/components/form' import { useRequiredParams } from 'app/hooks' type DiskMetricParams = { @@ -69,12 +69,7 @@ function DiskMetric({ function DiskMetrics({ disks }: { disks: Disk[] }) { const { orgName, projectName } = useRequiredParams('orgName', 'projectName') - const initialPreset = 'lastDay' - const { - startTime, - endTime, - onChange: onTimeChange, - } = useDateTimeRangePickerState(initialPreset) + const { startTime, endTime, dateTimeRangePicker } = useDateTimeRangePicker('lastDay') invariant(disks.length > 0, 'DiskMetrics should not be rendered with zero disks') const [diskName, setDiskName] = useState(disks[0].name) @@ -86,9 +81,6 @@ function DiskMetrics({ disks }: { disks: Disk[] }) { return ( <>
- {/* TODO: using a Formik field here feels like overkill, but we've built - ListboxField to require that, i.e., there's no way to get the nice worked-out - layout from ListboxField without using Formik. Something to think about. */} - + {dateTimeRangePicker}
{/* TODO: separate "Reads" from "(count)" so we can diff --git a/app/pages/system/CapacityUtilizationPage.tsx b/app/pages/system/CapacityUtilizationPage.tsx index c8396a6c53..ea71b40732 100644 --- a/app/pages/system/CapacityUtilizationPage.tsx +++ b/app/pages/system/CapacityUtilizationPage.tsx @@ -5,7 +5,7 @@ import { Divider, Listbox, PageHeader, PageTitle, Snapshots24Icon } from '@oxide import { bytesToGiB } from '@oxide/util' import { SystemMetric } from 'app/components/SystemMetric' -import { DateTimeRangePicker, useDateTimeRangePickerState } from 'app/components/form' +import { useDateTimeRangePicker } from 'app/components/form' const FLEET_ID = '001de000-1334-4000-8000-000000000000' const DEFAULT_SILO_ID = '001de000-5110-4000-8000-000000000000' @@ -20,11 +20,7 @@ export function CapacityUtilizationPage() { const { data: silos } = useApiQuery('siloList', {}) const initialPreset = 'lastHour' - const { - startTime, - endTime, - onChange: onTimeChange, - } = useDateTimeRangePickerState(initialPreset) + const { startTime, endTime, dateTimeRangePicker } = useDateTimeRangePicker(initialPreset) const siloItems = useMemo(() => { const items = silos?.items.map((silo) => ({ label: silo.name, value: silo.id })) || [] @@ -66,13 +62,7 @@ export function CapacityUtilizationPage() { {/* TODO: need a button to clear the silo */} - + {dateTimeRangePicker} {/* TODO: this divider is supposed to go all the way across */} diff --git a/libs/ui/lib/hooks/use-interval.ts b/libs/ui/lib/hooks/use-interval.ts index d7550c73cc..e4efd61c32 100644 --- a/libs/ui/lib/hooks/use-interval.ts +++ b/libs/ui/lib/hooks/use-interval.ts @@ -1,16 +1,29 @@ import { useEffect, useRef } from 'react' -// use null delay to prevent the interval from firing -export default function useInterval(callback: () => void, delay: number | null) { +interface UseIntervalProps { + fn: () => void + delay: number | null + /** Use to force a render because changes to the callback won't */ + key?: string +} + +/** + * Fire `fn` on an interval. Does not fire immediately, only after `delay`. + * + * Use null `delay` to prevent the interval from firing at all. Change `key` to + * force a render, which cleans up the currently set interval and possibly sets + * a new one.. + */ +export default function useInterval({ fn, delay, key }: UseIntervalProps) { const callbackRef = useRef<() => void>() useEffect(() => { - callbackRef.current = callback - }, [callback]) + callbackRef.current = fn + }, [fn]) useEffect(() => { if (delay === null) return const intervalId = setInterval(() => callbackRef.current?.(), delay) return () => clearInterval(intervalId) - }, [delay]) + }, [delay, key]) } diff --git a/libs/ui/lib/listbox/Listbox.tsx b/libs/ui/lib/listbox/Listbox.tsx index 0a20c17090..6cf77860ae 100644 --- a/libs/ui/lib/listbox/Listbox.tsx +++ b/libs/ui/lib/listbox/Listbox.tsx @@ -58,7 +58,7 @@ export const Listbox = ({ > {select.selectedItem ? itemToString(select.selectedItem) : placeholder} -
+