Skip to content

Commit

Permalink
[XY] Enables normal mode for percentage charts (elastic#120197) (elas…
Browse files Browse the repository at this point in the history
…tic#120467)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>
  • Loading branch information
kibanamachine and stratoula committed Dec 6, 2021
1 parent f9bb6bc commit 11aaff4
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 @@ -103,7 +103,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": "40.0.0",
"@elastic/charts": "40.1.0",
"@elastic/datemath": "link:bazel-bin/packages/elastic-datemath",
"@elastic/elasticsearch": "npm:@elastic/elasticsearch-canary@^8.0.0-canary.35",
"@elastic/ems-client": "8.0.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 @@ -134,8 +134,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@40.0.0":
version "40.0.0"
resolved "https://registry.yarnpkg.com/@elastic/charts/-/charts-40.0.0.tgz#aa79a34c160086bff3a4ade5f6c48bfdf4c7eab5"
integrity sha512-81gq7/loJO5znr3jUxKKUiXPvOFewsdFAGI/yIsbJJCMJ+1MR63A8V1h4ZzqVYoVne4S4btl/beIx0s/JFN+aw==
"@elastic/charts@40.1.0":
version "40.1.0"
resolved "https://registry.yarnpkg.com/@elastic/charts/-/charts-40.1.0.tgz#2cb19709ad6fb70ac5f96556bbccee2205ec0600"
integrity sha512-t3BCeSdILVKM+iXboTXD4y8mqrNnLUzlX/t6+NDjx2eBcAcfJ4Rr4FtAJBr16YtHlQZNxoFEMHAiRnBpwhxv8A==
dependencies:
"@popperjs/core" "^2.4.0"
chroma-js "^2.1.0"
Expand Down

0 comments on commit 11aaff4

Please sign in to comment.