fix(D3 plugin): fix some errors in bar-y series (#492)

* fix(D3 plugin): fix some errors in bar-y series * Add a clarification to the type description
gravity-ui · Jun 14, 2024 · 39138b3 · 39138b3
1 parent 3904443
commit 39138b3
Show file tree

Hide file tree

Showing 9 changed files with 94 additions and 43 deletions.
diff --git a/src/plugins/d3/__stories__/bar-y/Playground.stories.tsx b/src/plugins/d3/__stories__/bar-y/Playground.stories.tsx
@@ -36,6 +36,7 @@ function prepareData(): ChartKitWidgetData {
             labels: {
                 enabled: true,
             },
+            lineColor: 'transparent',
         },
         yAxis: [
             {

diff --git a/src/plugins/d3/renderer/hooks/useAxisScales/index.ts b/src/plugins/d3/renderer/hooks/useAxisScales/index.ts
@@ -7,7 +7,7 @@ import get from 'lodash/get';
 import {ChartKitWidgetAxis, ChartKitWidgetSeries} from '../../../../../types';
 import {DEFAULT_AXIS_TYPE} from '../../constants';
 import {
-    CHART_SERIES_WITH_VOLUME,
+    CHART_SERIES_WITH_VOLUME_ON_Y_AXIS,
     getAxisHeight,
     getDataCategoryValue,
     getDefaultMaxXAxisValue,
@@ -79,7 +79,7 @@ export function createYScale(axis: PreparedAxis, series: PreparedSeries[], bound
                 const [domainYMin, domainMax] = extent(domain) as [number, number];
                 const yMinValue = typeof yMin === 'number' ? yMin : domainYMin;
                 let yMaxValue = domainMax;
-                if (series.some((s) => CHART_SERIES_WITH_VOLUME.includes(s.type))) {
+                if (series.some((s) => CHART_SERIES_WITH_VOLUME_ON_Y_AXIS.includes(s.type))) {
                     yMaxValue = Math.max(yMaxValue, 0);
                 }
 
@@ -129,8 +129,12 @@ function calculateXAxisPadding(series: (PreparedSeries | ChartKitWidgetSeries)[]
         switch (s.type) {
             case 'bar-y': {
                 // Since labels can be located to the right of the bar, need to add an additional space
-                const labelsMaxWidth = get(s, 'dataLabels.maxWidth', 0);
-                result = Math.max(result, labelsMaxWidth);
+                const inside = get(s, 'dataLabels.inside');
+                if (!inside) {
+                    const labelsMaxWidth = get(s, 'dataLabels.maxWidth', 0);
+                    result = Math.max(result, labelsMaxWidth);
+                }
+
                 break;
             }
         }

diff --git a/src/plugins/d3/renderer/hooks/useChartOptions/x-axis.ts b/src/plugins/d3/renderer/hooks/useChartOptions/x-axis.ts
@@ -8,6 +8,7 @@ import {
     xAxisTitleDefaults,
 } from '../../constants';
 import {
+    CHART_SERIES_WITH_VOLUME_ON_X_AXIS,
     calculateCos,
     formatAxisTickLabel,
     getClosestPointsRange,
@@ -69,6 +70,22 @@ function getLabelSettings({
     return {height: Math.min(maxHeight, labelsHeight), rotation};
 }
 
+function getAxisMin(axis?: ChartKitWidgetXAxis, series?: ChartKitWidgetSeries[]) {
+    const min = axis?.min;
+
+    if (
+        typeof min === 'undefined' &&
+        series?.some((s) => CHART_SERIES_WITH_VOLUME_ON_X_AXIS.includes(s.type))
+    ) {
+        return series.reduce((minValue, s) => {
+            const minYValue = s.data.reduce((res, d) => Math.min(res, get(d, 'x', 0)), 0);
+            return Math.min(minValue, minYValue);
+        }, 0);
+    }
+
+    return min;
+}
+
 export const getPreparedXAxis = ({
     xAxis,
     series,
@@ -112,7 +129,7 @@ export const getPreparedXAxis = ({
                 ? getHorisontalSvgTextHeight({text: titleText, style: titleStyle})
                 : 0,
         },
-        min: get(xAxis, 'min'),
+        min: getAxisMin(xAxis, series),
         maxPadding: get(xAxis, 'maxPadding', 0.01),
         grid: {
             enabled: get(xAxis, 'grid.enabled', true),

diff --git a/src/plugins/d3/renderer/hooks/useChartOptions/y-axis.ts b/src/plugins/d3/renderer/hooks/useChartOptions/y-axis.ts
@@ -8,7 +8,7 @@ import {
     yAxisTitleDefaults,
 } from '../../constants';
 import {
-    CHART_SERIES_WITH_VOLUME,
+    CHART_SERIES_WITH_VOLUME_ON_Y_AXIS,
     formatAxisTickLabel,
     getClosestPointsRange,
     getHorisontalSvgTextHeight,
@@ -54,7 +54,7 @@ function getAxisMin(axis?: ChartKitWidgetYAxis, series?: ChartKitWidgetSeries[])
 
     if (
         typeof min === 'undefined' &&
-        series?.some((s) => CHART_SERIES_WITH_VOLUME.includes(s.type))
+        series?.some((s) => CHART_SERIES_WITH_VOLUME_ON_Y_AXIS.includes(s.type))
     ) {
         return series.reduce((minValue, s) => {
             switch (s.type) {

diff --git a/src/plugins/d3/renderer/hooks/useSeries/prepare-bar-y.ts b/src/plugins/d3/renderer/hooks/useSeries/prepare-bar-y.ts
@@ -24,10 +24,11 @@ function prepareDataLabels(series: BarYSeries) {
               style,
           })
         : {};
+    const inside = series.stacking === 'percent' ? true : get(series, 'dataLabels.inside', false);
 
     return {
         enabled,
-        inside: get(series, 'dataLabels.inside', false),
+        inside,
         style,
         maxHeight,
         maxWidth,

diff --git a/src/plugins/d3/renderer/hooks/useShapes/bar-y/prepare-data.ts b/src/plugins/d3/renderer/hooks/useShapes/bar-y/prepare-data.ts
@@ -83,6 +83,7 @@ export const prepareBarYData = (args: {
         xScale,
         yScale: [yScale],
     } = args;
+
     const xLinearScale = xScale as ScaleLinear<number, number>;
     const plotWidth = xLinearScale(xLinearScale.domain()[1]);
     const barMaxWidth = get(seriesOptions, 'bar-y.barMaxWidth');

diff --git a/src/plugins/d3/renderer/utils/axis-generators/bottom.ts b/src/plugins/d3/renderer/utils/axis-generators/bottom.ts
@@ -35,14 +35,17 @@ function addDomain(
 ) {
     const {size, color} = options;
 
-    selection
+    const domainPath = selection
         .selectAll('.domain')
         .data([null])
         .enter()
         .insert('path', '.tick')
         .attr('class', 'domain')
-        .attr('stroke', color || 'currentColor')
         .attr('d', `M0,0V0H${size}`);
+
+    if (color) {
+        domainPath.style('stroke', color);
+    }
 }
 
 export function axisBottom(args: AxisBottomArgs) {

diff --git a/src/plugins/d3/renderer/utils/index.ts b/src/plugins/d3/renderer/utils/index.ts
@@ -25,12 +25,14 @@ export * from './symbol';
 export * from './series';
 
 const CHARTS_WITHOUT_AXIS: ChartKitWidgetSeries['type'][] = ['pie', 'treemap'];
-export const CHART_SERIES_WITH_VOLUME: ChartKitWidgetSeries['type'][] = [
+export const CHART_SERIES_WITH_VOLUME_ON_Y_AXIS: ChartKitWidgetSeries['type'][] = [
     'bar-x',
     'area',
     'waterfall',
 ];
 
+export const CHART_SERIES_WITH_VOLUME_ON_X_AXIS: ChartKitWidgetSeries['type'][] = ['bar-y'];
+
 export type AxisDirection = 'x' | 'y';
 
 type UnknownSeries = {type: ChartKitWidgetSeries['type']; data: unknown};
@@ -66,10 +68,58 @@ export function isSeriesWithCategoryValues(series: UnknownSeries): series is {
     return isAxisRelatedSeries(series);
 }
 
+function getDomainDataForStackedSeries(
+    seriesList: StackedSeries[],
+    keyAttr: 'x' | 'y' = 'x',
+    valueAttr: 'x' | 'y' = 'y',
+) {
+    const acc: number[] = [];
+    const stackedSeries = group(seriesList, getSeriesStackId);
+    Array.from(stackedSeries).forEach(([_stackId, seriesStack]) => {
+        const values: Record<string, number> = {};
+
+        seriesStack.forEach((singleSeries) => {
+            const data = new Map();
+            singleSeries.data.forEach((point) => {
+                const key = String(point[keyAttr]);
+                let value = 0;
+
+                if (valueAttr in point && typeof point[valueAttr] === 'number') {
+                    value = point[valueAttr] as number;
+                }
+
+                if (data.has(key)) {
+                    value = Math.max(value, data.get(key));
+                }
+
+                data.set(key, value);
+            });
+
+            Array.from(data).forEach(([key, value]) => {
+                values[key] = (values[key] || 0) + value;
+            });
+        });
+
+        acc.push(...Object.values(values));
+    });
+
+    return acc;
+}
+
 export const getDomainDataXBySeries = (series: UnknownSeries[]) => {
-    return series.reduce<number[]>((acc, s) => {
-        if (isSeriesWithNumericalXValues(s)) {
-            acc.push(...s.data.map((d) => d.x));
+    const groupedSeries = group(series, (item) => item.type);
+
+    return Array.from(groupedSeries).reduce<unknown[]>((acc, [type, seriesList]) => {
+        switch (type) {
+            case 'bar-y': {
+                acc.push(...getDomainDataForStackedSeries(seriesList as StackedSeries[], 'y', 'x'));
+                break;
+            }
+            default: {
+                seriesList.filter(isSeriesWithNumericalXValues).forEach((s) => {
+                    acc.push(...s.data.map((d) => d.x));
+                });
+            }
         }
 
         return acc;
@@ -91,34 +141,7 @@ export const getDomainDataYBySeries = (series: UnknownSeries[]) => {
         switch (type) {
             case 'area':
             case 'bar-x': {
-                const stackedSeries = group(seriesList as StackedSeries[], getSeriesStackId);
-                Array.from(stackedSeries).forEach(([_stackId, seriesStack]) => {
-                    const values: Record<string, number> = {};
-
-                    seriesStack.forEach((singleSeries) => {
-                        const data = new Map();
-                        singleSeries.data.forEach((point) => {
-                            const key = String(point.x);
-                            let value = 0;
-
-                            if (point.y && typeof point.y === 'number') {
-                                value = point.y;
-                            }
-
-                            if (data.has(key)) {
-                                value = Math.max(value, data.get(key));
-                            }
-
-                            data.set(key, value);
-                        });
-
-                        Array.from(data).forEach(([key, value]) => {
-                            values[key] = (values[key] || 0) + value;
-                        });
-                    });
-
-                    acc.push(...Object.values(values));
-                });
+                acc.push(...getDomainDataForStackedSeries(seriesList as StackedSeries[]));
 
                 break;
             }

diff --git a/src/types/widget-data/bar-y.ts b/src/types/widget-data/bar-y.ts
@@ -48,7 +48,8 @@ export type BarYSeries<T = any> = BaseSeries & {
     grouping?: boolean;
     dataLabels?: ChartKitWidgetSeriesOptions['dataLabels'] & {
         /**
-         * Whether to align the data label inside or outside the box
+         * Whether to align the data label inside or outside the box.
+         * For charts with a percentage stack, it is always true.
          *
          * @default false
          * */