Skip to content

Commit

Permalink
refactor: rename banded spec check to be consistent throughout
Browse files Browse the repository at this point in the history
  • Loading branch information
nickofthyme committed Dec 9, 2021
1 parent 9eb5bf4 commit 73a9e3c
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 41 deletions.
5 changes: 2 additions & 3 deletions packages/charts/src/chart_types/xy_chart/legend/legend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import { mergePartial, Rotation } from '../../../utils/common';
import { BandedAccessorType } from '../../../utils/geometry';
import { getLegendCompareFn } from '../../../utils/series_sort';
import { PointStyle, Theme } from '../../../utils/themes/theme';
import { isStackedSpec } from '../domains/y_domain';
import { getXScaleTypeFromSpec } from '../scales/get_api_scales';
import { getAxesSpecForSpecId, getSpecsById } from '../state/utils/spec';
import { LastValues } from '../state/utils/types';
Expand All @@ -28,7 +27,7 @@ import {
getSeriesName,
DataSeries,
getSeriesKey,
isDataSeriesBanded,
isBandedSpec,
getSeriesIdentifierFromDataSeries,
} from '../utils/series';
import {
Expand Down Expand Up @@ -112,7 +111,7 @@ export function computeLegend(

dataSeries.forEach((series) => {
const { specId, yAccessor } = series;
const banded = isDataSeriesBanded(series) && !isStackedSpec(series.spec, false);
const banded = isBandedSpec(series.spec);
const key = getSeriesKey(series, series.groupId);
const spec = getSpecsById<BasicSeriesSpec>(specs, specId);
const dataSeriesKey = getSeriesKey(
Expand Down
9 changes: 4 additions & 5 deletions packages/charts/src/chart_types/xy_chart/rendering/area.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export function renderArea(
panel: Dimensions,
color: Color,
curve: CurveType,
hasY0Accessors: boolean,
isBandedSpec: boolean,
xScaleOffset: number,
style: AreaSeriesStyle,
markSizeOptions: MarkSizeOptions,
Expand All @@ -47,7 +47,6 @@ export function renderArea(
areaGeometry: AreaGeometry;
indexedGeometryMap: IndexedGeometryMap;
} {
const isBandChart = hasY0Accessors && !isStacked;
const y1Fn = getY1ScaledValueFn(yScale);
const y0Fn = getY0ScaledValueFn(yScale);
const definedFn = isYValueDefinedFn(yScale, xScale);
Expand All @@ -58,15 +57,15 @@ export function renderArea(
.y1(y1Fn)
.y0(y0Fn)
.defined((datum) => {
return definedFn(datum, y1DatumAccessor) && (isBandChart ? definedFn(datum, y0DatumAccessor) : true);
return definedFn(datum, y1DatumAccessor) && (isBandedSpec ? definedFn(datum, y0DatumAccessor) : true);
})
.curve(getCurveFactory(curve));

// TODO we can probably avoid this function call if no fit function is applied.
const clippedRanges = getClippedRanges(dataSeries.data, xScale, xScaleOffset);

const lines: string[] = [];
const y0Line = isBandChart && pathGenerator.lineY0()(dataSeries.data);
const y0Line = isBandedSpec && pathGenerator.lineY0()(dataSeries.data);
const y1Line = pathGenerator.lineY1()(dataSeries.data);
if (y1Line) lines.push(y1Line);
if (y0Line) lines.push(y0Line);
Expand All @@ -79,7 +78,7 @@ export function renderArea(
panel,
color,
style.point,
isBandChart,
isBandedSpec,
markSizeOptions,
false,
pointStyleAccessor,
Expand Down
15 changes: 10 additions & 5 deletions packages/charts/src/chart_types/xy_chart/state/utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,13 @@ import { fillSeries } from '../../utils/fill_series';
import { groupBy } from '../../utils/group_data_series';
import { IndexedGeometryMap } from '../../utils/indexed_geometry_map';
import { computeXScale, computeYScales } from '../../utils/scales';
import { DataSeries, getDataSeriesFromSpecs, getFormattedDataSeries, getSeriesKey } from '../../utils/series';
import {
DataSeries,
getDataSeriesFromSpecs,
getFormattedDataSeries,
getSeriesKey,
isBandedSpec,
} from '../../utils/series';
import {
AxisSpec,
BasicSeriesSpec,
Expand All @@ -46,7 +52,6 @@ import {
HistogramModeAlignment,
HistogramModeAlignments,
isAreaSeriesSpec,
isBandedSpec,
isBarSeriesSpec,
isBubbleSeriesSpec,
isLineSeriesSpec,
Expand Down Expand Up @@ -384,7 +389,7 @@ function renderGeometries(
yScale,
color,
panel,
isBandedSpec(spec.y0Accessors),
isBandedSpec(spec),
xScaleOffset,
bubbleSeriesStyle,
{
Expand Down Expand Up @@ -418,7 +423,7 @@ function renderGeometries(
panel,
color,
spec.curve || CurveType.LINEAR,
isBandedSpec(spec.y0Accessors),
isBandedSpec(spec),
xScaleOffset,
lineSeriesStyle,
{
Expand Down Expand Up @@ -451,7 +456,7 @@ function renderGeometries(
panel,
color,
spec.curve || CurveType.LINEAR,
isBandedSpec(spec.y0Accessors),
isBandedSpec(spec),
xScaleOffset,
areaSeriesStyle,
{
Expand Down
18 changes: 3 additions & 15 deletions packages/charts/src/chart_types/xy_chart/tooltip/tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,9 @@ import { TooltipValue } from '../../../specs';
import { getAccessorFormatLabel } from '../../../utils/accessor';
import { isDefined } from '../../../utils/common';
import { BandedAccessorType, IndexedGeometry } from '../../../utils/geometry';
import { isStackedSpec } from '../domains/y_domain';
import { defaultTickFormatter } from '../utils/axis_utils';
import { getSeriesName } from '../utils/series';
import {
AxisSpec,
BasicSeriesSpec,
isAreaSeriesSpec,
isBandedSpec,
isBarSeriesSpec,
TickFormatterOptions,
} from '../utils/specs';
import { getSeriesName, isBandedSpec } from '../utils/series';
import { AxisSpec, BasicSeriesSpec, isAreaSeriesSpec, isBarSeriesSpec, TickFormatterOptions } from '../utils/specs';

/** @internal */
export const Y0_ACCESSOR_POSTFIX = ' - lower';
Expand Down Expand Up @@ -67,11 +59,7 @@ export function formatTooltip(
): TooltipValue {
let label = getSeriesName(seriesIdentifier, hasSingleSeries, true, spec);

if (
isBandedSpec(spec.y0Accessors) &&
!isStackedSpec(spec, false) &&
(isAreaSeriesSpec(spec) || isBarSeriesSpec(spec))
) {
if (isBandedSpec(spec) && (isAreaSeriesSpec(spec) || isBarSeriesSpec(spec))) {
const { y0AccessorFormat = Y0_ACCESSOR_POSTFIX, y1AccessorFormat = Y1_ACCESSOR_POSTFIX } = spec;
const formatter = accessor === BandedAccessorType.Y0 ? y0AccessorFormat : y1AccessorFormat;
label = getAccessorFormatLabel(formatter, label);
Expand Down
18 changes: 10 additions & 8 deletions packages/charts/src/chart_types/xy_chart/utils/series.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,8 @@ export function splitSeriesDataByAccessors(
const dataSeries = new Map<SeriesKey, DataSeries>();
const xValues: Array<string | number> = [];
const nonNumericValues: any[] = [];
const hasStackedBands = isStacked && Boolean(y0Accessors?.length);

if (hasStackedBands) {
if (isStacked && Boolean(y0Accessors?.length)) {
Logger.warn(
`y0Accessors are not allowed with stackAccessors. y0Accessors will be ignored but available under initialY0.`,
);
Expand Down Expand Up @@ -168,7 +167,7 @@ export function splitSeriesDataByAccessors(
datum,
accessor,
nonNumericValues,
hasStackedBands,
isBandedSpec(spec),
y0Accessors && y0Accessors[index],
markSizeAccessor,
);
Expand Down Expand Up @@ -278,7 +277,7 @@ export function extractYAndMarkFromDatum(
datum: Datum,
yAccessor: Accessor | AccessorFn,
nonNumericValues: any[],
hasStackedBands: boolean,
bandedSpec: boolean,
y0Accessor?: Accessor | AccessorFn,
markSizeAccessor?: Accessor | AccessorFn,
): Pick<DataSeriesDatum, 'y0' | 'y1' | 'mark' | 'datum' | 'initialY0' | 'initialY1'> {
Expand All @@ -287,7 +286,7 @@ export function extractYAndMarkFromDatum(
const y1Value = getAccessorValue(datum, yAccessor);
const y1 = finiteOrNull(y1Value, nonNumericValues);
const y0 = y0Accessor ? finiteOrNull(getAccessorValue(datum, y0Accessor), nonNumericValues) : null;
return { y1, datum, y0: hasStackedBands ? null : y0, mark, initialY0: y0, initialY1: y1 };
return { y1, datum, y0: bandedSpec ? y0 : null, mark, initialY0: y0, initialY1: y1 };
}

function finiteOrNull(value: unknown, nonNumericValues: unknown[]): number | null {
Expand Down Expand Up @@ -453,9 +452,12 @@ export function getDataSeriesFromSpecs(
};
}

/** @internal */
export function isDataSeriesBanded({ spec }: DataSeries) {
return spec.y0Accessors && spec.y0Accessors.length > 0;
/**
* TODO: Add check for chart type other than area and bar.
* @internal
*/
export function isBandedSpec(spec: BasicSeriesSpec): boolean {
return Boolean(spec.y0Accessors && spec.y0Accessors.length > 0 && !isStackedSpec(spec, false));
}

function getSortedOrdinalXValues(
Expand Down
5 changes: 0 additions & 5 deletions packages/charts/src/chart_types/xy_chart/utils/specs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1000,8 +1000,3 @@ export function isLineSeriesSpec(spec: BasicSeriesSpec): spec is LineSeriesSpec
export function isAreaSeriesSpec(spec: BasicSeriesSpec): spec is AreaSeriesSpec {
return spec.seriesType === SeriesType.Area;
}

/** @internal */
export function isBandedSpec(y0Accessors: SeriesAccessors['y0Accessors']): boolean {
return Boolean(y0Accessors && y0Accessors.length > 0);
}

0 comments on commit 73a9e3c

Please sign in to comment.