From ffc5b2ac063e74625637d6059302295fc233cf7e Mon Sep 17 00:00:00 2001 From: Andrei Zhaleznichenka Date: Tue, 23 Apr 2024 13:28:58 +0200 Subject: [PATCH] revert: Partially rounded corners for stacked bar charts (#2190) --- .../__tests__/utils.test.ts | 179 ++++++++---------- src/mixed-line-bar-chart/bar-series.tsx | 130 ++++--------- src/mixed-line-bar-chart/data-series.tsx | 11 +- src/mixed-line-bar-chart/utils.ts | 60 +++--- 4 files changed, 167 insertions(+), 213 deletions(-) diff --git a/src/mixed-line-bar-chart/__tests__/utils.test.ts b/src/mixed-line-bar-chart/__tests__/utils.test.ts index 67a066a0dc..1d60cd3cfb 100644 --- a/src/mixed-line-bar-chart/__tests__/utils.test.ts +++ b/src/mixed-line-bar-chart/__tests__/utils.test.ts @@ -1,6 +1,6 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import { matchesX, calculateStackedBarValues } from '../../../lib/components/mixed-line-bar-chart/utils'; +import { matchesX, calculateOffsetMaps } from '../../../lib/components/mixed-line-bar-chart/utils'; import { barSeries, barSeries2 } from './common'; @@ -37,84 +37,78 @@ describe('matchesX', () => { }); }); -describe('calculateStackedBarValues', () => { +describe('calculateOffsetMaps', () => { + afterAll(() => { + jest.restoreAllMocks(); + }); + test('without data', () => { - expect(calculateStackedBarValues([])).toEqual(new Map()); + expect(calculateOffsetMaps([])).toEqual([]); }); test('with categorical data', () => { const data = [barSeries.data, barSeries2.data, barSeries.data]; - const computed = calculateStackedBarValues(data); - expect(computed).toEqual( - new Map([ - [ - 'Potatoes', - new Map([ - [0, 77], - [1, 77 + 10], - [2, 77 + 10 + 77], - ]), - ], - [ - 'Chocolate', - new Map([ - [0, 546], - [1, 546 + 20], - [2, 546 + 20 + 546], - ]), - ], - [ - 'Apples', - new Map([ - [0, 52], - [2, 52 + 0 + 52], - ]), - ], - [ - 'Oranges', - new Map([ - [0, 47], - [1, 47 + 50], - [2, 47 + 50 + 47], - ]), - ], - ]) - ); + + const actual = calculateOffsetMaps(data); + + expect(actual).toEqual([ + { positiveOffsets: new Map(), negativeOffsets: new Map() }, + { + positiveOffsets: new Map([ + ['Potatoes', 77], + ['Chocolate', 546], + ['Apples', 52], + ['Oranges', 47], + ]), + negativeOffsets: new Map(), + }, + { + positiveOffsets: new Map([ + ['Potatoes', 87], + ['Chocolate', 566], + ['Apples', 52], + ['Oranges', 97], + ]), + negativeOffsets: new Map(), + }, + ]); }); test('with timeseries data', () => { const date = new Date('1995-12-17T03:24:00'); const data = [[{ x: date, y: 1 }], [{ x: date, y: 2 }], [{ x: date, y: 3 }]]; - const computed = calculateStackedBarValues(data); - expect(computed).toEqual( - new Map([ - [ - date.getTime(), - new Map([ - [0, 1], - [1, 1 + 2], - [2, 1 + 2 + 3], - ]), - ], - ]) - ); + + const actual = calculateOffsetMaps(data); + + expect(actual).toEqual([ + { positiveOffsets: new Map(), negativeOffsets: new Map() }, + { + positiveOffsets: new Map([[date.getTime(), 1]]), + negativeOffsets: new Map(), + }, + { + positiveOffsets: new Map([[date.getTime(), 3]]), + negativeOffsets: new Map(), + }, + ]); }); - test('with numeric data', () => { + test('with number data', () => { const data = [[{ x: 1, y: 1 }], [{ x: 1, y: 2 }], [{ x: 1, y: 3 }]]; - const computed = calculateStackedBarValues(data); - expect(computed).toEqual( - new Map([ - [ - 1, - new Map([ - [0, 1], - [1, 1 + 2], - [2, 1 + 2 + 3], - ]), - ], - ]) - ); + + const actual = calculateOffsetMaps(data); + + expect(actual).toEqual([ + { positiveOffsets: new Map(), negativeOffsets: new Map() }, + { + positiveOffsets: new Map([[1, 1]]), + negativeOffsets: new Map(), + }, + { + positiveOffsets: new Map([[1, 3]]), + negativeOffsets: new Map(), + }, + ]); }); test('with mixed positive and negative numbers', () => { @@ -135,34 +129,29 @@ describe('calculateStackedBarValues', () => { { x: 3, y: 10 }, ], ]; - const computed = calculateStackedBarValues(data); - expect(computed).toEqual( - new Map([ - [ - 1, - new Map([ - [0, -1], - [1, 2], - [2, 2 + 2], - ]), - ], - [ - 2, - new Map([ - [0, 3], - [1, -3], - [2, -3 + -1], - ]), - ], - [ - 3, - new Map([ - [0, -4], - [1, -4 + -4], - [2, 10], - ]), - ], - ]) - ); + + const actual = calculateOffsetMaps(data); + + expect(actual).toEqual([ + { positiveOffsets: new Map(), negativeOffsets: new Map() }, + { + positiveOffsets: new Map([[2, 3]]), + negativeOffsets: new Map([ + [1, -1], + [3, -4], + ]), + }, + { + positiveOffsets: new Map([ + [1, 2], + [2, 3], + ]), + negativeOffsets: new Map([ + [1, -1], + [2, -3], + [3, -8], + ]), + }, + ]); }); }); diff --git a/src/mixed-line-bar-chart/bar-series.tsx b/src/mixed-line-bar-chart/bar-series.tsx index b11c80d1e1..3f2dfa119c 100644 --- a/src/mixed-line-bar-chart/bar-series.tsx +++ b/src/mixed-line-bar-chart/bar-series.tsx @@ -1,15 +1,14 @@ // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 -import React from 'react'; +import React, { useMemo } from 'react'; import clsx from 'clsx'; import { ScaleContinuousNumeric, ScaleTime } from '../internal/vendor/d3-scale'; import { ChartScale, NumericChartScale } from '../internal/components/cartesian-chart/scales'; import { ChartDataTypes, MixedLineBarChartProps } from './interfaces'; -import { matchesX, getKeyValue, StackedBarValues } from './utils'; +import { matchesX, getKeyValue, StackedOffsets } from './utils'; import styles from './styles.css.js'; import { useVisualRefresh } from '../internal/hooks/use-visual-mode'; -import { useUniqueId } from '../internal/hooks/use-unique-id'; export interface BarSeriesProps { axis: 'x' | 'y'; @@ -29,8 +28,8 @@ export interface BarSeriesProps { dimmed: boolean; highlightedGroupIndex: number | null; - // Contains values to be used for stacked bars. - stackedBarValues?: StackedBarValues; + // Contains the cumulative offset for each x value in a stacked bar chart + stackedBarOffsets?: StackedOffsets; } export default function BarSeries({ @@ -42,16 +41,15 @@ export default function BarSeries({ highlighted, dimmed, highlightedGroupIndex, + stackedBarOffsets, totalSeriesCount, seriesIndex, plotSize, chartAreaClipPath, - stackedBarValues, }: BarSeriesProps) { const isRefresh = useVisualRefresh(); - const isStacked = !!stackedBarValues; - const xCoordinates = (() => { + const xCoordinates = useMemo(() => { if (series.type !== 'bar' || !xScale.isCategorical()) { return []; } @@ -69,7 +67,7 @@ export default function BarSeries({ const PADDING = 4; const MINWIDTH = 4; - if (!isStacked && totalSeriesCount > 1) { + if (!stackedBarOffsets && totalSeriesCount > 1) { // Regular grouped bars barWidth = (barWidth - (totalSeriesCount - 1) * PADDING) / totalSeriesCount; barWidth = Math.max(barWidth, MINWIDTH); @@ -77,22 +75,15 @@ export default function BarSeries({ return xPoints.map((x, i) => { const d = series.data[i]; - const key = getKeyValue(d.x); let barX = x; let yValue = d.y; - let isMin = false; - let isMax = false; - - // Stacked bars - if (isStacked) { - const allXValues = stackedBarValues.get(key) ?? new Map(); - yValue = allXValues.get(seriesIndex) ?? 0; - const allXValuesSorted = Array.from(allXValues.values()).sort((a, b) => a - b); - isMin = yValue === allXValuesSorted[0]; - isMax = yValue === allXValuesSorted[allXValuesSorted.length - 1]; - } - // Regular grouped bars - else if (totalSeriesCount > 1) { + + if (stackedBarOffsets) { + // Stacked bars + const offsetMap = d.y < 0 ? stackedBarOffsets.negativeOffsets : stackedBarOffsets.positiveOffsets; + yValue = d.y + (offsetMap.get(getKeyValue(d.x)) || 0); + } else if (totalSeriesCount > 1) { + // Regular grouped bars barX += seriesIndex * (barWidth + PADDING); } @@ -104,14 +95,11 @@ export default function BarSeries({ y: yContinuosScale(yValue) ?? NaN, width: barWidth, height: Math.abs((yContinuosScale(d.y) ?? NaN) - baseY), - isMin, - isMax, }; }); - })(); + }, [series, xScale, yScale, plotSize, stackedBarOffsets, totalSeriesCount, seriesIndex]); const highlightedXValue = highlightedGroupIndex !== null ? xScale.domain[highlightedGroupIndex] : null; - const clipPathId = useUniqueId(); return ( ({ [styles['series--dimmed']]: dimmed, })} > - {xCoordinates.map(({ x, y, width, height, isMin, isMax }, i) => { + {xCoordinates.map(({ x, y, width, height }, i) => { if (!isFinite(x) || !isFinite(height)) { return; } // Create margins between stacked series but only when series data is not too small. - const baseHeightOffset = isStacked ? 3 : 0; + const baseHeightOffset = stackedBarOffsets ? 3 : 0; const isSmallBar = height < 4; const heightOffset = isSmallBar ? 0 : baseHeightOffset; const widthOffset = 2; @@ -138,70 +126,30 @@ export default function BarSeries({ [styles['series--dimmed']]: highlightedXValue !== null && !matchesX(highlightedXValue, series.data[i].x), }); - const rectPlacement = - axis === 'x' - ? { - x: x + widthOffset / 2, - y: y + heightOffset / 2, - width: width - widthOffset, - height: height - heightOffset, - } - : { - x: y - height + heightOffset / 2, - y: x + widthOffset / 2, - width: height - heightOffset, - height: width - widthOffset, - }; - - // Non-stacked bars have all corners rounded. - // Stacked bars only require rounded corners for the min/max segments. - const rxProps = !isStacked || (isMin && isMax) ? { rx } : { clipPath: `url(#${clipPathId}-${i})` }; - - return ( - - {/* Render clip paths to provide rounded corners for the min/max stacked bars */} - {rxProps.clipPath && (isMin || isMax) && ( - - )} - - {/* Render the bar rectangle */} - - + return axis === 'x' ? ( + + ) : ( + ); })} ); } - -// Creates a rectangle by adding 10px offset to the provided one in the given direction. -// This makes one side ignored when using the rectangle in a clip path. -function clipRect( - rect: { x: number; y: number; height: number; width: number }, - direction: 'left' | 'right' | 'up' | 'down' -) { - switch (direction) { - case 'up': - return { ...rect, height: rect.height + 10 }; - case 'down': - return { ...rect, y: rect.y - 10, height: rect.height + 10 }; - case 'left': - return { ...rect, width: rect.width + 10 }; - case 'right': - return { ...rect, x: rect.x - 10, width: rect.width + 10 }; - } -} diff --git a/src/mixed-line-bar-chart/data-series.tsx b/src/mixed-line-bar-chart/data-series.tsx index c0c50312d4..82c79e2287 100644 --- a/src/mixed-line-bar-chart/data-series.tsx +++ b/src/mixed-line-bar-chart/data-series.tsx @@ -10,7 +10,7 @@ import BarSeries from './bar-series'; import { ChartDataTypes, InternalChartSeries, MixedLineBarChartProps } from './interfaces'; import styles from './styles.css.js'; -import { calculateStackedBarValues } from './utils'; +import { calculateOffsetMaps, StackedOffsets } from './utils'; // Should have the same value as the `border-line-chart-width` token. const STROKE_WIDTH = 2; @@ -49,10 +49,11 @@ export default function DataSeries({ // Lines get a small extra space at the top and bottom to account for the strokes when they are at the edge of the graph. const lineAreaClipPath = useUniqueId('awsui-line-chart__chart-area-'); - const stackedBarValues = useMemo(() => { + const stackedBarOffsetMaps: StackedOffsets[] = useMemo(() => { if (!stackedBars) { - return undefined; + return []; } + const barData: Array[]> = []; visibleSeries.forEach(({ series }) => { if (series.type === 'bar') { @@ -61,7 +62,7 @@ export default function DataSeries({ barData.push([]); } }); - return calculateStackedBarValues(barData); + return calculateOffsetMaps(barData); }, [visibleSeries, stackedBars]); return ( @@ -119,7 +120,7 @@ export default function DataSeries({ highlighted={isHighlighted} dimmed={isDimmed} chartAreaClipPath={chartAreaClipPath} - stackedBarValues={stackedBarValues} + stackedBarOffsets={stackedBarOffsetMaps[index]} highlightedGroupIndex={highlightedGroupIndex} /> ); diff --git a/src/mixed-line-bar-chart/utils.ts b/src/mixed-line-bar-chart/utils.ts index 0baa16f70c..1d8a0a370a 100644 --- a/src/mixed-line-bar-chart/utils.ts +++ b/src/mixed-line-bar-chart/utils.ts @@ -67,31 +67,47 @@ export const matchesX = (x1: T, x2: T) => { return x1 === x2; }; -export type StackedBarValues = Map>; - -// Unlike for regular bars, stacked bar series values depend on the predecessors. -// The function computes all stacked values grouped by X and series index. -export function calculateStackedBarValues( - dataBySeries: Array[]> -): StackedBarValues { - const negativeValues = new Map(); - const positiveValues = new Map(); - const values = new Map>(); - for (let seriesIndex = 0; seriesIndex < dataBySeries.length; seriesIndex++) { - for (const datum of dataBySeries[seriesIndex]) { - const key = getKeyValue(datum.x); - if (datum.y < 0) { - negativeValues.set(key, (negativeValues.get(key) ?? 0) + datum.y); +export type OffsetMap = Map; + +export interface StackedOffsets { + positiveOffsets: OffsetMap; + negativeOffsets: OffsetMap; +} + +/** + * Calculates list of offset maps from all data by accumulating each value + */ +export function calculateOffsetMaps( + data: Array[]> +): StackedOffsets[] { + return data.reduce((acc, curr, idx) => { + // First series receives empty offsets map + if (idx === 0) { + acc.push({ positiveOffsets: new Map(), negativeOffsets: new Map() }); + } + const lastMap = acc[idx]; + const map: StackedOffsets = lastMap + ? { positiveOffsets: new Map(lastMap.positiveOffsets), negativeOffsets: new Map(lastMap.negativeOffsets) } + : { positiveOffsets: new Map(), negativeOffsets: new Map() }; + + curr.forEach(({ x, y }) => { + const key = getKeyValue(x); + if (y < 0) { + const lastValue = lastMap?.negativeOffsets.get(key) || 0; + map.negativeOffsets.set(key, lastValue + y); } else { - positiveValues.set(key, (positiveValues.get(key) ?? 0) + datum.y); + const lastValue = lastMap?.positiveOffsets.get(key) || 0; + map.positiveOffsets.set(key, lastValue + y); } - const seriesValue = (datum.y < 0 ? negativeValues.get(key) : positiveValues.get(key)) ?? 0; - const valuesByIndex = values.get(key) ?? new Map(); - valuesByIndex.set(seriesIndex, seriesValue); - values.set(key, valuesByIndex); + }); + + // Ignore last value for map but still run it for logging + if (idx < data.length - 1) { + acc.push(map); } - } - return values; + + return acc; + }, [] as StackedOffsets[]); } /** Returns string or number value for ChartDataTypes key */