From 941fcb94ad6511641cec188798b3ddea49343c6c Mon Sep 17 00:00:00 2001 From: Talisson Costa Date: Fri, 4 Sep 2026 14:34:52 -0300 Subject: [PATCH 1/2] feat(charts): let ColorSwatch fade Series drawn with an SVG fill-opacity had no matching swatch: colours are CSS var() strings, so a className can't carry a per-series alpha. Adds an optional opacity, and a story alongside the other prop variants. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/ColorSwatch.stories.tsx | 23 +++++++++++++++++++ frontend/web/components/ColorSwatch.tsx | 7 ++++++ 2 files changed, 30 insertions(+) diff --git a/frontend/documentation/components/ColorSwatch.stories.tsx b/frontend/documentation/components/ColorSwatch.stories.tsx index a3f78aac4c49..4fb76c2d3bf8 100644 --- a/frontend/documentation/components/ColorSwatch.stories.tsx +++ b/frontend/documentation/components/ColorSwatch.stories.tsx @@ -115,6 +115,29 @@ export const Shapes: Story = { ), } +export const Faded: Story = { + parameters: { + docs: { + description: { + story: + '`opacity` fades the swatch to match a series drawn with an SVG `fill-opacity`, such as the remainder segment of a part-of-whole bar. Colours can be CSS `var()` strings, so transparency cannot come from an alpha channel.', + }, + }, + }, + render: () => ( +
+
+ + Converted +
+
+ + Exposures +
+
+ ), +} + export const Palette: Story = { parameters: { docs: { diff --git a/frontend/web/components/ColorSwatch.tsx b/frontend/web/components/ColorSwatch.tsx index 3b2ffa5e4e10..813cb9cbbffe 100644 --- a/frontend/web/components/ColorSwatch.tsx +++ b/frontend/web/components/ColorSwatch.tsx @@ -9,6 +9,11 @@ type ColorSwatchProps = { size?: ColorSwatchSize shape?: ColorSwatchShape className?: string + /** + * Fade the swatch, for series drawn with an SVG `fill-opacity`. Colours can + * be CSS `var()` strings, so transparency can't come from an alpha channel. + */ + opacity?: number } const SIZE_MAP: Record = { @@ -25,6 +30,7 @@ const SHAPE_CLASS: Record = { const ColorSwatch: FC = ({ className, color, + opacity, shape = 'square', size = 'md', }) => ( @@ -38,6 +44,7 @@ const ColorSwatch: FC = ({ style={{ backgroundColor: color, height: SIZE_MAP[size], + opacity, width: SIZE_MAP[size], }} /> From 669a20881c80a47fba78c66c87d20a59e184caf5 Mon Sep 17 00:00:00 2001 From: Talisson Costa Date: Fri, 4 Sep 2026 14:35:04 -0300 Subject: [PATCH 2/2] refactor(charts): describe bar series as objects, not parallel maps BarChart took four maps keyed by dataKey (colours, labels, stack ids, fill opacities) with nothing tying the keys together, so a missing entry silently dropped a bar's colour or stack. One BarSeries per series replaces all four, and the legend now reads the same array the bars do, so a faded swatch can't drift from its faded bar. Two props go with it. `grouped` was unreachable: side by side is distinct stack ids, which is what the conversion chart's daily mode already did. The tooltip's formatter and its total flag become one object, with a formatted value hiding the non-additive total by default rather than asking every caller to remember. Existing charts keep their shared default stack, so nothing moves visually. toBarSeries bridges callers still holding maps from buildChartColorMap or useEnvChartProps, which LineChart continues to read. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/BarChart.stories.tsx | 104 +++++++++++- frontend/web/components/charts/BarChart.tsx | 159 +++++++----------- frontend/web/components/charts/index.ts | 3 +- frontend/web/components/charts/toBarSeries.ts | 17 ++ frontend/web/components/charts/types.ts | 19 +++ .../ExperimentConversionRateCard.tsx | 7 +- .../__tests__/deriveConversionRate.test.ts | 33 ++-- .../results/deriveConversionRate.ts | 52 +++--- .../FeatureNavTab/FeatureAnalytics.tsx | 12 +- .../usage/OrganisationUsage.container.tsx | 5 +- .../usage/components/SingleSDKLabelsChart.tsx | 4 +- .../UsageOverTime/UsageOverTime.tsx | 6 +- 12 files changed, 254 insertions(+), 167 deletions(-) create mode 100644 frontend/web/components/charts/toBarSeries.ts diff --git a/frontend/documentation/components/BarChart.stories.tsx b/frontend/documentation/components/BarChart.stories.tsx index b0549afed5dc..9992ba24099c 100644 --- a/frontend/documentation/components/BarChart.stories.tsx +++ b/frontend/documentation/components/BarChart.stories.tsx @@ -3,6 +3,8 @@ import type { Meta, StoryObj } from 'storybook' import BarChart from 'components/charts/BarChart' import { MultiSelect } from 'components/base/select/multi-select' import { buildChartColorMap } from 'components/charts/buildChartColorMap' +import { toBarSeries } from 'components/charts/toBarSeries' +import type { BarSeries, ChartDataPoint } from 'components/charts/types' import { generateChartFakeData } from './_chartFakeData' // ============================================================================ @@ -35,6 +37,51 @@ const generateFakeData = (days: number, labels: string[]) => weekendDip: 0.4, }) +// Cumulative exposures per variant with a fixed share converted, the shape the +// experiment conversion chart plots: the faded segment is the remainder, so the +// full bar is the denominator and the solid part is the numerator. +const CONVERSION_SHARE: Record = { + control: 0.12, + variant_a: 0.21, +} + +const buildPartOfWholeData = (): ChartDataPoint[] => { + const variants = Object.keys(CONVERSION_SHARE) + const running: Record = { control: 0, variant_a: 0 } + return generateChartFakeData({ + days: 14, + defaultBase: 300, + labels: variants, + variance: 0.6, + }).map((point) => { + const stacked: ChartDataPoint = { day: point.day } + variants.forEach((key) => { + running[key] += Number(point[key]) + const converted = Math.round(running[key] * CONVERSION_SHARE[key]) + stacked[key] = converted + stacked[`${key}-rest`] = running[key] - converted + }) + return stacked + }) +} + +const buildPartOfWholeSeries = (): BarSeries[] => { + const colours = buildChartColorMap(Object.keys(CONVERSION_SHARE)) + return [ + { key: 'control', label: 'Control converted', name: 'Control' }, + { key: 'variant_a', label: 'Variant A converted', name: 'Variant A' }, + ].flatMap(({ key, label, name }) => [ + { colour: colours[key], key, label, stackId: key }, + { + colour: colours[key], + key: `${key}-rest`, + label: `${name} exposures`, + opacity: 0.25, + stackId: key, + }, + ]) +} + // ============================================================================ // Stories // ============================================================================ @@ -80,8 +127,7 @@ export const WithLabelledBuckets: Story = { @@ -105,8 +151,7 @@ export const WithoutLabels: Story = {

@@ -116,6 +161,54 @@ export const WithoutLabels: Story = { ], } +export const PartOfWholeStacks: Story = { + decorators: [ + () => { + const data = useMemo(() => buildPartOfWholeData(), []) + const series = useMemo(() => buildPartOfWholeSeries(), []) + const counts = (label: string, key: string) => { + const point = data.find((p) => p.day === label) + const converted = Number(point?.[key.replace('-rest', '')] ?? 0) + const rest = Number(point?.[`${key.replace('-rest', '')}-rest`] ?? 0) + return { converted, exposed: converted + rest } + } + + return ( +
+

+ Part-of-whole stacks: cumulative exposures per variant with the + converted share filled in. +

+ { + const { converted, exposed } = counts(label, seriesKey) + if (seriesKey.endsWith('-rest')) return exposed.toLocaleString() + const rate = exposed ? (converted / exposed) * 100 : 0 + return `${converted.toLocaleString()} of ${exposed.toLocaleString()} (${rate.toFixed( + 1, + )}%)` + }, + }} + /> +
+ ) + }, + ], + parameters: { + docs: { + description: { + story: + "Series sharing a `stackId` stack into one bar; distinct ids sit side by side. Giving the remainder segment an `opacity` fades it, and the legend swatch fades with it (recharts' own legend swatch ignores `fillOpacity`, so the chart renders its own key). `tooltip.formatValue` reports the pair, and a formatted value hides the total row by default since it is no longer additive.", + }, + }, + }, +} + export const SingleSeries: Story = { decorators: [ () => { @@ -130,8 +223,7 @@ export const SingleSeries: Story = {

diff --git a/frontend/web/components/charts/BarChart.tsx b/frontend/web/components/charts/BarChart.tsx index ccf00e395b3f..7ca1bb8e7804 100644 --- a/frontend/web/components/charts/BarChart.tsx +++ b/frontend/web/components/charts/BarChart.tsx @@ -10,98 +10,70 @@ import { YAxis, } from 'recharts' import { colorTextSecondary } from 'common/theme/tokens' +import ColorSwatch from 'components/ColorSwatch' import ChartTooltip from './ChartTooltip' -import { ChartDataPoint } from './types' +import { BarSeries, ChartDataPoint } from './types' + +// Series with no stack id of their own share this one, so the default shape is +// a single stacked bar per x value. +const DEFAULT_STACK_ID = 'series' + +type BarChartTooltipProps = { + /** + * Per-entry value renderer, e.g. `"120 of 1,450 (8.3%)"`. Skipped for + * missing or non-numeric values, which render blank. + */ + formatValue?: (value: number, seriesKey: string, label: string) => string + /** + * Hide the total row. A formatted value is usually not additive (a + * percentage, an "x of y"), so `formatValue` hides the total by default; + * pass `false` to keep it. + */ + hideTotal?: boolean +} type BarChartProps = { data: ChartDataPoint[] - series: string[] - colorMap: Record - xAxisInterval?: number /** - * Render recharts' built-in `` below the chart. Default `false` — - * most consumers already expose a coloured filter UI (tags / MultiSelect) - * that serves the same purpose, so a second legend is redundant and can - * display raw dataKeys (e.g. numeric env IDs) that are meaningless to users. + * One entry per bar series, in render order. `key` is the dataKey to read + * from each `data` point; `stackId` and `opacity` shape how it draws. */ - showLegend?: boolean + series: BarSeries[] + xAxisInterval?: number /** - * Optional dataKey → display name map, threaded through to the tooltip (and - * the legend when enabled). Use this when dataKeys are opaque identifiers - * (e.g. numeric env ids) that need a human-readable label on display. + * Render a legend below the chart. Default `false` — most consumers already + * expose a coloured filter UI (tags / MultiSelect) that serves the same + * purpose, so a second legend is redundant. */ - seriesLabels?: Record + showLegend?: boolean /** Fixed bar width in pixels. Default: recharts auto-sizes by available space. */ barSize?: number /** Render vertical grid lines (one per x tick). Default `true`. */ verticalGrid?: boolean /** Chart height in pixels. Default 400. */ height?: number - /** - * Render series side by side instead of stacked. Required for non-additive - * values (rates, percentages) where stacking would be meaningless. - */ - grouped?: boolean - /** - * dataKey → stack id, for part-of-whole bars: series sharing a stack id - * stack together, distinct ids sit side by side (e.g. converted/remainder - * segments stacked per variant, variants grouped). Overrides `grouped`. - */ - stackMap?: Record - /** - * dataKey → fill opacity (0–1). Colours are CSS `var()` strings, so - * transparency must come from SVG fill-opacity, not an alpha channel. - */ - opacityMap?: Record - /** Left axis overrides, e.g. a `%` tick formatter. */ - yAxis?: { - tickFormatter?: (value: number) => string - domain?: [number, number] - } - /** - * Per-entry tooltip value renderer, threaded to ChartTooltip. Skipped for - * missing or non-numeric values, which render blank. - */ - tooltipValueFormatter?: ( - value: number, - seriesKey: string, - label: string, - ) => string - /** - * Hide the tooltip's total row — required when `tooltipValueFormatter` - * renders a non-additive unit such as a percentage. - */ - tooltipHideTotal?: boolean + tooltip?: BarChartTooltipProps } -type FadedSwatchLegendProps = { - opacityMap: Record - seriesLabels?: Record +type BarChartLegendProps = { + series: BarSeries[] // Injected by recharts' . payload?: { value?: string | number; color?: string }[] } -const FadedSwatchLegend: FC = ({ - opacityMap, - payload, - seriesLabels, -}) => ( +// recharts' own legend swatch ignores fillOpacity, so a chart with faded +// series needs this to keep the key and the bars looking the same. +const FadedSwatchLegend: FC = ({ payload, series }) => (
{payload?.map((entry) => { const key = String(entry.value) + const bar = series.find((s) => s.key === key) + const colour = bar?.colour ?? entry.color ?? '' return ( - - - {seriesLabels?.[key] ?? key} + + + {bar?.label ?? key} ) @@ -111,22 +83,19 @@ const FadedSwatchLegend: FC = ({ const BarChart: FC = ({ barSize, - colorMap, data, - grouped = false, height = 400, - opacityMap, series, - seriesLabels, showLegend = false, - stackMap, - tooltipHideTotal, - tooltipValueFormatter, + tooltip, verticalGrid = true, xAxisInterval = 0, - yAxis, }) => { - const defaultStackId = grouped ? undefined : 'series' + const labels = series.reduce>((acc, s) => { + acc[s.key] = s.label + return acc + }, {}) + const hasFadedSeries = series.some((s) => s.opacity !== undefined) return ( @@ -149,48 +118,36 @@ const BarChart: FC = ({ - value >= 1000 ? `${(value / 1000).toFixed(0)}k` : value) + tickFormatter={(value) => + value >= 1000 ? `${(value / 1000).toFixed(0)}k` : value } /> } /> {showLegend && ( - seriesLabels?.[String(value)] ?? String(value) - } + formatter={(value) => labels[String(value)] ?? String(value)} content={ - // The default legend swatch ignores fillOpacity, so faded - // series need their own renderer to match the bars. - opacityMap ? ( - - ) : undefined + hasFadedSeries ? : undefined } /> )} - {series.map((label, index) => ( + {series.map((s, index) => ( , + seriesLabels?: Record, +): BarSeries[] => + keys.map((key) => ({ + colour: colorMap[key], + key, + label: seriesLabels?.[key] ?? key, + })) diff --git a/frontend/web/components/charts/types.ts b/frontend/web/components/charts/types.ts index 7e77b5ce49d1..3f4cef85ddcc 100644 --- a/frontend/web/components/charts/types.ts +++ b/frontend/web/components/charts/types.ts @@ -2,3 +2,22 @@ export type ChartDataPoint = { day: string [key: string]: string | number } + +export type BarSeries = { + /** dataKey to read from each `ChartDataPoint`. */ + key: string + label: string + colour: string + /** + * Bars sharing a stack id stack on top of each other; distinct ids sit side + * by side. Defaults to one shared stack, so a chart that says nothing gets a + * single stacked bar per x value. + */ + stackId?: string + /** + * SVG fill-opacity (0-1), for the faded part of a part-of-whole bar. Colours + * can be CSS `var()` strings, so transparency can't come from an alpha + * channel. The legend swatch matches it. + */ + opacity?: number +} diff --git a/frontend/web/components/experiments/results/ExperimentConversionRateCard/ExperimentConversionRateCard.tsx b/frontend/web/components/experiments/results/ExperimentConversionRateCard/ExperimentConversionRateCard.tsx index 671b8a983220..8644b7510ef1 100644 --- a/frontend/web/components/experiments/results/ExperimentConversionRateCard/ExperimentConversionRateCard.tsx +++ b/frontend/web/components/experiments/results/ExperimentConversionRateCard/ExperimentConversionRateCard.tsx @@ -117,16 +117,11 @@ const ExperimentConversionRateCard: FC = ({ />
{asOf diff --git a/frontend/web/components/experiments/results/__tests__/deriveConversionRate.test.ts b/frontend/web/components/experiments/results/__tests__/deriveConversionRate.test.ts index f1aac9501b95..ba59ba756639 100644 --- a/frontend/web/components/experiments/results/__tests__/deriveConversionRate.test.ts +++ b/frontend/web/components/experiments/results/__tests__/deriveConversionRate.test.ts @@ -3,6 +3,7 @@ import { buildConversionRateChartData, buildConversionStackChartData, } from 'components/experiments/results/deriveConversionRate' +import type { ConversionStackChartData } from 'components/experiments/results/deriveConversionRate' import type { VariantIdentity } from 'components/experiments/results/derive' import { BayesianMetricResult, @@ -204,6 +205,9 @@ describe('buildConversionRateChartData', () => { }) }) +const seriesFor = (chart: ConversionStackChartData | null, key: string) => + chart?.series.find((s) => s.key === key) + describe('buildConversionStackChartData', () => { const exposures = exposuresTs([ { @@ -273,12 +277,14 @@ describe('buildConversionStackChartData', () => { }, ]) // Segments stack per variant; the remainder fades the variant colour. - expect(chart?.stackMap[`control${REST_SUFFIX}`]).toBe('control') - expect(chart?.stackMap.control).toBe('control') - expect(chart?.opacityMap[`control${REST_SUFFIX}`]).toBe(0.25) - expect(chart?.seriesLabels[`control${REST_SUFFIX}`]).toBe( - 'Control exposures', - ) + expect(seriesFor(chart, 'control')?.stackId).toBe('control') + expect(seriesFor(chart, `control${REST_SUFFIX}`)).toEqual({ + colour: '#111111', + key: `control${REST_SUFFIX}`, + label: 'Control exposures', + opacity: 0.25, + stackId: 'control', + }) }) it('plots raw per-bucket increments side by side in daily mode', () => { @@ -309,12 +315,13 @@ describe('buildConversionStackChartData', () => { // Daily quantities are not part-of-whole (a day's first conversions can // exceed its new exposures), so each series gets its own stack // (side-by-side bars) and the faded series becomes "new exposures". - expect(chart?.stackMap.control).toBe('control') - expect(chart?.stackMap[`control${REST_SUFFIX}`]).toBe( - `control${REST_SUFFIX}`, - ) - expect(chart?.seriesLabels[`control${REST_SUFFIX}`]).toBe( - 'Control new exposures', - ) + expect(seriesFor(chart, 'control')?.stackId).toBe('control') + expect(seriesFor(chart, `control${REST_SUFFIX}`)).toEqual({ + colour: '#111111', + key: `control${REST_SUFFIX}`, + label: 'Control new exposures', + opacity: 0.25, + stackId: `control${REST_SUFFIX}`, + }) }) }) diff --git a/frontend/web/components/experiments/results/deriveConversionRate.ts b/frontend/web/components/experiments/results/deriveConversionRate.ts index 932f4790e36c..b9ab440da7a7 100644 --- a/frontend/web/components/experiments/results/deriveConversionRate.ts +++ b/frontend/web/components/experiments/results/deriveConversionRate.ts @@ -1,5 +1,5 @@ import moment from 'moment' -import { ChartDataPoint } from 'components/charts' +import { BarSeries, ChartDataPoint } from 'components/charts' import { BayesianResultsSummary, ConversionsTimeseries, @@ -156,11 +156,7 @@ export type ConversionStackMode = 'cumulative' | 'daily' export type ConversionStackChartData = { points: ChartDataPoint[] - series: string[] - seriesLabels: Record - colorMap: Record - opacityMap: Record - stackMap: Record + series: BarSeries[] } // Stacked-bar encodings of exposures vs conversions per variant. @@ -180,27 +176,29 @@ export const buildConversionStackChartData = ( if (!exposures || !conversions) return null const daily = mode === 'daily' - const series: string[] = [] - const seriesLabels: Record = {} - const colorMap: Record = {} - const opacityMap: Record = {} - const stackMap: Record = {} - identities.forEach((v) => { + const series: BarSeries[] = identities.flatMap((v) => { const restKey = `${v.key}${REST_SUFFIX}` - series.push(v.key, restKey) - seriesLabels[v.key] = daily - ? `${v.name} conversions` - : `${v.name} converted` - seriesLabels[restKey] = daily - ? `${v.name} new exposures` - : `${v.name} exposures` - colorMap[v.key] = v.colour - // The faded series reuses the variant colour via fill-opacity (palette - // colours are CSS var() strings, so no alpha channel can be appended). - colorMap[restKey] = v.colour - opacityMap[restKey] = 0.25 - stackMap[v.key] = v.key - stackMap[restKey] = daily ? restKey : v.key + return [ + { + colour: v.colour, + key: v.key, + label: daily ? `${v.name} conversions` : `${v.name} converted`, + stackId: v.key, + }, + { + // The faded segment reuses the variant colour via fill-opacity + // (palette colours are CSS var() strings, so no alpha channel can be + // appended). + colour: v.colour, + key: restKey, + label: daily ? `${v.name} new exposures` : `${v.name} exposures`, + opacity: 0.25, + // Cumulative segments are part-of-whole, so they share the variant's + // stack. Daily ones are not (a day's first conversions can exceed its + // new exposures), so they sit side by side. + stackId: daily ? restKey : v.key, + }, + ] }) const points: ChartDataPoint[] = accumulateBuckets( @@ -217,5 +215,5 @@ export const buildConversionStackChartData = ( }) return point }) - return { colorMap, opacityMap, points, series, seriesLabels, stackMap } + return { points, series } } diff --git a/frontend/web/components/feature-page/FeatureNavTab/FeatureAnalytics.tsx b/frontend/web/components/feature-page/FeatureNavTab/FeatureAnalytics.tsx index 729a785fff50..d6b8c78515eb 100644 --- a/frontend/web/components/feature-page/FeatureNavTab/FeatureAnalytics.tsx +++ b/frontend/web/components/feature-page/FeatureNavTab/FeatureAnalytics.tsx @@ -3,6 +3,7 @@ import EmptyState from 'components/EmptyState' import InfoMessage from 'components/InfoMessage' import EnvironmentTagSelect from 'components/EnvironmentTagSelect' import BarChart from 'components/charts/BarChart' +import { toBarSeries } from 'components/charts/toBarSeries' import { MultiSelect } from 'components/base/select/multi-select' import { useGetFeatureAnalyticsQuery } from 'common/services/useFeatureAnalytics' import Utils from 'common/utils/utils' @@ -136,17 +137,18 @@ const FlagAnalytics: FC = ({ (isLabelled ? ( ) : ( ))} diff --git a/frontend/web/components/organisation-settings/usage/OrganisationUsage.container.tsx b/frontend/web/components/organisation-settings/usage/OrganisationUsage.container.tsx index a98db6fd613c..d139f411de3b 100644 --- a/frontend/web/components/organisation-settings/usage/OrganisationUsage.container.tsx +++ b/frontend/web/components/organisation-settings/usage/OrganisationUsage.container.tsx @@ -3,6 +3,7 @@ import moment from 'moment' import { AggregateUsageDataItem } from 'common/types/responses' import EmptyState from 'components/EmptyState' import BarChart, { ChartDataPoint } from 'components/charts/BarChart' +import { toBarSeries } from 'components/charts/toBarSeries' import UsageAPIDefinitions from './components/UsageAPIDefinitions' type OrganisationUsageProps = { @@ -94,9 +95,7 @@ const OrganisationUsage: FC = ({ ) : ( 31 ? 7 : 0} barSize={14} verticalGrid={false} diff --git a/frontend/web/components/organisation-settings/usage/components/SingleSDKLabelsChart.tsx b/frontend/web/components/organisation-settings/usage/components/SingleSDKLabelsChart.tsx index 53ad7720f989..9f8caa00784e 100644 --- a/frontend/web/components/organisation-settings/usage/components/SingleSDKLabelsChart.tsx +++ b/frontend/web/components/organisation-settings/usage/components/SingleSDKLabelsChart.tsx @@ -1,5 +1,6 @@ import React, { FC } from 'react' import BarChart, { ChartDataPoint } from 'components/charts/BarChart' +import { toBarSeries } from 'components/charts/toBarSeries' import EmptyState from 'components/EmptyState' interface SingleSDKLabelsChartProps { @@ -23,8 +24,7 @@ const SingleSDKLabelsChart: FC = ({ {hasData ? ( 31 ? 7 : 0} showLegend /> diff --git a/frontend/web/components/pages/usage/components/UsageOverTime/UsageOverTime.tsx b/frontend/web/components/pages/usage/components/UsageOverTime/UsageOverTime.tsx index 0b2e07ce2271..8f68c0ba448e 100644 --- a/frontend/web/components/pages/usage/components/UsageOverTime/UsageOverTime.tsx +++ b/frontend/web/components/pages/usage/components/UsageOverTime/UsageOverTime.tsx @@ -51,9 +51,9 @@ const UsageOverTime: FC = ({ ) : (