[feat: chart] implementation of dual y axis for bar, line or area chart#1241
Open
socallmebertille wants to merge 4 commits into
Open
[feat: chart] implementation of dual y axis for bar, line or area chart#1241socallmebertille wants to merge 4 commits into
socallmebertille wants to merge 4 commits into
Conversation
socallmebertille
force-pushed
the
feat/873-dual-axis-charts
branch
from
July 22, 2026 13:10
9c2b288 to
2b6a39f
Compare
Contributor
🚀 Preview Deployment
Preview will be automatically removed when this PR is closed. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 22 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
socallmebertille
force-pushed
the
feat/873-dual-axis-charts
branch
from
July 22, 2026 16:25
9e9c6d1 to
cdc584c
Compare
Contributor
There was a problem hiding this comment.
9 issues found across 22 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/frontend/src/components/tool-calls/display-chart.tsx">
<violation number="1" location="apps/frontend/src/components/tool-calls/display-chart.tsx:518">
P2: Regular area charts still use colliding `grad-${i}` SVG IDs, so this newly added prefix does not isolate all chart instances and can produce incorrect area fills when charts coexist. Thread the prefix through `buildAreaChart` as well, or otherwise give its gradients instance-scoped IDs.</violation>
</file>
<file name="apps/shared/src/tools/display-chart.ts">
<violation number="1" location="apps/shared/src/tools/display-chart.ts:276">
P3: `hasRightAxisSeries` is never used in the repository, so this new export currently adds dead code and a second source of truth for right-axis detection. Removing it or using it from the combo renderer would keep the axis logic centralized.</violation>
</file>
<file name="apps/shared/src/story-segments.ts">
<violation number="1" location="apps/shared/src/story-segments.ts:5">
P3: The parser now maintains a second copy of the chart series contract, so adding a series field to `SeriesConfigSchema` can silently leave story parsing out of sync. Reusing the exported `SeriesConfig` type would keep the parser and chart input contract aligned.</violation>
</file>
<file name="apps/shared/src/chart-builder.tsx">
<violation number="1" location="apps/shared/src/chart-builder.tsx:479">
P2: Numeric X axes stop working for bar charts: values are rendered as equally spaced categories instead of according to their numeric distance. Preserving `props.xAxisType` here keeps existing numeric bar charts correct while retaining category as the caller's default.</violation>
<violation number="2" location="apps/shared/src/chart-builder.tsx:706">
P2: Mixed charts containing a bar ignore `x_axis_type: "number"`, so numeric X values are no longer positioned proportionally. Passing the requested `xAxisType` through preserves continuous axes; Recharts supports numeric X axes in `ComposedChart` with bars.</violation>
<violation number="3" location="apps/shared/src/chart-builder.tsx:792">
P2: Mixed-chart data labels can be clipped at the top of the SVG or collide with the title because this new label path does not trigger the existing headroom reservation. Including `mixed` in the headroom calculation and accounting for its plotted series keeps enabled labels visible.</violation>
</file>
<file name="apps/frontend/src/components/tool-calls/display-chart-edit-dialog.tsx">
<violation number="1" location="apps/frontend/src/components/tool-calls/display-chart-edit-dialog.tsx:99">
P1: Y-axis min/max range fields are no longer shown in the edit dialog for standard chart types like bar, line, and area. Previously, users could set a fixed Y-axis lower/upper bound for any non-pie, non-radar, non-kpi_card chart. The new condition `isCombo && (hasLeftAxis || hasRightAxis)` only renders the axis section when the chart type is `'mixed'`, which drops min/max controls for all single-axis types. The `supportsYAxisRange` variable and the min/max state are still maintained, suggesting this is an oversight in the condition. Add a fallback that renders the axis fields for non-combo charts that support a range.</violation>
<violation number="2" location="apps/frontend/src/components/tool-calls/display-chart-edit-dialog.tsx:354">
P3: Keyboard and screen-reader users cannot reliably identify the icon-only axis-side control. Adding an `aria-label` such as `isRight ? 'Right Y-axis' : 'Left Y-axis'` to the `Button` would make the control discoverable without changing its visual UI.</violation>
<violation number="3" location="apps/frontend/src/components/tool-calls/display-chart-edit-dialog.tsx:715">
P3: A series without an explicit hex color can show a different swatch from the rendered chart. The theme uses OKLCH colors, but this conversion rejects any non-hex canvas result and falls back to hardcoded colors; converting CSS colors reliably to hex (or using a color input value that preserves the resolved CSS color) would keep the editor preview theme-consistent.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // the same swatch the chart draws for it. Refreshed on open for the theme. | ||
| const [paletteHexes, setPaletteHexes] = useState<string[]>(DEFAULT_COLORS); | ||
| const supportsYAxisRange = !Y_AXIS_RANGE_UNSUPPORTED_CHART_TYPES.has(draft.chart_type); | ||
| const isCombo = displayChart.chartTypeSupportsComboSeries(draft.chart_type); |
Contributor
There was a problem hiding this comment.
P1: Y-axis min/max range fields are no longer shown in the edit dialog for standard chart types like bar, line, and area. Previously, users could set a fixed Y-axis lower/upper bound for any non-pie, non-radar, non-kpi_card chart. The new condition isCombo && (hasLeftAxis || hasRightAxis) only renders the axis section when the chart type is 'mixed', which drops min/max controls for all single-axis types. The supportsYAxisRange variable and the min/max state are still maintained, suggesting this is an oversight in the condition. Add a fallback that renders the axis fields for non-combo charts that support a range.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/frontend/src/components/tool-calls/display-chart-edit-dialog.tsx, line 99:
<comment>Y-axis min/max range fields are no longer shown in the edit dialog for standard chart types like bar, line, and area. Previously, users could set a fixed Y-axis lower/upper bound for any non-pie, non-radar, non-kpi_card chart. The new condition `isCombo && (hasLeftAxis || hasRightAxis)` only renders the axis section when the chart type is `'mixed'`, which drops min/max controls for all single-axis types. The `supportsYAxisRange` variable and the min/max state are still maintained, suggesting this is an oversight in the condition. Add a fallback that renders the axis fields for non-combo charts that support a range.</comment>
<file context>
@@ -79,14 +89,25 @@ export function ChartConfigEditDialog({
+ // the same swatch the chart draws for it. Refreshed on open for the theme.
+ const [paletteHexes, setPaletteHexes] = useState<string[]>(DEFAULT_COLORS);
const supportsYAxisRange = !Y_AXIS_RANGE_UNSUPPORTED_CHART_TYPES.has(draft.chart_type);
+ const isCombo = displayChart.chartTypeSupportsComboSeries(draft.chart_type);
+ const hasRightAxis = isCombo && displayChart.hasRightAxisSeries(draft.series);
+ const hasLeftAxis = !isCombo || draft.series.some((s) => s.y_axis !== 'right');
</file context>
| labelFormatter, | ||
| showGrid, | ||
| showDataLabels, | ||
| idPrefix: gradientIdPrefix, |
Contributor
There was a problem hiding this comment.
P2: Regular area charts still use colliding grad-${i} SVG IDs, so this newly added prefix does not isolate all chart instances and can produce incorrect area fills when charts coexist. Thread the prefix through buildAreaChart as well, or otherwise give its gradients instance-scoped IDs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/frontend/src/components/tool-calls/display-chart.tsx, line 518:
<comment>Regular area charts still use colliding `grad-${i}` SVG IDs, so this newly added prefix does not isolate all chart instances and can produce incorrect area fills when charts coexist. Thread the prefix through `buildAreaChart` as well, or otherwise give its gradients instance-scoped IDs.</comment>
<file context>
@@ -500,9 +515,14 @@ export const ChartDisplay = memo(function ChartDisplay({
labelFormatter,
showGrid,
showDataLabels,
+ idPrefix: gradientIdPrefix,
margin: { top: 0, right: 0, bottom: 0, left: 0 },
yAxisMin,
</file context>
| radius={[4, 4, 4, 4]} | ||
| isAnimationActive={false} | ||
| > | ||
| {showDataLabels && <LabelList position='top' formatter={formatDataLabel} {...DATA_LABEL_PROPS} />} |
Contributor
There was a problem hiding this comment.
P2: Mixed-chart data labels can be clipped at the top of the SVG or collide with the title because this new label path does not trigger the existing headroom reservation. Including mixed in the headroom calculation and accounting for its plotted series keeps enabled labels visible.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/shared/src/chart-builder.tsx, line 792:
<comment>Mixed-chart data labels can be clipped at the top of the SVG or collide with the title because this new label path does not trigger the existing headroom reservation. Including `mixed` in the headroom calculation and accounting for its plotted series keeps enabled labels visible.</comment>
<file context>
@@ -599,6 +610,202 @@ function buildAreaChart(props: ResolvedProps) {
+ radius={[4, 4, 4, 4]}
+ isAnimationActive={false}
+ >
+ {showDataLabels && <LabelList position='top' formatter={formatDataLabel} {...DATA_LABEL_PROPS} />}
+ </Bar>
+ );
</file context>
| )} | ||
| {renderCategoryXAxis({ | ||
| xAxisKey, | ||
| xAxisType: hasBarSeries ? 'category' : xAxisType, |
Contributor
There was a problem hiding this comment.
P2: Mixed charts containing a bar ignore x_axis_type: "number", so numeric X values are no longer positioned proportionally. Passing the requested xAxisType through preserves continuous axes; Recharts supports numeric X axes in ComposedChart with bars.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/shared/src/chart-builder.tsx, line 706:
<comment>Mixed charts containing a bar ignore `x_axis_type: "number"`, so numeric X values are no longer positioned proportionally. Passing the requested `xAxisType` through preserves continuous axes; Recharts supports numeric X axes in `ComposedChart` with bars.</comment>
<file context>
@@ -599,6 +610,202 @@ function buildAreaChart(props: ResolvedProps) {
+ )}
+ {renderCategoryXAxis({
+ xAxisKey,
+ xAxisType: hasBarSeries ? 'category' : xAxisType,
+ xAxisInterval,
+ labelFormatter,
</file context>
| /> | ||
| )} | ||
| {renderCategoryXAxis({ xAxisKey, xAxisType, xAxisInterval, labelFormatter })} | ||
| {renderCategoryXAxis({ xAxisKey, xAxisType: 'category', xAxisInterval, labelFormatter })} |
Contributor
There was a problem hiding this comment.
P2: Numeric X axes stop working for bar charts: values are rendered as equally spaced categories instead of according to their numeric distance. Preserving props.xAxisType here keeps existing numeric bar charts correct while retaining category as the caller's default.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/shared/src/chart-builder.tsx, line 479:
<comment>Numeric X axes stop working for bar charts: values are rendered as equally spaced categories instead of according to their numeric distance. Preserving `props.xAxisType` here keeps existing numeric bar charts correct while retaining category as the caller's default.</comment>
<file context>
@@ -465,7 +476,7 @@ function buildBarChart(props: ResolvedProps) {
/>
)}
- {renderCategoryXAxis({ xAxisKey, xAxisType, xAxisInterval, labelFormatter })}
+ {renderCategoryXAxis({ xAxisKey, xAxisType: 'category', xAxisInterval, labelFormatter })}
{children}
{renderedSeries.map((s, i) => (
</file context>
Suggested change
| {renderCategoryXAxis({ xAxisKey, xAxisType: 'category', xAxisInterval, labelFormatter })} | |
| {renderCategoryXAxis({ xAxisKey, xAxisType: props.xAxisType, xAxisInterval, labelFormatter })} |
| return type === 'mixed'; | ||
| } | ||
|
|
||
| export function hasRightAxisSeries(series: Pick<SeriesConfig, 'y_axis'>[]): boolean { |
Contributor
There was a problem hiding this comment.
P3: hasRightAxisSeries is never used in the repository, so this new export currently adds dead code and a second source of truth for right-axis detection. Removing it or using it from the combo renderer would keep the axis logic centralized.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/shared/src/tools/display-chart.ts, line 276:
<comment>`hasRightAxisSeries` is never used in the repository, so this new export currently adds dead code and a second source of truth for right-axis detection. Removing it or using it from the combo renderer would keep the axis logic centralized.</comment>
<file context>
@@ -225,3 +268,15 @@ export function chartTypeRequiresXAxisKey(type: ChartType): boolean {
+ return type === 'mixed';
+}
+
+export function hasRightAxisSeries(series: Pick<SeriesConfig, 'y_axis'>[]): boolean {
+ return series.some((s) => s.y_axis === 'right');
+}
</file context>
| import { type ColumnConditionalFormats, sanitizeConditionalFormats } from './conditional-formatting'; | ||
| import type { SeriesType, YAxisSide } from './tools/display-chart'; | ||
|
|
||
| export interface ParsedChartSeries { |
Contributor
There was a problem hiding this comment.
P3: The parser now maintains a second copy of the chart series contract, so adding a series field to SeriesConfigSchema can silently leave story parsing out of sync. Reusing the exported SeriesConfig type would keep the parser and chart input contract aligned.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/shared/src/story-segments.ts, line 5:
<comment>The parser now maintains a second copy of the chart series contract, so adding a series field to `SeriesConfigSchema` can silently leave story parsing out of sync. Reusing the exported `SeriesConfig` type would keep the parser and chart input contract aligned.</comment>
<file context>
@@ -1,14 +1,28 @@
import { type ColumnConditionalFormats, sanitizeConditionalFormats } from './conditional-formatting';
+import type { SeriesType, YAxisSide } from './tools/display-chart';
+
+export interface ParsedChartSeries {
+ data_key: string;
+ color?: string;
</file context>
| onChange={(e) => updateSeriesAt(index, { color: e.target.value })} | ||
| className='h-8 w-8 cursor-pointer overflow-hidden rounded-lg border-none bg-transparent p-0 [&::-moz-color-swatch]:rounded-lg [&::-moz-color-swatch]:border-none [&::-webkit-color-swatch-wrapper]:p-0 [&::-webkit-color-swatch]:rounded-lg [&::-webkit-color-swatch]:border-none' | ||
| /> | ||
| <Button |
Contributor
There was a problem hiding this comment.
P3: Keyboard and screen-reader users cannot reliably identify the icon-only axis-side control. Adding an aria-label such as isRight ? 'Right Y-axis' : 'Left Y-axis' to the Button would make the control discoverable without changing its visual UI.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/frontend/src/components/tool-calls/display-chart-edit-dialog.tsx, line 354:
<comment>Keyboard and screen-reader users cannot reliably identify the icon-only axis-side control. Adding an `aria-label` such as `isRight ? 'Right Y-axis' : 'Left Y-axis'` to the `Button` would make the control discoverable without changing its visual UI.</comment>
<file context>
@@ -284,80 +317,117 @@ export function ChartConfigEditDialog({
+ onChange={(e) => updateSeriesAt(index, { color: e.target.value })}
+ className='h-8 w-8 cursor-pointer overflow-hidden rounded-lg border-none bg-transparent p-0 [&::-moz-color-swatch]:rounded-lg [&::-moz-color-swatch]:border-none [&::-webkit-color-swatch-wrapper]:p-0 [&::-webkit-color-swatch]:rounded-lg [&::-webkit-color-swatch]:border-none'
+ />
+ <Button
+ type='button'
+ size='icon-sm'
</file context>
| context.fillStyle = sentinel; | ||
| context.fillStyle = value; | ||
| const resolved = context.fillStyle; | ||
| return resolved !== sentinel && HEX_RE.test(resolved) ? resolved : fallback; |
Contributor
There was a problem hiding this comment.
P3: A series without an explicit hex color can show a different swatch from the rendered chart. The theme uses OKLCH colors, but this conversion rejects any non-hex canvas result and falls back to hardcoded colors; converting CSS colors reliably to hex (or using a color input value that preserves the resolved CSS color) would keep the editor preview theme-consistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/frontend/src/components/tool-calls/display-chart-edit-dialog.tsx, line 715:
<comment>A series without an explicit hex color can show a different swatch from the rendered chart. The theme uses OKLCH colors, but this conversion rejects any non-hex canvas result and falls back to hardcoded colors; converting CSS colors reliably to hex (or using a color input value that preserves the resolved CSS color) would keep the editor preview theme-consistent.</comment>
<file context>
@@ -492,11 +690,30 @@ function parseRangeInput(value: string): number | undefined {
+ context.fillStyle = sentinel;
+ context.fillStyle = value;
+ const resolved = context.fillStyle;
+ return resolved !== sentinel && HEX_RE.test(resolved) ? resolved : fallback;
+ });
}
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mixedtype of chart that can draw 3 different types of chart mixed together with 2 possible y axis :series_type— render the serie as bar, line or area and ignore the initialchart_type, which enable multiple types in a unique charty_axis—left|rightComposedChart(byrecharts, that automatically decide of the min and max axis border if not specified by user or agent)Demo
Related issue
#873