Skip to content

Commit

Permalink
fix: custom domain error with fallback ordinal scale (#757)
Browse files Browse the repository at this point in the history
Ignores `xDomain` prop when using a fallback ordinal scale.

fixes #756
  • Loading branch information
nickofthyme authored Jul 21, 2020
1 parent 750ee54 commit 142c3df
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 3 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
14 changes: 12 additions & 2 deletions src/chart_types/xy_chart/domains/x_domain.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -470,24 +470,34 @@ describe('X Domain', () => {
],
};
const specDataSeries = [ds1, ds2];
const customDomain = {
min: 0,
};

const { xValues } = getSplittedSeries(specDataSeries);
const mergedDomain = mergeXDomain(
const getResult = () => mergeXDomain(
[
{
seriesType: SeriesTypes.Bar,
xScaleType: ScaleType.Linear,
},
{
seriesType: SeriesTypes.Bar,
xScaleType: ScaleType.Ordinal,
xScaleType: ScaleType.Linear,
},
],
xValues,
customDomain,
ScaleType.Ordinal,
);

expect(getResult).not.toThrow();

const mergedDomain = getResult();
expect(mergedDomain.domain).toEqual([0, 'a', 2, 5, 7]);
expect(mergedDomain.scaleType).toEqual(ScaleType.Ordinal);
});

test('Should merge multi bar/line ordinal series correctly', () => {
const ds1: BasicSeriesSpec = {
chartType: ChartTypes.XYAxis,
Expand Down
9 changes: 8 additions & 1 deletion src/chart_types/xy_chart/domains/x_domain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,14 @@ export function mergeXDomain(
if (Array.isArray(customXDomain)) {
seriesXComputedDomains = customXDomain;
} else {
throw new TypeError('xDomain for ordinal scale should be an array of values, not a DomainRange object');
if (fallbackScale === ScaleType.Ordinal) {
Logger.warn(`xDomain ignored for fallback ordinal scale. Options to resolve:
1) Correct data to match ${mainXScaleType.scaleType} scale type (see previous warning)
2) Change xScaleType to ordinal and set xDomain to Domain array`);
} else {
throw new TypeError('xDomain for ordinal scale should be an array of values, not a DomainRange object.');
}
}
}
} else {
Expand Down
55 changes: 55 additions & 0 deletions stories/scales/6_x_scale_fallback.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { boolean } from '@storybook/addon-knobs';
import React from 'react';

import { Axis, Chart, BarSeries, Position, ScaleType, Settings } from '../../src';

export const Example = () => {
const includeString = boolean('include string is x data', true);
return (
<Chart className="story-chart">
<Settings xDomain={{ min: 0, max: 10 }} />
<Axis id="y" title="count" position={Position.Left} />
<Axis id="x" title={includeString ? 'ordinal fallback scale' : 'linear scale'} position={Position.Bottom} />
<BarSeries
id="bars"
name="amount"
xScaleType={ScaleType.Linear}
xAccessor="x"
yAccessors={['y']}
data={[
{ x: 1, y: 390, val: 1222 },
{ x: 2, y: 23, val: 1222 },
{ x: includeString ? '3' : 3, y: 750, val: 1222 },
{ x: 4, y: 854, val: 1222 },
]}
/>
</Chart>
);
};

Example.story = {
parameters: {
info: {
text: 'Using string values with a `Linear` scale will attempt to fallback to an `Ordinal` scale. Notice how the custom `xDomain` is ignored when the scale falls back to `Ordinal`.',
},
},
};
1 change: 1 addition & 0 deletions stories/scales/scales.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,4 @@ export { Example as tooltipInLocalTimezone } from './2_local_tooltip';
export { Example as tooltipInUTC } from './3_utc_tooltip';
export { Example as specifiedTimezone } from './4_specified_timezone';
export { Example as removeDuplicateAxis } from './5_remove_duplicates';
export { Example as xScaleFallback } from './6_x_scale_fallback';

0 comments on commit 142c3df

Please sign in to comment.