Skip to content

feat(charts): let LineChart pin the y-axis ceiling - #67

Open
gitlawr wants to merge 2 commits into
gpustack:mainfrom
gitlawr:line-chart-yaxis-max
Open

feat(charts): let LineChart pin the y-axis ceiling#67
gitlawr wants to merge 2 commits into
gpustack:mainfrom
gitlawr:line-chart-yaxis-max

Conversation

@gitlawr

@gitlawr gitlawr commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Two small LineChart capabilities, both driven by metric charts that plot
ratio and sparse time series.

yAxisMax

A new yAxisMax prop pins the y-axis ceiling. Ratio charts (hit rate,
usage ratio) need a fixed 0–100 axis: with auto-scaling, a series
hovering at 59% fills the frame and reads as saturation. The value is
passed through as yAxis.max ?? undefined, so a null from an untyped
caller falls back to ECharts' auto-scaling.

Per-series showSymbol

Series data can now opt back into symbol rendering: the shared line
config sets showSymbol: false, and previously overrode any
series-provided value. A series with gaps can carry isolated points no
line segment reaches (e.g. a single sample from one traffic burst on a
wide time window) — the caller marks those points with per-item
symbol/symbolSize and sets showSymbol on the series, and they
render instead of existing only in the hover tooltip. Continuous series
are unaffected.

Both are additive; existing callers see no behavior change.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new optional yAxisMax prop to the LineChart component, allowing users to pin the y-axis ceiling. The feedback suggests using the nullish coalescing operator (yAxisMax ?? undefined) when assigning this value to the ECharts configuration to safely handle null values and ensure proper auto-scaling fallback.

Comment thread src/lib/components/echarts/line-chart.tsx Outdated
Percentage charts read wrong when the axis scales to the data — 59%
usage fills the frame like 100% would. An optional yAxisMax pins the
ceiling; unset keeps the adaptive axis.
A series with gaps can carry isolated points no line segment reaches;
callers mark them with per-item symbols and flip showSymbol on the
series, which the shared line config used to override.
@gitlawr
gitlawr force-pushed the line-chart-yaxis-max branch from 99cf559 to 58c8762 Compare August 26, 2026 10:22
@gitlawr

gitlawr commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new yAxisMax prop to the LineChart component and its type definitions, allowing users to pin the y-axis ceiling. It also updates the series mapping to preserve individual item showSymbol configurations. The reviewer suggested a minor cleanup to remove a redundant nullish coalescing operator (?? undefined) when setting the max property on the y-axis, as yAxisMax is already typed as optional.

Comment thread src/lib/components/echarts/line-chart.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant