Skip to content

Commit

Permalink
feat: order ordinal values by sum (#814)
Browse files Browse the repository at this point in the history
  • Loading branch information
nickofthyme authored Sep 12, 2020
1 parent 8afce43 commit 5b2758b
Show file tree
Hide file tree
Showing 12 changed files with 436 additions and 76 deletions.
31 changes: 31 additions & 0 deletions api/charts.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,17 @@ export type BasicListener = () => undefined | void;
// @public (undocumented)
export type BasicSeriesSpec = SeriesSpec & SeriesAccessors & SeriesScales;

// Warning: (ae-missing-release-tag) "BinAgg" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public
export const BinAgg: Readonly<{
Sum: "sum";
None: "none";
}>;

// @public (undocumented)
export type BinAgg = $Values<typeof BinAgg>;

// Warning: (ae-missing-release-tag) "BrushAxis" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public (undocumented)
Expand Down Expand Up @@ -523,6 +534,17 @@ export const DEFAULT_TOOLTIP_TYPE: "vertical";
// @public (undocumented)
export type DefaultSettingsProps = 'id' | 'chartType' | 'specType' | 'rendering' | 'rotation' | 'resizeDebounce' | 'animateData' | 'showLegend' | 'debug' | 'tooltip' | 'showLegendExtra' | 'theme' | 'legendPosition' | 'hideDuplicateAxes' | 'brushAxis' | 'minBrushDelta' | 'externalPointerEvents';

// Warning: (ae-missing-release-tag) "Direction" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public
export const Direction: Readonly<{
Ascending: "ascending";
Descending: "descending";
}>;

// @public (undocumented)
export type Direction = $Values<typeof Direction>;

// Warning: (ae-missing-release-tag) "DisplayValueSpec" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public (undocumented)
Expand Down Expand Up @@ -920,6 +942,14 @@ export interface Opacity {
opacity: number;
}

// @public
export interface OrderBy {
// (undocumented)
binAgg?: BinAgg;
// (undocumented)
direction?: Direction;
}

// @public (undocumented)
export type PartialTheme = RecursivePartial<Theme>;

Expand Down Expand Up @@ -1332,6 +1362,7 @@ export interface SettingsSpec extends Spec {
onPointerUpdate?: PointerUpdateListener;
// (undocumented)
onRenderChange?: RenderChangeListener;
orderOrdinalBinsBy?: OrderBy;
// (undocumented)
pointBuffer?: MarkBuffer;
// (undocumented)
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion integration/tests/legend_stories.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('Legend stories', () => {
const action = async () => await common.moveMouseRelativeToDOMElement({ left: 30, top: 10 }, '.echLegendItem');
await common.expectChartAtUrlToMatchScreenshot('http://localhost:9001/?path=/story/legend--actions', {
action,
delay: 200, // needed for icon to load
delay: 500, // needed for icon to load
});
});

Expand Down
96 changes: 95 additions & 1 deletion src/chart_types/xy_chart/domains/x_domain.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@
*/

import { ChartTypes } from '../..';
import { MockSeriesSpecs } from '../../../mocks/specs';
import { ScaleType } from '../../../scales/constants';
import { SpecTypes } from '../../../specs/constants';
import { SpecTypes, Direction, BinAgg } from '../../../specs/constants';
import { getDataSeriesBySpecId } from '../utils/series';
import { BasicSeriesSpec, SeriesTypes } from '../utils/specs';
import { convertXScaleTypes, findMinInterval, mergeXDomain } from './x_domain';
Expand Down Expand Up @@ -837,4 +838,97 @@ describe('X Domain', () => {
expect(attemptToMerge).toThrowError(expectedError);
});
});

describe('orderOrdinalBinsBySum', () => {
const ordinalSpecs = MockSeriesSpecs.fromPartialSpecs([
{
id: 'ordinal1',
seriesType: SeriesTypes.Bar,
xScaleType: ScaleType.Ordinal,
data: [
{ x: 'a', y: 2 },
{ x: 'b', y: 4 },
{ x: 'c', y: 8 },
{ x: 'd', y: 6 },
],
},
{
id: 'ordinal2',
seriesType: SeriesTypes.Bar,
xScaleType: ScaleType.Ordinal,
data: [
{ x: 'a', y: 4 },
{ x: 'b', y: 8 },
{ x: 'c', y: 16 },
{ x: 'd', y: 12 },
],
},
]);

const linearSpecs = MockSeriesSpecs.fromPartialSpecs([
{
id: 'linear1',
seriesType: SeriesTypes.Bar,
xScaleType: ScaleType.Linear,
data: [
{ x: 1, y: 2 },
{ x: 2, y: 4 },
{ x: 3, y: 8 },
{ x: 4, y: 6 },
],
},
{
id: 'linear2',
seriesType: SeriesTypes.Bar,
xScaleType: ScaleType.Linear,
data: [
{ x: 1, y: 4 },
{ x: 2, y: 8 },
{ x: 3, y: 16 },
{ x: 4, y: 12 },
],
},
]);

it('should sort ordinal xValues by descending sum by default', () => {
const { xValues } = getDataSeriesBySpecId(ordinalSpecs, [], {});
expect(xValues).toEqual(new Set(['c', 'd', 'b', 'a']));
});

it('should sort ordinal xValues by descending sum', () => {
const { xValues } = getDataSeriesBySpecId(ordinalSpecs, [], {
binAgg: BinAgg.None,
direction: Direction.Descending,
});
expect(xValues).toEqual(new Set(['c', 'd', 'b', 'a']));
});

it('should sort ordinal xValues by ascending sum', () => {
const { xValues } = getDataSeriesBySpecId(ordinalSpecs, [], {
binAgg: BinAgg.None,
direction: Direction.Ascending,
});
expect(xValues).toEqual(new Set(['a', 'b', 'd', 'c']));
});

it('should NOT sort ordinal xValues sum', () => {
const { xValues } = getDataSeriesBySpecId(ordinalSpecs, [], undefined);
expect(xValues).toEqual(new Set(['a', 'b', 'c', 'd']));
});

it('should NOT sort ordinal xValues sum when undefined', () => {
const { xValues } = getDataSeriesBySpecId(ordinalSpecs, [], {
binAgg: BinAgg.None,
direction: Direction.Descending,
});
expect(xValues).toEqual(new Set(['a', 'b', 'c', 'd']));
});

it('should NOT sort linear xValue by descending sum', () => {
const { xValues } = getDataSeriesBySpecId(linearSpecs, [], {
direction: Direction.Descending,
});
expect(xValues).toEqual(new Set([1, 2, 3, 4]));
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export const computeSeriesDomainsSelector = createCachedSelector(
customYDomainsByGroupId,
deselectedDataSeries,
settingsSpec.xDomain,
settingsSpec.orderOrdinalBinsBy,
// @ts-ignore blind sort option for vislib
settingsSpec.enableVislibSeriesSort,
);
Expand Down
4 changes: 3 additions & 1 deletion src/chart_types/xy_chart/state/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import { SeriesKey, SeriesIdentifier } from '../../../../commons/series_id';
import { Scale } from '../../../../scales';
import { ScaleType } from '../../../../scales/constants';
import { OrderBy } from '../../../../specs/settings';
import { mergePartial, Rotation, Color, isUniqueArray } from '../../../../utils/commons';
import { CurveType } from '../../../../utils/curves';
import { Dimensions } from '../../../../utils/dimensions';
Expand Down Expand Up @@ -204,14 +205,15 @@ export function computeSeriesDomains(
customYDomainsByGroupId: Map<GroupId, YDomainRange> = new Map(),
deselectedDataSeries: SeriesIdentifier[] = [],
customXDomain?: DomainRange | Domain,
orderOrdinalBinsBy?: OrderBy,
enableVislibSeriesSort?: boolean,
): SeriesDomainsAndData {
const { dataSeriesBySpecId, xValues, seriesCollection, fallbackScale } = getDataSeriesBySpecId(
seriesSpecs,
deselectedDataSeries,
orderOrdinalBinsBy,
enableVislibSeriesSort,
);

// compute the x domain merging any custom domain
const specsArray = [...seriesSpecs.values()];
const xDomain = mergeXDomain(specsArray, xValues, customXDomain, fallbackScale);
Expand Down
Loading

0 comments on commit 5b2758b

Please sign in to comment.