Skip to content

Convert old charts to new style #2762

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.

Sign up for GitHub

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

Merged
david-crespo merged 5 commits into from
Mar 24, 2025
Merged

Convert old charts to new style #2762

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7bf064d
remove width and height from TimeSeriesChart
david-crespo Mar 21, 2025
2b10d3d
use new chart style on system and silo utilization
david-crespo Mar 22, 2025
fe6d11e
fix lint error
david-crespo Mar 22, 2025
b423805
Tighten spacing and more consistent with other layouts (#2764)
benjaminleonard Mar 24, 2025
04128b7
very important and helpful comment
david-crespo Mar 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 26 additions & 49 deletions app/components/RefetchIntervalPicker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@
* Copyright Oxide Computer Company
*/
import cn from 'classnames'
import { useEffect, useState } from 'react'
import { useState } from 'react'

import { Refresh16Icon, Time16Icon } from '@oxide/design-system/icons/react'
import { Refresh16Icon } from '@oxide/design-system/icons/react'

import { Listbox, type ListboxItem } from '~/ui/lib/Listbox'
import { SpinnerLoader } from '~/ui/lib/Spinner'
import { useInterval } from '~/ui/lib/use-interval'
import { toLocaleTimeString } from '~/util/date'

const intervalPresets = {
Off: undefined,
Expand All @@ -37,63 +36,41 @@ type Props = {
enabled: boolean
isLoading: boolean
fn: () => void
showLastFetched?: boolean
className?: string
isSlim?: boolean
}

export function useIntervalPicker({
enabled,
isLoading,
fn,
showLastFetched = false,
className,
isSlim = false,
}: Props) {
export function useIntervalPicker({ enabled, isLoading, fn, className }: Props) {
const [intervalPreset, setIntervalPreset] = useState<IntervalPreset>('10s')

const [lastFetched, setLastFetched] = useState(new Date())
useEffect(() => {
if (isLoading) setLastFetched(new Date())
}, [isLoading])

const delay = enabled ? intervalPresets[intervalPreset] : null
useInterval({ fn, delay })

return {
intervalMs: (enabled && intervalPresets[intervalPreset]) || undefined,
intervalPicker: (
<div className={cn('flex items-center justify-between', className)}>
{showLastFetched && (
<div className="hidden items-center gap-2 text-right text-mono-sm text-tertiary lg+:flex">
<Time16Icon className="text-quaternary" /> Refreshed{' '}
{toLocaleTimeString(lastFetched)}
</div>
)}
<div className="flex">
<button
type="button"
className={cn(
'flex w-10 items-center justify-center rounded-l border-b border-l border-t border-default disabled:cursor-default',
isLoading && 'hover:bg-hover',
!enabled && 'cursor-not-allowed bg-disabled'
)}
onClick={fn}
disabled={isLoading || !enabled}
>
<SpinnerLoader isLoading={isLoading}>
<Refresh16Icon className="text-secondary" />
</SpinnerLoader>
</button>
<Listbox
selected={enabled ? intervalPreset : 'Off'}
className={cn('[&_button]:!rounded-l-none', isSlim ? '' : 'w-24')}
items={intervalItems}
onChange={setIntervalPreset}
disabled={!enabled}
hideSelected={isSlim}
/>
</div>
<div className={cn('flex', className)}>
<button
type="button"
className={cn(
'flex w-10 items-center justify-center rounded-l border-b border-l border-t border-default disabled:cursor-default',
isLoading && 'hover:bg-hover',
!enabled && 'cursor-not-allowed bg-disabled'
)}
onClick={fn}
disabled={isLoading || !enabled}
>
<SpinnerLoader isLoading={isLoading}>
<Refresh16Icon className="text-secondary" />
</SpinnerLoader>
</button>
<Listbox
selected={enabled ? intervalPreset : 'Off'}
className="[&_button]:!rounded-l-none"
items={intervalItems}
onChange={setIntervalPreset}
disabled={!enabled}
hideSelected
/>
</div>
),
}
Expand Down
66 changes: 24 additions & 42 deletions app/components/SystemMetric.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,15 @@ import {
type SystemMetricName,
} from '@oxide/api'

import { Spinner } from '~/ui/lib/Spinner'

import { TimeSeriesChart } from './TimeSeriesChart'
import { ChartContainer, ChartHeader, TimeSeriesChart } from './TimeSeriesChart'

// The difference between system metric and silo metric is
// 1. different endpoints
// 2. silo metric doesn't have capacity

type MetricProps = {
title: string
unit?: string
unit: string
startTime: Date
endTime: Date
metricName: SystemMetricName
Expand Down Expand Up @@ -93,25 +91,17 @@ export function SiloMetric({
// in the tooltip. could be just once on the end of the x-axis like GCP

return (
<div>
<h2 className="flex items-center gap-1.5 px-3 text-mono-sm text-default">
{title} {unit && <span className="text-tertiary">({unit})</span>}{' '}
{(inRange.isPending || beforeStart.isPending) && <Spinner />}
Copy link
Copy Markdown
Collaborator Author

@david-crespo david-crespo Mar 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loading state is handled inside the chart. The logic needs to be improved, but the loading state does show up for these charts.

</h2>
{/* TODO: proper skeleton for empty chart */}
<div className="mt-3 h-[300px]">
<TimeSeriesChart
data={data}
title={title}
width={480}
height={240}
interpolation="stepAfter"
startTime={startTime}
endTime={endTime}
unit={unit !== 'count' ? unit : undefined}
/>
</div>
</div>
<ChartContainer>
<ChartHeader title={title} label={`(${unit})`} />
<TimeSeriesChart
data={data}
title={title}
interpolation="stepAfter"
startTime={startTime}
endTime={endTime}
unit={unit !== 'Count' ? unit : undefined}
/>
</ChartContainer>
)
}

Expand Down Expand Up @@ -169,24 +159,16 @@ export function SystemMetric({
// in the tooltip. could be just once on the end of the x-axis like GCP

return (
<div>
<h2 className="flex items-center gap-1.5 px-3 text-mono-sm text-default">
{title} {unit && <span className="text-tertiary">({unit})</span>}{' '}
{(inRange.isPending || beforeStart.isPending) && <Spinner />}
</h2>
{/* TODO: proper skeleton for empty chart */}
<div className="mt-3 h-[300px]">
<TimeSeriesChart
data={data}
title={title}
width={480}
height={240}
interpolation="stepAfter"
startTime={startTime}
endTime={endTime}
unit={unit !== 'count' ? unit : undefined}
/>
</div>
</div>
<ChartContainer>
<ChartHeader title={title} label={`(${unit})`} />
<TimeSeriesChart
data={data}
title={title}
interpolation="stepAfter"
startTime={startTime}
endTime={endTime}
unit={unit !== 'Count' ? unit : undefined}
/>
</ChartContainer>
)
}
55 changes: 34 additions & 21 deletions app/components/TimeSeriesChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import type { TooltipProps } from 'recharts/types/component/Tooltip'
import type { ChartDatum } from '@oxide/api'
import { Error12Icon } from '@oxide/design-system/icons/react'

import { classed } from '~/util/classed'

// 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
Expand Down Expand Up @@ -110,14 +112,11 @@ type TimeSeriesChartProps = {
className?: string
data: ChartDatum[] | undefined
title: string
width: number
height: number
Copy link
Copy Markdown
Collaborator Author

@david-crespo david-crespo Mar 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were passing height and width at every callsite but it had no effect, probably because the ResponsiveContainer was overriding it.

interpolation?: 'linear' | 'stepAfter'
startTime: Date
endTime: Date
unit?: string
yAxisTickFormatter?: (val: number) => string
hasBorder?: boolean
hasError?: boolean
}

Expand Down Expand Up @@ -166,17 +165,13 @@ const SkeletonMetric = ({
)

export function TimeSeriesChart({
className,
data: rawData,
title,
width,
height,
interpolation = 'linear',
startTime,
endTime,
unit,
yAxisTickFormatter = (val) => val.toLocaleString(),
hasBorder = true,
hasError = false,
}: TimeSeriesChartProps) {
// We use the largest data point +20% for the graph scale. !rawData doesn't
Expand Down Expand Up @@ -210,34 +205,28 @@ export function TimeSeriesChart({
// re-render on every render of the parent when the data is undefined
const data = useMemo(() => rawData || [], [rawData])

const wrapperClass = cn(className, hasBorder && 'rounded-lg border border-default')

if (hasError) {
return (
<SkeletonMetric className={wrapperClass}>
<SkeletonMetric>
<MetricsError />
</SkeletonMetric>
)
}

if (!data || data.length === 0) {
return (
<SkeletonMetric shimmer className={wrapperClass}>
<SkeletonMetric shimmer>
<MetricsLoadingIndicator />
</SkeletonMetric>
)
}

// ResponsiveContainer has default height and width of 100%
// https://recharts.org/en-US/api/ResponsiveContainer
return (
<div className="h-[300px] w-full">
{/* temporary until we migrate the old metrics to the new style */}
<ResponsiveContainer className={wrapperClass}>
<AreaChart
width={width}
height={height}
data={data}
margin={{ top: 0, right: hasBorder ? 16 : 0, bottom: 0, left: 0 }}
>
<div className="px-5 pb-5 pt-8">
<ResponsiveContainer height={300}>
<AreaChart data={data} margin={{ top: 0, right: 0, bottom: 0, left: 0 }}>
<CartesianGrid stroke={GRID_GRAY} vertical={false} />
<XAxis
axisLine={{ stroke: GRID_GRAY }}
Expand Down Expand Up @@ -277,7 +266,7 @@ export function TimeSeriesChart({
/>
<Area
dataKey="value"
name={title}
name={title} // Provides name for value in hover tooltip
type={interpolation}
stroke={GREEN_600}
fill={GREEN_400}
Expand Down Expand Up @@ -320,3 +309,27 @@ const MetricsError = () => (
/>
</>
)

export const ChartContainer = classed.div`flex w-full grow flex-col rounded-lg border border-default`

type ChartHeaderProps = {
title: string
label: string
description?: string
children?: ReactNode
}

export function ChartHeader({ title, label, description, children }: ChartHeaderProps) {
return (
<div className="flex items-center justify-between border-b px-5 pb-4 pt-5 border-secondary">
<div>
<h2 className="flex items-baseline gap-1.5">
<div className="text-sans-semi-lg">{title}</div>
<div className="text-sans-md text-secondary">{label}</div>
</h2>
<div className="mt-0.5 text-sans-md text-secondary">{description}</div>
</div>
{children}
</div>
)
}
40 changes: 14 additions & 26 deletions app/components/oxql-metrics/OxqlMetric.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import * as Dropdown from '~/ui/lib/DropdownMenu'
import { classed } from '~/util/classed'
import { links } from '~/util/links'

import { TimeSeriesChart } from '../TimeSeriesChart'
import { ChartContainer, ChartHeader, TimeSeriesChart } from '../TimeSeriesChart'
import { HighlightedOxqlQuery, toOxqlStr } from './HighlightedOxqlQuery'
import {
composeOxqlData,
Expand Down Expand Up @@ -77,15 +77,8 @@ export function OxqlMetric({ title, description, unit, ...queryObj }: OxqlMetric
const [modalOpen, setModalOpen] = useState(false)

return (
<div className="flex w-full grow flex-col rounded-lg border border-default">
<div className="flex items-center justify-between border-b px-5 pb-4 pt-5 border-secondary">
<div>
<h2 className="flex items-baseline gap-1.5">
<div className="text-sans-semi-lg">{title}</div>
<div className="text-sans-md text-secondary">{label}</div>
</h2>
<div className="mt-0.5 text-sans-md text-secondary">{description}</div>
</div>
<ChartContainer>
<ChartHeader title={title} label={label} description={description}>
<MoreActionsMenu label="Instance actions" isSmall>
<Dropdown.LinkItem to={links.oxqlSchemaDocs(queryObj.metricName)}>
About this metric
Expand All @@ -102,22 +95,17 @@ export function OxqlMetric({ title, description, unit, ...queryObj }: OxqlMetric
>
<HighlightedOxqlQuery {...queryObj} />
</CopyCodeModal>
</div>
<div className="px-5 pb-5 pt-8">
<TimeSeriesChart
title={title}
startTime={startTime}
endTime={endTime}
unit={unitForSet}
data={data}
yAxisTickFormatter={yAxisTickFormatter}
width={480}
height={240}
hasBorder={false}
hasError={!!error}
/>
</div>
</div>
</ChartHeader>
<TimeSeriesChart
title={title}
startTime={startTime}
endTime={endTime}
unit={unitForSet}
data={data}
yAxisTickFormatter={yAxisTickFormatter}
hasError={!!error}
/>
</ChartContainer>
)
}

Expand Down
Loading
Loading