mirror of
https://github.com/supabase/supabase.git
synced 2026-10-05 09:25:06 +03:00
fix(studio): stop stacking overlaid area series in report charts (#48034)
Line-style report charts (auth processing time, percentiles, edge functions, realtime, etc.) hardcoded `stackId="1"` on every `<Area>`, so recharts summed the series additively instead of overlaying them. When multiple series share a value (e.g. Max/Min/Avg all `153.98`), they rendered as three stacked bands at 1x/2x/3x the value, even though the tooltip showed the true identical values. ## Fix Default each area to its own `stackId` (its attribute name) so series overlay, while still honoring an explicit per-attribute `stackId` from config — matching the existing bar-chart path directly above it. `normalizeVisibleStackToPercent`, the only flag that would make stacked areas intentional, is never enabled anywhere. ## Before / after Before: three equal values drawn at `0→153.98`, `153.98→307.96`, `307.96→461.94`. After: all three overlay at `153.98`. Affects every line-style multi-series report, not just auth. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved stacked chart behavior by consistently applying configured series stacking settings. - Ensured area charts can overlay correctly when no explicit stacking configuration is provided. - Added safeguards for missing or invalid chart attribute data. - **Tests** - Added coverage for configured stack IDs, fallback behavior, empty values, and invalid inputs. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
1 parent
76f44bfa71
commit
c166bfc044
3 files changed
+83
-2
No files matched your search
@@ -38,6 +38,7 @@ import {
|
||||
calculateTotalChartAggregate,
|
||||
CustomLabel,
|
||||
CustomTooltip,
|
||||
getStackId,
|
||||
MultiAttribute,
|
||||
} from './ComposedChart.utils'
|
||||
import NoDataPlaceholder from './NoDataPlaceholder'
|
||||
@@ -560,7 +561,7 @@ export function ComposedChart({
|
||||
<Bar
|
||||
key={attribute.name}
|
||||
dataKey={attribute.name}
|
||||
stackId={attributes?.find((a) => a.attribute === attribute?.name)?.stackId ?? '1'}
|
||||
stackId={getStackId(attributes, attribute?.name, '1')}
|
||||
fill={attribute.color}
|
||||
radius={0.75}
|
||||
opacity={1}
|
||||
@@ -576,7 +577,7 @@ export function ComposedChart({
|
||||
key={attribute.name}
|
||||
type="linear"
|
||||
dataKey={attribute.name}
|
||||
stackId="1"
|
||||
stackId={getStackId(attributes, attribute.name, attribute.name)}
|
||||
fill={`url(#gradient-${attribute.name})`}
|
||||
fillOpacity={1}
|
||||
stroke={attribute.color}
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { getStackId } from './ComposedChart.utils'
|
||||
import type { MultiAttribute } from './ComposedChart.utils'
|
||||
|
||||
const attr = (attribute: string, stackId?: string): MultiAttribute => ({ attribute, stackId })
|
||||
|
||||
describe('getStackId', () => {
|
||||
it('returns the explicit stackId when the attribute configures one', () => {
|
||||
const attributes = [attr('ingress', 'traffic'), attr('egress', 'traffic')]
|
||||
expect(getStackId(attributes, 'ingress', '1')).toBe('traffic')
|
||||
expect(getStackId(attributes, 'egress', '1')).toBe('traffic')
|
||||
})
|
||||
|
||||
it('falls back when the attribute has no stackId (overlay case)', () => {
|
||||
const attributes = [attr('avg'), attr('min'), attr('max')]
|
||||
expect(getStackId(attributes, 'avg', 'avg')).toBe('avg')
|
||||
expect(getStackId(attributes, 'min', 'min')).toBe('min')
|
||||
expect(getStackId(attributes, 'max', 'max')).toBe('max')
|
||||
})
|
||||
|
||||
it('falls back to the shared bar id when no stackId is configured', () => {
|
||||
const attributes = [attr('reads'), attr('writes')]
|
||||
expect(getStackId(attributes, 'reads', '1')).toBe('1')
|
||||
expect(getStackId(attributes, 'writes', '1')).toBe('1')
|
||||
})
|
||||
|
||||
it('falls back when the attribute is not found', () => {
|
||||
expect(getStackId([attr('reads', 'io')], 'writes', 'fallback')).toBe('fallback')
|
||||
})
|
||||
|
||||
it('does not crash on undefined or null attributes', () => {
|
||||
expect(getStackId(undefined, 'reads', '1')).toBe('1')
|
||||
expect(getStackId(null, 'reads', '1')).toBe('1')
|
||||
})
|
||||
|
||||
it('does not crash on undefined or null name', () => {
|
||||
expect(getStackId([attr('reads', 'io')], undefined, '1')).toBe('1')
|
||||
expect(getStackId([attr('reads', 'io')], null, '1')).toBe('1')
|
||||
})
|
||||
|
||||
it('does not crash on falsy entries in the attributes array', () => {
|
||||
const attributes: (MultiAttribute | false | null | undefined)[] = [
|
||||
false,
|
||||
null,
|
||||
undefined,
|
||||
attr('reads', 'io'),
|
||||
]
|
||||
expect(getStackId(attributes, 'reads', '1')).toBe('io')
|
||||
expect(getStackId(attributes, 'writes', '1')).toBe('1')
|
||||
})
|
||||
|
||||
it('does not crash on a non-array value', () => {
|
||||
expect(getStackId('nope' as unknown as MultiAttribute[], 'reads', '1')).toBe('1')
|
||||
})
|
||||
|
||||
it('treats an empty-string stackId as configured', () => {
|
||||
expect(getStackId([attr('reads', '')], 'reads', '1')).toBe('')
|
||||
})
|
||||
})
|
||||
@@ -127,6 +127,26 @@ interface TooltipProps {
|
||||
|
||||
const isMaxAttribute = (attributes?: MultiAttribute[]) => attributes?.find((a) => a.isMaxValue)
|
||||
|
||||
/**
|
||||
* Resolve the recharts `stackId` for a series.
|
||||
*
|
||||
* Series that share a `stackId` are stacked additively, so overlaid series
|
||||
* (e.g. min/max/avg of the same metric) must each get a distinct id. Bar
|
||||
* charts pass `'1'` as the fallback to stack together; area charts pass the
|
||||
* attribute name so each series overlays independently. An explicit per-
|
||||
* attribute `stackId` always wins.
|
||||
*/
|
||||
export const getStackId = (
|
||||
attributes: (MultiAttribute | false | null | undefined)[] | null | undefined,
|
||||
name: string | null | undefined,
|
||||
fallback: string
|
||||
): string => {
|
||||
const configured = Array.isArray(attributes)
|
||||
? attributes.find((a): a is MultiAttribute => !!a && a.attribute === name)?.stackId
|
||||
: undefined
|
||||
return configured ?? fallback
|
||||
}
|
||||
|
||||
/**
|
||||
* Calculate the total aggregate of the chart values
|
||||
* by summing the values of the attributes
|
||||
|
||||
Reference in new issue
Block a user