From 09681e673f19965ae11a143a148703405ab68099 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 20 Oct 2022 16:28:55 -0500 Subject: [PATCH 1/8] hella basic refetch interval control --- app/hooks/index.ts | 1 + app/hooks/use-refetch-interval.tsx | 36 +++++++++++++++++++ app/pages/project/instances/InstancesPage.tsx | 13 +++++-- 3 files changed, 48 insertions(+), 2 deletions(-) create mode 100644 app/hooks/use-refetch-interval.tsx diff --git a/app/hooks/index.ts b/app/hooks/index.ts index ed14ea9491..e8768dea17 100644 --- a/app/hooks/index.ts +++ b/app/hooks/index.ts @@ -3,3 +3,4 @@ export * from './use-key' export * from './use-params' export * from './use-toast' export * from './use-reduce-motion' +export * from './use-refetch-interval' diff --git a/app/hooks/use-refetch-interval.tsx b/app/hooks/use-refetch-interval.tsx new file mode 100644 index 0000000000..05ca2ceda9 --- /dev/null +++ b/app/hooks/use-refetch-interval.tsx @@ -0,0 +1,36 @@ +import { useState } from 'react' + +import { Listbox } from '@oxide/ui' + +type RefetchIntervalKey = 'Off' | '5s' | '10s' | '1m' | '2m' | '5m' +type RefetchInterval = false | 5_000 | 10_000 | 60_000 | 120_000 | 300_000 + +const intervalValues: Record = { + Off: false, + '5s': 5_000, + '10s': 10_000, + '1m': 60_000, + '2m': 120_000, + '5m': 300_000, +} + +const intervalItems = Object.keys(intervalValues).map((k) => ({ label: k, value: k })) + +export function useRefetchInterval(initialKey: RefetchIntervalKey = 'Off') { + const [intervalKey, setIntervalKey] = useState(initialKey) + return { + refetchInterval: intervalValues[intervalKey], + intervalListbox: ( + // TODO: this is actually more of a menu button in the design? + { + if (item) { + setIntervalKey(item.value as RefetchIntervalKey) + } + }} + /> + ), + } +} diff --git a/app/pages/project/instances/InstancesPage.tsx b/app/pages/project/instances/InstancesPage.tsx index c1bcd6f01c..a5c00a1d71 100644 --- a/app/pages/project/instances/InstancesPage.tsx +++ b/app/pages/project/instances/InstancesPage.tsx @@ -19,7 +19,12 @@ import { buttonStyle, } from '@oxide/ui' -import { requireProjectParams, useProjectParams, useQuickActions } from 'app/hooks' +import { + requireProjectParams, + useProjectParams, + useQuickActions, + useRefetchInterval, +} from 'app/hooks' import { pb } from 'app/util/path-builder' import { useMakeInstanceActions } from './actions' @@ -73,11 +78,13 @@ export function InstancesPage() { ) ) + const { refetchInterval, intervalListbox } = useRefetchInterval('10s') + const { Table, Column } = useQueryTable( 'instanceList', { path: projectParams }, { - refetchInterval: 5000, + refetchInterval, keepPreviousData: true, } ) @@ -90,6 +97,8 @@ export function InstancesPage() { }>Instances + {/* TODO: last refetched time */} + {intervalListbox} Date: Thu, 20 Oct 2022 16:28:55 -0500 Subject: [PATCH 2/8] do it on the metrics page instead --- app/pages/project/instances/InstancesPage.tsx | 13 ++----------- .../project/instances/instance/tabs/MetricsTab.tsx | 10 +++++----- libs/ui/lib/listbox/Listbox.tsx | 2 +- 3 files changed, 8 insertions(+), 17 deletions(-) diff --git a/app/pages/project/instances/InstancesPage.tsx b/app/pages/project/instances/InstancesPage.tsx index a5c00a1d71..c1bcd6f01c 100644 --- a/app/pages/project/instances/InstancesPage.tsx +++ b/app/pages/project/instances/InstancesPage.tsx @@ -19,12 +19,7 @@ import { buttonStyle, } from '@oxide/ui' -import { - requireProjectParams, - useProjectParams, - useQuickActions, - useRefetchInterval, -} from 'app/hooks' +import { requireProjectParams, useProjectParams, useQuickActions } from 'app/hooks' import { pb } from 'app/util/path-builder' import { useMakeInstanceActions } from './actions' @@ -78,13 +73,11 @@ export function InstancesPage() { ) ) - const { refetchInterval, intervalListbox } = useRefetchInterval('10s') - const { Table, Column } = useQueryTable( 'instanceList', { path: projectParams }, { - refetchInterval, + refetchInterval: 5000, keepPreviousData: true, } ) @@ -97,8 +90,6 @@ export function InstancesPage() { }>Instances - {/* TODO: last refetched time */} - {intervalListbox}
- {/* 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. */} + {intervalListbox} = ({ > {select.selectedItem ? itemToString(select.selectedItem) : placeholder} -
+
From acb695100bf9371357976b6bb0e40a0d39486455 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 8 Nov 2022 10:14:41 -0600 Subject: [PATCH 3/8] get rid of refetch interval in favor of hard-coded 10s interval --- .../form/fields/DateTimeRangePicker.tsx | 15 ++++---- app/hooks/index.ts | 1 - app/hooks/use-refetch-interval.tsx | 36 ------------------- app/pages/SiloUtilizationPage.tsx | 1 - app/pages/project/disks/DisksPage.tsx | 12 +++++-- app/pages/project/instances/InstancesPage.tsx | 5 +++ .../instances/instance/tabs/MetricsTab.tsx | 6 +--- app/pages/system/CapacityUtilizationPage.tsx | 1 - 8 files changed, 22 insertions(+), 55 deletions(-) delete mode 100644 app/hooks/use-refetch-interval.tsx diff --git a/app/components/form/fields/DateTimeRangePicker.tsx b/app/components/form/fields/DateTimeRangePicker.tsx index c3ecfb4a2a..0f292546af 100644 --- a/app/components/form/fields/DateTimeRangePicker.tsx +++ b/app/components/form/fields/DateTimeRangePicker.tsx @@ -34,11 +34,6 @@ const computeStart: Record Date> = { 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 @@ -67,13 +62,17 @@ function validateRange(startTime: Date, endTime: Date): string | null { return null } +/** Interval for sliding range forward when using a relative time preset */ +const SLIDE_INTERVAL = 10_000 + /** * Exposes `startTime` and `endTime` plus the whole set of picker UI controls as - * a JSX element to render. + * 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 DateTimeRangePicker({ initialPreset, - slideInterval, startTime, endTime, onChange, @@ -105,7 +104,7 @@ export function DateTimeRangePicker({ () => { if (preset !== 'custom') setTimesForPreset(preset) }, - slideInterval && preset !== 'custom' ? slideInterval : null + preset !== 'custom' ? SLIDE_INTERVAL : null ) return ( diff --git a/app/hooks/index.ts b/app/hooks/index.ts index 08f8f46437..c258c39f88 100644 --- a/app/hooks/index.ts +++ b/app/hooks/index.ts @@ -3,5 +3,4 @@ export * from './use-key' export * from './use-params' export * from './use-toast' export * from './use-reduce-motion' -export * from './use-refetch-interval' export * from './use-uuid' diff --git a/app/hooks/use-refetch-interval.tsx b/app/hooks/use-refetch-interval.tsx deleted file mode 100644 index 05ca2ceda9..0000000000 --- a/app/hooks/use-refetch-interval.tsx +++ /dev/null @@ -1,36 +0,0 @@ -import { useState } from 'react' - -import { Listbox } from '@oxide/ui' - -type RefetchIntervalKey = 'Off' | '5s' | '10s' | '1m' | '2m' | '5m' -type RefetchInterval = false | 5_000 | 10_000 | 60_000 | 120_000 | 300_000 - -const intervalValues: Record = { - Off: false, - '5s': 5_000, - '10s': 10_000, - '1m': 60_000, - '2m': 120_000, - '5m': 300_000, -} - -const intervalItems = Object.keys(intervalValues).map((k) => ({ label: k, value: k })) - -export function useRefetchInterval(initialKey: RefetchIntervalKey = 'Off') { - const [intervalKey, setIntervalKey] = useState(initialKey) - return { - refetchInterval: intervalValues[intervalKey], - intervalListbox: ( - // TODO: this is actually more of a menu button in the design? - { - if (item) { - setIntervalKey(item.value as RefetchIntervalKey) - } - }} - /> - ), - } -} diff --git a/app/pages/SiloUtilizationPage.tsx b/app/pages/SiloUtilizationPage.tsx index 5f3c39fd8c..91762af079 100644 --- a/app/pages/SiloUtilizationPage.tsx +++ b/app/pages/SiloUtilizationPage.tsx @@ -111,7 +111,6 @@ export function SiloUtilizationPage() { + queryClient.invalidateQueries('diskList', { path: { orgName, projectName } }) + const deleteDisk = useApiMutation('diskDelete', { - onSuccess() { - queryClient.invalidateQueries('diskList', { path: { orgName, projectName } }) - }, + onSuccess: refetchDisks, }) const createSnapshot = useApiMutation('snapshotCreate', { @@ -120,6 +123,9 @@ export function DisksPage() { }>Disks + }>Instances + - {intervalListbox} Date: Wed, 9 Nov 2022 10:53:19 -0600 Subject: [PATCH 4/8] fix up bad behavior due to not clearing interval when changing preset --- .../form/fields/DateTimeRangePicker.tsx | 40 ++++++++----------- libs/ui/lib/hooks/use-interval.ts | 21 ++++++++-- 2 files changed, 34 insertions(+), 27 deletions(-) diff --git a/app/components/form/fields/DateTimeRangePicker.tsx b/app/components/form/fields/DateTimeRangePicker.tsx index 0f292546af..8d15a14f9c 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' @@ -46,10 +46,10 @@ export function useDateTimeRangePickerState(initialPreset: RangeKey) { const [startTime, setStartTime] = useState(computeStart[initialPreset](now)) const [endTime, setEndTime] = useState(now) - function onChange(newStart: Date, newEnd: Date) { + const onChange = useCallback((newStart: Date, newEnd: Date) => { setStartTime(newStart) setEndTime(newEnd) - } + }, []) return { startTime, endTime, onChange } } @@ -91,21 +91,20 @@ 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) + useInterval({ + fn: () => { + if (preset !== 'custom') { + const now = new Date() + const newStartTime = computeStart[preset](now) + onChange(newStartTime, now) + setStartTimeInput(newStartTime) + setEndTimeInput(now) + } }, - preset !== 'custom' ? SLIDE_INTERVAL : null - ) + delay: preset !== 'custom' ? SLIDE_INTERVAL : null, + key: preset, // force a render which clears current interval + immediate: true, // run callback immediately as well as on interval + }) return (
@@ -115,12 +114,7 @@ export function DateTimeRangePicker({ defaultValue={initialPreset} aria-label="Choose a time range" items={rangePresets} - onChange={(item) => { - if (item) { - setPreset(item.value as RangeKeyAll) - if (item.value !== 'custom') setTimesForPreset(item.value as RangeKey) - } - }} + onChange={(item) => item && setPreset(item.value as RangeKeyAll)} /> {/* TODO: real React date picker lib instead of native for consistent styling across browsers */} diff --git a/libs/ui/lib/hooks/use-interval.ts b/libs/ui/lib/hooks/use-interval.ts index d7550c73cc..768bca1ccf 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' +interface UseIntervalProps { + fn: () => void + delay: number | null + /** Use to force a render because changes to the callback won't */ + key?: string + /** + * If true, run the callback immediately while setting up the interval. + * Otherwise it will only run after the first interval. + */ + immediate?: boolean +} + // use null delay to prevent the interval from firing -export default function useInterval(callback: () => void, delay: number | null) { +export default function useInterval({ fn, delay, key, immediate }: UseIntervalProps) { const callbackRef = useRef<() => void>() useEffect(() => { - callbackRef.current = callback - }, [callback]) + callbackRef.current = fn + }, [fn]) useEffect(() => { if (delay === null) return + if (immediate) callbackRef.current?.() const intervalId = setInterval(() => callbackRef.current?.(), delay) return () => clearInterval(intervalId) - }, [delay]) + }, [delay, key, immediate]) } From f5bcee3c246035ee919a5a55126b8e1f1f1a17c3 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 9 Nov 2022 12:00:33 -0600 Subject: [PATCH 5/8] all right forget the immediate: true thing --- .../form/fields/DateTimeRangePicker.spec.tsx | 14 +++++++------ .../form/fields/DateTimeRangePicker.tsx | 21 +++++++++++++++---- libs/ui/lib/hooks/use-interval.ts | 18 ++++++++-------- 3 files changed, 34 insertions(+), 19 deletions(-) diff --git a/app/components/form/fields/DateTimeRangePicker.spec.tsx b/app/components/form/fields/DateTimeRangePicker.spec.tsx index dd418db2de..dc21809695 100644 --- a/app/components/form/fields/DateTimeRangePicker.spec.tsx +++ b/app/components/form/fields/DateTimeRangePicker.spec.tsx @@ -22,6 +22,13 @@ function renderLastDay() { return { onChange } } +beforeAll(() => { + vi.useFakeTimers() + vi.setSystemTime(now) + + return () => vi.useRealTimers() +}) + describe('useDateTimeRangePicker', () => { it.each([ ['lastHour', subHours(now, 1)], @@ -52,17 +59,12 @@ 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() clickByRole('button', 'Choose a time range') clickByRole('option', option) expect(onChange).toBeCalledWith(start, now) - - vi.useRealTimers() }) describe('custom mode', () => { @@ -100,7 +102,7 @@ describe('custom mode', () => { // changing the input value without clicking Load doesn't do anything expect(onChange).not.toBeCalled() - // clicking Load calls onChange + // clicking Load calls onChange with the new range clickByRole('button', 'Load') expect(onChange).toBeCalledWith(new Date(2020, 0, 15), new Date(2020, 0, 17)) }) diff --git a/app/components/form/fields/DateTimeRangePicker.tsx b/app/components/form/fields/DateTimeRangePicker.tsx index 8d15a14f9c..5a24be22d7 100644 --- a/app/components/form/fields/DateTimeRangePicker.tsx +++ b/app/components/form/fields/DateTimeRangePicker.tsx @@ -91,8 +91,11 @@ export function DateTimeRangePicker({ const enableInputs = preset === 'custom' - useInterval({ - fn: () => { + // 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) @@ -101,9 +104,13 @@ export function DateTimeRangePicker({ setEndTimeInput(now) } }, + [onChange] + ) + + useInterval({ + fn: () => setRange(preset), delay: preset !== 'custom' ? SLIDE_INTERVAL : null, key: preset, // force a render which clears current interval - immediate: true, // run callback immediately as well as on interval }) return ( @@ -114,7 +121,13 @@ export function DateTimeRangePicker({ defaultValue={initialPreset} aria-label="Choose a time range" items={rangePresets} - onChange={(item) => item && setPreset(item.value as RangeKeyAll)} + onChange={(item) => { + if (item) { + const newPreset = item.value as RangeKeyAll + setPreset(newPreset) + setRange(newPreset) + } + }} /> {/* TODO: real React date picker lib instead of native for consistent styling across browsers */} diff --git a/libs/ui/lib/hooks/use-interval.ts b/libs/ui/lib/hooks/use-interval.ts index 768bca1ccf..e4efd61c32 100644 --- a/libs/ui/lib/hooks/use-interval.ts +++ b/libs/ui/lib/hooks/use-interval.ts @@ -5,15 +5,16 @@ interface UseIntervalProps { delay: number | null /** Use to force a render because changes to the callback won't */ key?: string - /** - * If true, run the callback immediately while setting up the interval. - * Otherwise it will only run after the first interval. - */ - immediate?: boolean } -// use null delay to prevent the interval from firing -export default function useInterval({ fn, delay, key, immediate }: UseIntervalProps) { +/** + * 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(() => { @@ -22,8 +23,7 @@ export default function useInterval({ fn, delay, key, immediate }: UseIntervalPr useEffect(() => { if (delay === null) return - if (immediate) callbackRef.current?.() const intervalId = setInterval(() => callbackRef.current?.(), delay) return () => clearInterval(intervalId) - }, [delay, key, immediate]) + }, [delay, key]) } From fe6a144e1fde5c92426483a50ff700caca0e7d2d Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 9 Nov 2022 12:07:38 -0600 Subject: [PATCH 6/8] useDateTimeRange --- app/components/form/fields/DateTimeRangePicker.tsx | 2 +- app/pages/SiloUtilizationPage.tsx | 8 ++------ app/pages/project/instances/instance/tabs/MetricsTab.tsx | 8 ++------ app/pages/system/CapacityUtilizationPage.tsx | 8 ++------ 4 files changed, 7 insertions(+), 19 deletions(-) diff --git a/app/components/form/fields/DateTimeRangePicker.tsx b/app/components/form/fields/DateTimeRangePicker.tsx index 5a24be22d7..635d22ac22 100644 --- a/app/components/form/fields/DateTimeRangePicker.tsx +++ b/app/components/form/fields/DateTimeRangePicker.tsx @@ -39,7 +39,7 @@ type Props = { onChange: (startTime: Date, endTime: Date) => void } -export function useDateTimeRangePickerState(initialPreset: RangeKey) { +export function useDateTimeRange(initialPreset: RangeKey) { // default endTime is now, i.e., mount time const now = useMemo(() => new Date(), []) diff --git a/app/pages/SiloUtilizationPage.tsx b/app/pages/SiloUtilizationPage.tsx index 91762af079..1d6b981446 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 { DateTimeRangePicker, useDateTimeRange } from 'app/components/form' const DEFAULT_SILO_ID = '001de000-5110-4000-8000-000000000000' const ALL_PROJECTS = '|ALL_PROJECTS|' @@ -33,11 +33,7 @@ export function SiloUtilizationPage() { ) const initialPreset = 'lastHour' - const { - startTime, - endTime, - onChange: onTimeChange, - } = useDateTimeRangePickerState(initialPreset) + const { startTime, endTime, onChange: onTimeChange } = useDateTimeRange(initialPreset) const orgItems = useMemo(() => { const items = orgs?.items.map(toListboxItem) || [] diff --git a/app/pages/project/instances/instance/tabs/MetricsTab.tsx b/app/pages/project/instances/instance/tabs/MetricsTab.tsx index 8b9a6e76b2..ca4f93a218 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 { DateTimeRangePicker, useDateTimeRange } from 'app/components/form' import { useRequiredParams } from 'app/hooks' type DiskMetricParams = { @@ -70,11 +70,7 @@ function DiskMetrics({ disks }: { disks: Disk[] }) { const { orgName, projectName } = useRequiredParams('orgName', 'projectName') const initialPreset = 'lastDay' - const { - startTime, - endTime, - onChange: onTimeChange, - } = useDateTimeRangePickerState(initialPreset) + const { startTime, endTime, onChange: onTimeChange } = useDateTimeRange(initialPreset) invariant(disks.length > 0, 'DiskMetrics should not be rendered with zero disks') const [diskName, setDiskName] = useState(disks[0].name) diff --git a/app/pages/system/CapacityUtilizationPage.tsx b/app/pages/system/CapacityUtilizationPage.tsx index e2a6258913..0cb6e68791 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 { DateTimeRangePicker, useDateTimeRange } 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, onChange: onTimeChange } = useDateTimeRange(initialPreset) const siloItems = useMemo(() => { const items = silos?.items.map((silo) => ({ label: silo.name, value: silo.id })) || [] From 9f4cdbeb07a54662fabe94474f082befc4c5550d Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 9 Nov 2022 15:13:25 -0600 Subject: [PATCH 7/8] go back to the hook model --- .../form/fields/DateTimeRangePicker.spec.tsx | 38 +++++++------ .../form/fields/DateTimeRangePicker.tsx | 56 ++++++++++--------- app/pages/SiloUtilizationPage.tsx | 12 +--- .../instances/instance/tabs/MetricsTab.tsx | 12 +--- app/pages/system/CapacityUtilizationPage.tsx | 11 +--- 5 files changed, 62 insertions(+), 67 deletions(-) diff --git a/app/components/form/fields/DateTimeRangePicker.spec.tsx b/app/components/form/fields/DateTimeRangePicker.spec.tsx index dc21809695..048625610c 100644 --- a/app/components/form/fields/DateTimeRangePicker.spec.tsx +++ b/app/components/form/fields/DateTimeRangePicker.spec.tsx @@ -10,16 +10,18 @@ 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(() => { @@ -37,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={() => {}} /> ) @@ -59,31 +61,33 @@ it.each([ ['Last week', subDays(now, 7)], ['Last 30 days', subDays(now, 30)], ])('choosing a preset sets the times', (option, start) => { - const { onChange } = renderLastDay() + const { setStartTime, setEndTime } = renderLastDay() clickByRole('button', 'Choose a time range') clickByRole('option', option) - expect(onChange).toBeCalledWith(start, now) + 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)) @@ -100,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 with the new range + // 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)) @@ -129,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 635d22ac22..e7b50752a5 100644 --- a/app/components/form/fields/DateTimeRangePicker.tsx +++ b/app/components/form/fields/DateTimeRangePicker.tsx @@ -32,26 +32,25 @@ const computeStart: Record Date> = { // - list of presets is hard-coded // - initial preset can't be "custom" -type Props = { - initialPreset: RangeKey - startTime: Date - endTime: Date - onChange: (startTime: Date, endTime: Date) => void -} - -export function useDateTimeRange(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) - const onChange = useCallback((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 { @@ -65,18 +64,21 @@ function validateRange(startTime: Date, endTime: Date): string | null { /** Interval for sliding range forward when using a relative time preset */ const SLIDE_INTERVAL = 10_000 -/** - * 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. - */ +type DateTimeRangePickerProps = { + initialPreset: RangeKey + startTime: Date + endTime: Date + setStartTime: (startTime: Date) => void + setEndTime: (endTime: Date) => void +} + export function DateTimeRangePicker({ initialPreset, startTime, endTime, - onChange, -}: Props) { + setStartTime, + setEndTime, +}: DateTimeRangePickerProps) { const [preset, setPreset] = useState(initialPreset) // needs a separate pair of values because they can be edited without @@ -99,12 +101,13 @@ export function DateTimeRangePicker({ if (preset !== 'custom') { const now = new Date() const newStartTime = computeStart[preset](now) - onChange(newStartTime, now) + setStartTime(newStartTime) + setEndTime(now) setStartTimeInput(newStartTime) setEndTimeInput(now) } }, - [onChange] + [setStartTime, setEndTime] ) useInterval({ @@ -176,7 +179,10 @@ export function DateTimeRangePicker({ {enableInputs && ( diff --git a/app/pages/SiloUtilizationPage.tsx b/app/pages/SiloUtilizationPage.tsx index 1d6b981446..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, useDateTimeRange } 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,8 +32,7 @@ export function SiloUtilizationPage() { { enabled: !!orgName } ) - const initialPreset = 'lastHour' - const { startTime, endTime, onChange: onTimeChange } = useDateTimeRange(initialPreset) + const { startTime, endTime, dateTimeRangePicker } = useDateTimeRangePicker('lastHour') const orgItems = useMemo(() => { const items = orgs?.items.map(toListboxItem) || [] @@ -105,12 +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 ca4f93a218..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, useDateTimeRange } from 'app/components/form' +import { useDateTimeRangePicker } from 'app/components/form' import { useRequiredParams } from 'app/hooks' type DiskMetricParams = { @@ -69,8 +69,7 @@ function DiskMetric({ function DiskMetrics({ disks }: { disks: Disk[] }) { const { orgName, projectName } = useRequiredParams('orgName', 'projectName') - const initialPreset = 'lastDay' - const { startTime, endTime, onChange: onTimeChange } = useDateTimeRange(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) @@ -94,12 +93,7 @@ function DiskMetrics({ disks }: { disks: Disk[] }) { }} defaultValue={diskName} /> - + {dateTimeRangePicker} {/* TODO: separate "Reads" from "(count)" so we can diff --git a/app/pages/system/CapacityUtilizationPage.tsx b/app/pages/system/CapacityUtilizationPage.tsx index 0cb6e68791..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, useDateTimeRange } 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,7 +20,7 @@ export function CapacityUtilizationPage() { const { data: silos } = useApiQuery('siloList', {}) const initialPreset = 'lastHour' - const { startTime, endTime, onChange: onTimeChange } = useDateTimeRange(initialPreset) + const { startTime, endTime, dateTimeRangePicker } = useDateTimeRangePicker(initialPreset) const siloItems = useMemo(() => { const items = silos?.items.map((silo) => ({ label: silo.name, value: silo.id })) || [] @@ -62,12 +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 */} From 9048dc5e50815717bf4f617f81c81c582e7dea31 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 9 Nov 2022 15:17:21 -0600 Subject: [PATCH 8/8] don't do the refetch buttons in this branch --- app/pages/project/disks/DisksPage.tsx | 12 +++--------- app/pages/project/instances/InstancesPage.tsx | 5 ----- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/app/pages/project/disks/DisksPage.tsx b/app/pages/project/disks/DisksPage.tsx index 103ed0e944..ea65a1b7a3 100644 --- a/app/pages/project/disks/DisksPage.tsx +++ b/app/pages/project/disks/DisksPage.tsx @@ -12,11 +12,9 @@ import { DateCell } from '@oxide/table' import { SizeCell } from '@oxide/table' import { useQueryTable } from '@oxide/table' import { - Button, EmptyMessage, PageHeader, PageTitle, - Refresh16Icon, Storage24Icon, Success16Icon, TableActions, @@ -75,11 +73,10 @@ export function DisksPage() { const { Table, Column } = useQueryTable('diskList', { path: { orgName, projectName } }) const addToast = useToast() - const refetchDisks = () => - queryClient.invalidateQueries('diskList', { path: { orgName, projectName } }) - const deleteDisk = useApiMutation('diskDelete', { - onSuccess: refetchDisks, + onSuccess() { + queryClient.invalidateQueries('diskList', { path: { orgName, projectName } }) + }, }) const createSnapshot = useApiMutation('snapshotCreate', { @@ -123,9 +120,6 @@ export function DisksPage() { }>Disks - }>Instances -