Skip to content

Commit

Permalink
[XY] Enables normal mode for percentage charts (#119582)
Browse files Browse the repository at this point in the history
* [XY] Enable normal mode for percentage charts

* Update the library version

* Fix

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
stratoula and kibanamachine authored Dec 8, 2021
1 parent 11ffcfa commit a75f472
Show file tree
Hide file tree
Showing 9 changed files with 69 additions and 43 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@
"@elastic/apm-rum": "^5.9.1",
"@elastic/apm-rum-react": "^1.3.1",
"@elastic/apm-synthtrace": "link:bazel-bin/packages/elastic-apm-synthtrace",
"@elastic/charts": "38.0.3",
"@elastic/charts": "38.0.5",
"@elastic/datemath": "link:bazel-bin/packages/elastic-datemath",
"@elastic/elasticsearch": "npm:@elastic/elasticsearch-canary@^7.16.0-canary.7",
"@elastic/ems-client": "7.16.0",
Expand Down
4 changes: 1 addition & 3 deletions src/plugins/vis_types/xy/public/config/get_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,6 @@ const shouldEnableHistogramMode = (
}

return bars.every(({ valueAxis: groupId, mode }) => {
const yAxisScale = yAxes.find(({ groupId: axisGroupId }) => axisGroupId === groupId)?.scale;

return mode === 'stacked' || yAxisScale?.mode === 'percentage';
return mode === 'stacked';
});
};

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
*/

import React from 'react';
import { shallow, mount } from 'enzyme';
import { shallow } from 'enzyme';

import { ChartOptions, ChartOptionsParams } from './chart_options';
import { SeriesParam, ChartMode, AxisMode } from '../../../../types';
import { SeriesParam, ChartMode } from '../../../../types';
import { LineOptions } from './line_options';
import { PointOptions } from './point_options';
import { valueAxis, seriesParam } from './mocks';
Expand Down Expand Up @@ -85,14 +85,4 @@ describe('ChartOptions component', () => {

expect(setParamByIndex).toBeCalledWith('seriesParams', 0, paramName, ChartMode.Normal);
});

it('should set "stacked" mode and disabled control if the referenced axis is "percentage"', () => {
defaultProps.valueAxes[0].scale.mode = AxisMode.Percentage;
defaultProps.chart.mode = ChartMode.Normal;
const paramName = 'mode';
const comp = mount(<ChartOptions {...defaultProps} />);

expect(setParamByIndex).toBeCalledWith('seriesParams', 0, paramName, ChartMode.Stacked);
expect(comp.find({ paramName }).prop('disabled')).toBeTruthy();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@
* Side Public License, v 1.
*/

import React, { useMemo, useCallback, useEffect, useState } from 'react';
import React, { useMemo, useCallback } from 'react';

import { i18n } from '@kbn/i18n';
import { EuiFlexGroup, EuiFlexItem, EuiSpacer } from '@elastic/eui';

import { SelectOption } from '../../../../../../../vis_default_editor/public';

import { SeriesParam, ValueAxis, ChartMode, AxisMode } from '../../../../types';
import { SeriesParam, ValueAxis } from '../../../../types';
import { LineOptions } from './line_options';
import { PointOptions } from './point_options';
import { SetParamByIndex, ChangeValueAxis } from '.';
Expand All @@ -39,7 +39,6 @@ function ChartOptions({
changeValueAxis,
setParamByIndex,
}: ChartOptionsParams) {
const [disabledMode, setDisabledMode] = useState<boolean>(false);
const setChart: SetChart = useCallback(
(paramName, value) => {
setParamByIndex('seriesParams', index, paramName, value);
Expand Down Expand Up @@ -70,20 +69,6 @@ function ChartOptions({
[valueAxes]
);

useEffect(() => {
const valueAxisToMetric = valueAxes.find((valueAxis) => valueAxis.id === chart.valueAxis);
if (valueAxisToMetric) {
if (valueAxisToMetric.scale.mode === AxisMode.Percentage) {
setDisabledMode(true);
if (chart.mode !== ChartMode.Stacked) {
setChart('mode', ChartMode.Stacked);
}
} else if (disabledMode) {
setDisabledMode(false);
}
}
}, [valueAxes, chart, disabledMode, setChart, setDisabledMode]);

return (
<>
<SelectOption
Expand Down Expand Up @@ -120,7 +105,6 @@ function ChartOptions({
})}
options={collections.chartModes}
paramName="mode"
disabled={disabledMode}
value={chart.mode}
setValue={setChart}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export const getVisConfig = (): VisConfig => {
show: false,
},
scale: {
mode: AxisMode.Normal,
mode: AxisMode.Percentage,
type: 'linear',
},
domain: {
Expand Down
37 changes: 37 additions & 0 deletions src/plugins/vis_types/xy/public/utils/render_all_series.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,43 @@ describe('renderAllSeries', function () {
expect(wrapper.find(BarSeries).length).toBe(1);
});

it('renders percentage data for percentage mode', () => {
const barSeriesParams = [{ ...defaultSeriesParams[0], type: 'histogram', mode: 'percentage' }];
const config = getVisConfig();

const renderBarSeries = renderAllSeries(
config,
barSeriesParams as SeriesParam[],
defaultData,
jest.fn(),
jest.fn(),
'Europe/Athens',
'col-0-2',
[]
);
const wrapper = shallow(<div>{renderBarSeries}</div>);
expect(wrapper.find(BarSeries).length).toBe(1);
expect(wrapper.find(BarSeries).prop('stackMode')).toEqual('percentage');
expect(wrapper.find(BarSeries).prop('data')).toEqual([
{
'col-0-2': 1610960220000,
'col-1-3': 1,
},
{
'col-0-2': 1610961300000,
'col-1-3': 1,
},
{
'col-0-2': 1610961900000,
'col-1-3': 1,
},
{
'col-0-2': 1610962980000,
'col-1-3': 1,
},
]);
});

it('renders the correct yAccessors for not percentile aggs', () => {
const renderSeries = getAllSeries(getVisConfig(), defaultSeriesParams, defaultData);
const wrapper = shallow(<div>{renderSeries}</div>);
Expand Down
24 changes: 21 additions & 3 deletions src/plugins/vis_types/xy/public/utils/render_all_series.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
AccessorFn,
ColorVariant,
LabelOverflowConstraint,
computeRatioByGroups,
} from '@elastic/charts';

import { DatatableRow } from '../../../../expressions/public';
Expand Down Expand Up @@ -90,7 +91,24 @@ export const renderAllSeries = (

const id = `${type}-${yAccessors[0]}`;
const yAxisScale = yAxes.find(({ groupId: axisGroupId }) => axisGroupId === groupId)?.scale;
const isStacked = mode === 'stacked' || yAxisScale?.mode === 'percentage';
// compute percentage mode data
const splitChartAccessor = aspects.splitColumn?.accessor || aspects.splitRow?.accessor;
const groupAccessors = [String(xAccessor)];
if (splitChartAccessor) {
groupAccessors.push(splitChartAccessor);
}
let computedData = data;
if (yAxisScale?.mode === 'percentage') {
yAccessors.forEach((accessor) => {
computedData = computeRatioByGroups(
computedData,
groupAccessors,
(d) => d[accessor],
accessor
);
});
}
const isStacked = mode === 'stacked';
const stackMode = yAxisScale?.mode === 'normal' ? undefined : yAxisScale?.mode;
// needed to seperate stacked and non-stacked bars into unique pseudo groups
const pseudoGroupId = isStacked ? `__pseudo_stacked_group-${groupId}__` : groupId;
Expand All @@ -113,7 +131,7 @@ export const renderAllSeries = (
xAccessor={xAccessor}
yAccessors={yAccessors}
splitSeriesAccessors={splitSeriesAccessors}
data={data}
data={computedData}
timeZone={timeZone}
stackAccessors={isStacked ? ['__any_value__'] : undefined}
enableHistogramMode={enableHistogramMode}
Expand Down Expand Up @@ -153,7 +171,7 @@ export const renderAllSeries = (
markSizeAccessor={markSizeAccessor}
markFormat={aspects.z?.formatter}
splitSeriesAccessors={splitSeriesAccessors}
data={data}
data={computedData}
stackAccessors={isStacked ? ['__any_value__'] : undefined}
displayValueSettings={{
showValueLabel,
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1520,10 +1520,10 @@
dependencies:
object-hash "^1.3.0"

"@elastic/charts@38.0.3":
version "38.0.3"
resolved "https://registry.yarnpkg.com/@elastic/charts/-/charts-38.0.3.tgz#0c52dd59abc9b2591348d162f5a8ff4544d1d301"
integrity sha512-BRCVCtOqIJ8L6sOEeeqcQIj4MIzyst1M7cwUMd3QjThd3ZC2iPfTBjxb7lJGqITU79p4zbVkU5w4jtGTQ3sKgw==
"@elastic/charts@38.0.5":
version "38.0.5"
resolved "https://registry.yarnpkg.com/@elastic/charts/-/charts-38.0.5.tgz#b6ca7a41903a5b31589294d141f84134daa590d5"
integrity sha512-Utr1n+j71qQbscCucO7frnMC1j1TgftzqSLoUE0CezFRcwgcy1i3KhSCtdnSthXtvLUvh341WpkKOqp7UyOuhw==
dependencies:
"@popperjs/core" "^2.4.0"
chroma-js "^2.1.0"
Expand Down

0 comments on commit a75f472

Please sign in to comment.