-
Notifications
You must be signed in to change notification settings - Fork 18
Instance metrics design tweaks #2676
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7fc945c
3590b62
0aabe23
b351ecc
f30f649
d4b1cbe
e2e1ad9
7de7de4
0372ca9
ebf4ece
172df67
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,20 +39,30 @@ export interface RouteTabsProps { | |
| children: ReactNode | ||
| fullWidth?: boolean | ||
| sideTabs?: boolean | ||
| tabListClassName?: string | ||
| } | ||
| /** Tabbed views, controlling both the layout and functioning of tabs and the panel contents. | ||
| * sideTabs: Whether the tabs are displayed on the side of the panel. Default is false. | ||
| */ | ||
| export function RouteTabs({ children, fullWidth, sideTabs = false }: RouteTabsProps) { | ||
| export function RouteTabs({ | ||
| children, | ||
| fullWidth, | ||
| sideTabs = false, | ||
| tabListClassName, | ||
| }: RouteTabsProps) { | ||
| const wrapperClasses = sideTabs | ||
| ? 'ox-side-tabs flex' | ||
| : cn('ox-tabs', { 'full-width': fullWidth }) | ||
| const tabListClasses = sideTabs ? 'ox-side-tabs-list' : 'ox-tabs-list' | ||
| const panelClasses = cn('ox-tabs-panel', { 'flex-grow': sideTabs }) | ||
| const panelClasses = cn('ox-tabs-panel @container', { 'ml-5 flex-grow': sideTabs }) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Container query lets us move the datepicker onto the next line when we start running out of space. Flex wrap might work here, but eventually I think I'd like to do something cleverer with the layout when it reflows. |
||
| return ( | ||
| <div className={wrapperClasses}> | ||
| {/* eslint-disable-next-line jsx-a11y/interactive-supports-focus */} | ||
| <div role="tablist" className={tabListClasses} onKeyDown={selectTab}> | ||
| <div | ||
| role="tablist" | ||
| className={cn(tabListClasses, tabListClassName)} | ||
| onKeyDown={selectTab} | ||
| > | ||
| {children} | ||
| </div> | ||
| {/* TODO: Add aria-describedby for active tab */} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,8 @@ import type { TooltipProps } from 'recharts/types/component/Tooltip' | |
|
|
||
| import type { ChartDatum } from '@oxide/api' | ||
|
|
||
| import { Spinner } from '~/ui/lib/Spinner' | ||
|
|
||
| // Recharts's built-in ticks behavior is useless and probably broken | ||
| /** | ||
| * Split the data into n evenly spaced ticks, with one at the left end and one a | ||
|
|
@@ -110,6 +112,7 @@ type TimeSeriesChartProps = { | |
| endTime: Date | ||
| unit?: string | ||
| yAxisTickFormatter?: (val: number) => string | ||
| hasBorder?: boolean | ||
| } | ||
|
|
||
| const TICK_COUNT = 6 | ||
|
|
@@ -132,6 +135,7 @@ export default function TimeSeriesChart({ | |
| endTime, | ||
| unit, | ||
| yAxisTickFormatter = (val) => val.toLocaleString(), | ||
| hasBorder = true, | ||
| }: TimeSeriesChartProps) { | ||
| // We use the largest data point +20% for the graph scale. !rawData doesn't | ||
| // mean it's empty (it will never be empty because we fill in artificial 0s at | ||
|
|
@@ -153,9 +157,22 @@ export default function TimeSeriesChart({ | |
| // re-render on every render of the parent when the data is undefined | ||
| const data = useMemo(() => rawData || [], [rawData]) | ||
|
|
||
| if (!data || data.length === 0) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Temporary empty state. We should handle errors and loading differently. |
||
| return ( | ||
| <div className="flex h-[300px] w-full items-center justify-center"> | ||
| <div className="m-4 flex max-w-[18rem] flex-col items-center text-center"> | ||
| <Spinner variant="secondary" /> | ||
| </div> | ||
| </div> | ||
| ) | ||
| } | ||
|
|
||
| return ( | ||
| <div className="h-[300px] w-full"> | ||
| <ResponsiveContainer className={cn(className, 'rounded-lg border border-default')}> | ||
| {/* temporary until we migrate the old metrics to the new style */} | ||
| <ResponsiveContainer | ||
| className={cn(className, hasBorder && 'rounded-lg border border-default')} | ||
| > | ||
| <AreaChart | ||
| width={width} | ||
| height={height} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,9 +6,11 @@ | |
| * Copyright Oxide Computer Company | ||
| */ | ||
|
|
||
| import { useIsFetching } from '@tanstack/react-query' | ||
| import { createContext, useContext, type ReactNode } from 'react' | ||
|
|
||
| import { useDateTimeRangePicker } from '~/components/form/fields/DateTimeRangePicker' | ||
| import { useIntervalPicker } from '~/components/RefetchIntervalPicker' | ||
| import { RouteTabs, Tab } from '~/components/RouteTabs' | ||
| import { useInstanceSelector } from '~/hooks/use-params' | ||
| import { pb } from '~/util/path-builder' | ||
|
|
@@ -23,21 +25,33 @@ const MetricsContext = createContext<{ | |
| startTime: Date | ||
| endTime: Date | ||
| dateTimeRangePicker: ReactNode | ||
| }>({ startTime, endTime, dateTimeRangePicker: <></> }) | ||
| intervalPicker: ReactNode | ||
| }>({ startTime, endTime, dateTimeRangePicker: <></>, intervalPicker: <></> }) | ||
|
|
||
| export const useMetricsContext = () => useContext(MetricsContext) | ||
|
|
||
| export const MetricsTab = () => { | ||
| const { project, instance } = useInstanceSelector() | ||
|
|
||
| const { startTime, endTime, dateTimeRangePicker } = useDateTimeRangePicker({ | ||
| initialPreset: 'lastHour', | ||
| const { preset, onRangeChange, startTime, endTime, dateTimeRangePicker } = | ||
| useDateTimeRangePicker({ | ||
| initialPreset: 'lastHour', | ||
| }) | ||
|
|
||
| const { intervalPicker } = useIntervalPicker({ | ||
| enabled: preset !== 'custom', | ||
| isLoading: useIsFetching({ queryKey: ['siloMetric'] }) > 0, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to update with query keys |
||
| // sliding the range forward is sufficient to trigger a refetch | ||
| fn: () => onRangeChange(preset), | ||
| isSlim: true, | ||
| }) | ||
|
|
||
| // Find the relevant <Outlet> in RouteTabs | ||
| return ( | ||
| <MetricsContext.Provider value={{ startTime, endTime, dateTimeRangePicker }}> | ||
| <RouteTabs sideTabs> | ||
| <MetricsContext.Provider | ||
| value={{ startTime, endTime, dateTimeRangePicker, intervalPicker }} | ||
| > | ||
| <RouteTabs sideTabs tabListClassName="mt-24"> | ||
| <Tab to={pb.instanceCpuMetrics({ project, instance })} sideTab> | ||
| CPU | ||
| </Tab> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This gives us a version that has the dropdown but not the number...perhaps we just scrap the selected interval in the listbox all together here and we can simplify the implementation. I don't think the interval necessarily needs to be visible anywhere.