Skip to content

Commit

Permalink
feat: clean up types (#554)
Browse files Browse the repository at this point in the history
This commit fix some wrongly configured annotation TS types and expose the Specs types as requested by #547. It also expose some more types that can be used by consumers that where buried inside our chart library. Finally it removes the `getSpecId`, `getAnnotationId`, `getGroupId`, `getAxisId` getters in favour of just using a string.

BREAKING CHANGE: The `getSpecId`, `getGroupId`, `getAxisId` and `getAnnotationId` are no longer available. Use a simple `string` instead.

close #547
  • Loading branch information
markov00 authored Feb 28, 2020
1 parent f1ac77c commit 124c458
Show file tree
Hide file tree
Showing 29 changed files with 234 additions and 195 deletions.
18 changes: 16 additions & 2 deletions .playground/playground.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
import React from 'react';

import { Chart, LineSeries, ScaleType, Position, Axis } from '../src';
import {
Chart,
ScaleType,
Position,
Axis,
LineSeries,
LineAnnotation,
RectAnnotation,
AnnotationDomainTypes,
LineAnnotationDatum,
RectAnnotationDatum,
} from '../src';
import { SeededDataGenerator } from '../src/mocks/utils';

export class Playground extends React.Component<{}, { isSunburstShown: boolean }> {
Expand All @@ -10,6 +20,8 @@ export class Playground extends React.Component<{}, { isSunburstShown: boolean }
...item,
y1: item.y + 100,
}));
const lineDatum: LineAnnotationDatum[] = [{ dataValue: 321321 }];
const rectDatum: RectAnnotationDatum[] = [{ coordinates: { x1: 100 } }];

return (
<>
Expand All @@ -35,6 +47,8 @@ export class Playground extends React.Component<{}, { isSunburstShown: boolean }
splitSeriesAccessors={['g']}
data={data}
/>
<LineAnnotation id="sss" dataValues={lineDatum} domainType={AnnotationDomainTypes.XDomain} />
<RectAnnotation id="111" dataValues={rectDatum} />
</Chart>
</div>
</>
Expand Down
71 changes: 35 additions & 36 deletions docs/1-Typesofchart/5-AnnotatingBars.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,18 @@ import {
Axis,
BarSeries,
Chart,
getAnnotationId,
LineAnnotation,
LineAnnotationDatum,
LineSeries,
RectAnnotation,
ScaleType,
Settings,
timeFormatter,
Position,
} from '../../src';
import { Icon } from '../../src/components/icons/icon';
import { KIBANA_METRICS } from '../../src/utils/data_samples/test_dataset_kibana';
import { BandedAccessorType } from '../../src/utils/geometry';
import { Position } from '../../src/chart_types/xy_chart/utils/specs';
import { getChartRotationKnob, generateAnnotationData, arrayKnobs } from '../../docs/charts'

<Meta title="Types of charts/Bar Charts/Annotating Bar Charts" />
Expand All @@ -31,22 +30,22 @@ Here is a basic `line` annotation with `x domain continuous`
<Chart className={'story-chart'}>
<Settings showLegend debug={boolean('debug', false)} rotation={getChartRotationKnob} />
<LineAnnotation
id={getAnnotationId('anno_1')}
domainType={AnnotationDomainTypes.XDomain}
dataValues={[{dataValue:2.5, details:'detail-0'}, {dataValue: 7.2, details: 'detail-1'}]}
marker={<Icon type="alert" />}
style = {{line: {
strokeWidth: 3,
stroke: '#f00',
opacity: 1,
},
details: {
fontSize: 12,
fontFamily: 'Arial',
fontStyle: 'bold',
fill: 'gray',
padding: 0,
},}}
id="anno_1"
domainType={AnnotationDomainTypes.XDomain}
dataValues={[{dataValue:2.5, details:'detail-0'}, {dataValue: 7.2, details: 'detail-1'}]}
marker={<Icon type="alert" />}
style = {{line: {
strokeWidth: 3,
stroke: '#f00',
opacity: 1,
},
details: {
fontSize: 12,
fontFamily: 'Arial',
fontStyle: 'bold',
fill: 'gray',
padding: 0,
},}}
/>
<Axis id={'horizontal'} position={Position.Bottom} title={'x-domain axis'} />
<Axis id={'vertical'} title={'y-domain axis'} position={Position.Left} />
Expand Down Expand Up @@ -74,22 +73,22 @@ The code sample for the above chart is shown below in the details. What you shou
<Chart className={'story-chart'}>
<Settings showLegend debug={boolean('debug', false)} rotation={getChartRotationKnob} />
<LineAnnotation
id={getAnnotationId('anno_1')}
domainType={AnnotationDomainTypes.XDomain}
dataValues={[{dataValue:2.5, details:'detail-0'}, {dataValue: 7.2, details: 'detail-1'}]}
marker={<Icon type="alert" />}
style = {{line: {
strokeWidth: 3,
stroke: '#f00',
opacity: 1,
},
details: {
fontSize: 12,
fontFamily: 'Arial',
fontStyle: 'bold',
fill: 'gray',
padding: 0,
},}}
id="anno_1"
domainType={AnnotationDomainTypes.XDomain}
dataValues={[{dataValue:2.5, details:'detail-0'}, {dataValue: 7.2, details: 'detail-1'}]}
marker={<Icon type="alert" />}
style = {{line: {
strokeWidth: 3,
stroke: '#f00',
opacity: 1,
},
details: {
fontSize: 12,
fontFamily: 'Arial',
fontStyle: 'bold',
fill: 'gray',
padding: 0,
},}}
/>
<Axis id={'horizontal'} position={Position.Bottom} title={'x-domain axis'} />
<Axis id={'vertical'} title={'y-domain axis'} position={Position.Left} />
Expand All @@ -114,7 +113,7 @@ Instead of `continuous` you can use `ordinal` annotations
<Chart className={'story-chart'}>
<Settings debug={boolean('debug', false)} rotation={getChartRotationKnob} />
<LineAnnotation
id={'anno_1'}
id="anno_1"
domainType={AnnotationDomainTypes.XDomain}
dataValues={[{dataValue: 'a', details: 'detail-0'}, {dataValue: 'c', details:'details-1'}]}
marker={<Icon type="alert" />}
Expand Down Expand Up @@ -144,7 +143,7 @@ The code sample for the above chart can be seen below.
<Chart className={'story-chart'}>
<Settings debug={boolean('debug', false)} rotation={getChartRotationKnob} />
<LineAnnotation
id={'anno_1'}
id="anno_1"
domainType={AnnotationDomainTypes.XDomain}
dataValues={[{dataValue: 'a', details: 'detail-0'}, {dataValue: 'c', details:'details-1'}]}
marker={<Icon type="alert" />}
Expand Down
2 changes: 1 addition & 1 deletion docs/2-ChartPropTables/12-AxisProps.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ The bar chart with axis example in the `Types of charts` section includes only s
|:------|:------:|:---------:|:------|
| chartType | `typeof ChartTypes.XYAxis` | ChartTypes.XYAxis | |
| specType | `typeof SpecTypes.Axis` | SpecTypes.Axis | |
| groupId | GroupId | `__global__` | The ID of the axis group, generated via getGroupId method |
| groupId | GroupId | `__global__` | The ID of the axis group |
| hide | boolean | false | Hide this axis |
| showOverlappingTicks | boolean | false | Shows all ticks, also the one from the overlapping labels |
| showOverlappingLabels | boolean | false | Shows all labels, also the overlapping ones |
Expand Down
2 changes: 1 addition & 1 deletion docs/2-ChartPropTables/14-LineProps.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
| chartType | `typeof ChartTypes.XYAxis` | ChartTypes.XYAxis | |
| specType | `typeof SpecTypes.Series` | SpecTypes.Series | |
| seriesTypes | | SeriesTypes.Line | |
| groupId || DEFAULT_GROUP_ID |The ID of the line, generated via getGroupId method|
| groupId || DEFAULT_GROUP_ID |The ID of the line |
| xScaleType `(required)`| `ScaleType (ScaleType.Ordinal or ScaleType.Linear or ScaleType.Time)`|ScaleType.Ordinal | The x axis scale type |
| yScaleType `(required)`| `ScaleType (ScaleType.Ordinal or ScaleType.Linear or ScaleType.Time)`| ScaleType.Linear | The y axis scale type |
| xAccessor `(required)` | Accessor | 'x' | the field name of the x value on the Datum object|
Expand Down
3 changes: 1 addition & 2 deletions docs/charts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
Position,
Settings,
LineAnnotation,
getAnnotationId,
AnnotationDomainTypes,
Axis,
LineAnnotationDatum,
Expand Down Expand Up @@ -107,7 +106,7 @@ export const lineBasicXDomainContinous = () => {
<Chart className="story-chart">
<Settings showLegend debug={boolean('debug', false)} rotation={getChartRotationKnob()} />
<LineAnnotation
id={getAnnotationId('anno_1')}
id="anno_1"
domainType={AnnotationDomainTypes.XDomain}
dataValues={dataValues}
style={style}
Expand Down
2 changes: 1 addition & 1 deletion integration/tests/mixed_stories.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { common } from '../page_objects';
import { Fit } from '../../src/chart_types/xy_chart/utils/specs';
import { Fit } from '../../src';

describe('Mixed series stories', () => {
describe('Fitting functions', () => {
Expand Down
2 changes: 2 additions & 0 deletions src/chart_types/specs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,6 @@ export {
HistogramBarSeries,
} from './xy_chart/specs';

export * from './xy_chart/utils/specs';

export { Partition } from './partition_chart/specs';
10 changes: 5 additions & 5 deletions src/chart_types/xy_chart/annotations/annotation_utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
import { Position, Rotation } from '../../../utils/commons';
import { DEFAULT_ANNOTATION_LINE_STYLE } from '../../../utils/themes/theme';
import { Dimensions } from '../../../utils/dimensions';
import { getAxisId, getGroupId, GroupId, AnnotationId } from '../../../utils/ids';
import { GroupId, AnnotationId } from '../../../utils/ids';
import { Scale, ScaleType, ScaleBand, ScaleContinuous } from '../../../scales';
import {
computeAnnotationDimensions,
Expand Down Expand Up @@ -64,13 +64,13 @@ describe('annotation utils', () => {
left: 15,
};

const groupId = getGroupId('foo-group');
const groupId = 'foo-group';

const axesSpecs: AxisSpec[] = [];
const verticalAxisSpec: AxisSpec = {
chartType: ChartTypes.XYAxis,
specType: SpecTypes.Axis,
id: getAxisId('vertical_axis'),
id: 'vertical_axis',
groupId,
hide: false,
showOverlappingTicks: false,
Expand All @@ -84,7 +84,7 @@ describe('annotation utils', () => {
const horizontalAxisSpec: AxisSpec = {
chartType: ChartTypes.XYAxis,
specType: SpecTypes.Axis,
id: getAxisId('horizontal_axis'),
id: 'horizontal_axis',
groupId,
hide: false,
showOverlappingTicks: false,
Expand Down Expand Up @@ -1356,7 +1356,7 @@ describe('annotation utils', () => {
chartType: ChartTypes.XYAxis,
specType: SpecTypes.Annotation,
id: 'rect',
groupId: getGroupId('foo'),
groupId: 'foo',
annotationType: AnnotationTypes.Rectangle,
dataValues: [{ coordinates: { x0: 1, x1: 2, y0: 3, y1: 5 } }],
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,24 +1,23 @@
import { computeXScale } from '../utils/scales';
import { BasicSeriesSpec, SeriesTypes } from '../utils/specs';
import { Dimensions } from '../../../utils/dimensions';
import { getGroupId, getSpecId } from '../../../utils/ids';
import { ScaleType } from '../../../scales';
import { getCursorBandPosition, getSnapPosition } from './crosshair_utils';
import { computeSeriesDomains } from '../state/utils';
import { ChartTypes } from '../..';
import { SpecTypes } from '../../../specs/settings';

describe('Crosshair utils linear scale', () => {
const barSeries1SpecId = getSpecId('barSeries1');
const barSeries2SpecId = getSpecId('barSeries2');
const lineSeries1SpecId = getSpecId('lineSeries1');
const lineSeries2SpecId = getSpecId('lineSeries2');
const barSeries1SpecId = 'barSeries1';
const barSeries2SpecId = 'barSeries2';
const lineSeries1SpecId = 'lineSeries1';
const lineSeries2SpecId = 'lineSeries2';

const barSeries1: BasicSeriesSpec = {
chartType: ChartTypes.XYAxis,
specType: SpecTypes.Series,
id: barSeries1SpecId,
groupId: getGroupId('group1'),
groupId: 'group1',
seriesType: SeriesTypes.Bar,
data: [
[0, 0],
Expand All @@ -35,7 +34,7 @@ describe('Crosshair utils linear scale', () => {
chartType: ChartTypes.XYAxis,
specType: SpecTypes.Series,
id: barSeries2SpecId,
groupId: getGroupId('group1'),
groupId: 'group1',
seriesType: SeriesTypes.Bar,
data: [
[0, 2],
Expand All @@ -52,7 +51,7 @@ describe('Crosshair utils linear scale', () => {
chartType: ChartTypes.XYAxis,
specType: SpecTypes.Series,
id: lineSeries1SpecId,
groupId: getGroupId('group1'),
groupId: 'group1',
seriesType: SeriesTypes.Line,
data: [
[0, 0],
Expand All @@ -69,7 +68,7 @@ describe('Crosshair utils linear scale', () => {
chartType: ChartTypes.XYAxis,
specType: SpecTypes.Series,
id: lineSeries2SpecId,
groupId: getGroupId('group1'),
groupId: 'group1',
seriesType: SeriesTypes.Line,
data: [
[0, 2],
Expand Down
Original file line number Diff line number Diff line change
@@ -1,23 +1,22 @@
import { computeXScale } from '../utils/scales';
import { BasicSeriesSpec, SeriesTypes } from '../utils/specs';
import { getGroupId, getSpecId } from '../../../utils/ids';
import { ScaleType } from '../../../scales';
import { getSnapPosition } from './crosshair_utils';
import { computeSeriesDomains } from '../state/utils';
import { ChartTypes } from '../..';
import { SpecTypes } from '../../../specs/settings';

describe('Crosshair utils ordinal scales', () => {
const barSeries1SpecId = getSpecId('barSeries1');
const barSeries2SpecId = getSpecId('barSeries2');
const lineSeries1SpecId = getSpecId('lineSeries1');
const lineSeries2SpecId = getSpecId('lineSeries2');
const barSeries1SpecId = 'barSeries1';
const barSeries2SpecId = 'barSeries2';
const lineSeries1SpecId = 'lineSeries1';
const lineSeries2SpecId = 'lineSeries2';

const barSeries1: BasicSeriesSpec = {
chartType: ChartTypes.XYAxis,
specType: SpecTypes.Series,
id: barSeries1SpecId,
groupId: getGroupId('group1'),
groupId: 'group1',
seriesType: SeriesTypes.Bar,
data: [
['a', 0],
Expand All @@ -34,7 +33,7 @@ describe('Crosshair utils ordinal scales', () => {
chartType: ChartTypes.XYAxis,
specType: SpecTypes.Series,
id: barSeries2SpecId,
groupId: getGroupId('group1'),
groupId: 'group1',
seriesType: SeriesTypes.Bar,
data: [
['a', 2],
Expand All @@ -51,7 +50,7 @@ describe('Crosshair utils ordinal scales', () => {
chartType: ChartTypes.XYAxis,
specType: SpecTypes.Series,
id: lineSeries1SpecId,
groupId: getGroupId('group1'),
groupId: 'group1',
seriesType: SeriesTypes.Line,
data: [
['a', 0],
Expand All @@ -68,7 +67,7 @@ describe('Crosshair utils ordinal scales', () => {
chartType: ChartTypes.XYAxis,
specType: SpecTypes.Series,
id: lineSeries2SpecId,
groupId: getGroupId('group1'),
groupId: 'group1',
seriesType: SeriesTypes.Line,
data: [
['a', 2],
Expand Down
4 changes: 2 additions & 2 deletions src/chart_types/xy_chart/domains/y_domain.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { BasicSeriesSpec, DomainRange, DEFAULT_GLOBAL_ID, SeriesTypes } from '../utils/specs';
import { GroupId, SpecId, getGroupId } from '../../../utils/ids';
import { GroupId, SpecId } from '../../../utils/ids';
import { ScaleContinuousType, ScaleType } from '../../../scales';
import { isCompleteBound, isLowerBound, isUpperBound } from '../utils/axis_utils';
import { BaseDomain } from './domain';
Expand Down Expand Up @@ -33,7 +33,7 @@ export function mergeYDomain(
// group specs by group ids
const specsByGroupIds = splitSpecsByGroupId(specs);
const specsByGroupIdsEntries = [...specsByGroupIds.entries()];
const globalId = getGroupId(DEFAULT_GLOBAL_ID);
const globalId = DEFAULT_GLOBAL_ID;

const yDomains = specsByGroupIdsEntries.map<YDomain>(([groupId, groupSpecs]) => {
const customDomain = domainsByGroupId.get(groupId);
Expand Down
Loading

0 comments on commit 124c458

Please sign in to comment.