Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(xy): stacked polarity (#1502) [40.1.x] #1522

Merged
merged 1 commit into from
Dec 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,6 @@ coverage/
npm-debug.log
yarn-error.log
playground/playground.tsx
.jest-image-snapshot-touched-files

**/__diff_output__/
2 changes: 1 addition & 1 deletion NOTICE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ THIS SOFTWARE.

---

This product includes code that is adapted from d3-array@3.0.4 and d3-scale@4.0.2,
This product includes code that is adapted from d3-shape@3.0.1, d3-array@3.0.4 and d3-scale@4.0.2,
which are both available under a "ISC" license.

ISC License
Expand Down
7 changes: 5 additions & 2 deletions integration/page_objects/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -362,9 +362,12 @@ class CommonPage {
async expectChartWithMouseAtUrlToMatchScreenshot(
url: string,
mousePosition: MousePosition,
options?: Omit<ScreenshotElementAtUrlOptions, 'action'>,
options?: ScreenshotElementAtUrlOptions,
) {
const action = async () => await this.moveMouseRelativeToDOMElement(mousePosition, this.chartSelector);
const action = async () => {
await options?.action?.();
await this.moveMouseRelativeToDOMElement(mousePosition, this.chartSelector);
};
await this.expectChartAtUrlToMatchScreenshot(url, {
...options,
action,
Expand Down
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
Diff not rendered.
16 changes: 0 additions & 16 deletions integration/tests/area_stories.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,6 @@ describe('Area series stories', () => {
});
});

describe('scale to extents', () => {
describe('domain.fit is true', () => {
const trueUrl = 'http://localhost:9001/?path=/story/area-chart--stacked-band&knob-fit Y domain=true';
it('should show correct extents - Banded', async () => {
await common.expectChartAtUrlToMatchScreenshot(trueUrl);
});
});

describe('domain.fit is false', () => {
const falseUrl = 'http://localhost:9001/?path=/story/area-chart--stacked-band&knob-fit Y domain=false';

it('should show correct extents - Banded', async () => {
await common.expectChartAtUrlToMatchScreenshot(falseUrl);
});
});
});
describe('Non-Stacked Linear Area with discontinuous data points', () => {
it('with fit', async () => {
await common.expectChartAtUrlToMatchScreenshot(
Expand Down
25 changes: 24 additions & 1 deletion integration/tests/mixed_stories.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import { Fit } from '../../packages/charts/src';
import { Fit, StackMode, SeriesType } from '../../packages/charts/src';
import { common } from '../page_objects';

describe('Mixed series stories', () => {
Expand Down Expand Up @@ -195,4 +195,27 @@ describe('Mixed series stories', () => {
});
});
});

describe.each(Object.values(StackMode))('Stack mode - %s', (mode) => {
describe.each(['Mixed', 'Positive', 'Negative'])('Polarity - %s', (polarity) => {
describe.each([SeriesType.Bar, SeriesType.Area])('%s series', (type) => {
it('should display correct stacking', async () => {
await common.expectChartAtUrlToMatchScreenshot(
`http://localhost:9001/?path=/story/mixed-charts---polarized-stacked&globals=theme:light&knob-stacked=true&knob-data polarity=${polarity}&knob-custom domain=false&knob-stackMode=${mode}&knob-SeriesType=${type}`,
);
});

it('should show area chart with toggled series and mouse over', async () => {
const action = async () => {
await page.click('.echLegendItem:nth-child(2) .echLegendItem__label');
};
await common.expectChartWithMouseAtUrlToMatchScreenshot(
`http://localhost:9001/?path=/story/mixed-charts---polarized-stacked&globals=theme:light&knob-stacked=true&knob-data polarity=${polarity}&knob-custom domain=false&knob-stackMode=${mode}&knob-SeriesType=${type}`,
{ top: 170, left: 490 },
{ action },
);
});
});
});
});
});
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@
"@types/d3-collection": "^1.0.8",
"@types/d3-interpolate": "^1.3.1",
"@types/d3-scale": "^2.1.1",
"@types/d3-shape": "^1.3.1",
"@types/d3-shape": "^2.0.0",
"@types/enzyme": "^3.9.0",
"@types/enzyme-adapter-react-16": "^1.0.5",
"@types/expect-puppeteer": "^4.4.5",
Expand Down
2 changes: 1 addition & 1 deletion packages/charts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
"d3-collection": "^1.0.7",
"d3-interpolate": "^1.4.0",
"d3-scale": "^1.0.7",
"d3-shape": "^1.3.4",
"d3-shape": "^2.0.0",
"prop-types": "^15.7.2",
"re-reselect": "^3.4.0",
"react-redux": "^7.1.0",
Expand Down
4 changes: 2 additions & 2 deletions packages/charts/src/chart_types/xy_chart/domains/y_domain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@ function mergeYDomainForGroup(
const dataSeries = [...stacked, ...nonStacked];
if (dataSeries.length === 0) return null;

const [{ stackMode, spec }] = dataSeries;
const [{ isStacked, stackMode, spec }] = dataSeries;
const groupId = getSpecDomainGroupId(spec);
const { customDomain, type, nice, desiredTickCount } = yScaleConfig[groupId];
const newCustomDomain: YDomainRange = customDomain ? { ...customDomain } : { min: NaN, max: NaN };
const { paddingUnit, padding, constrainPadding } = newCustomDomain;

let mergedDomain: ContinuousDomain;
if (stackMode === StackMode.Percentage) {
if (isStacked && stackMode === StackMode.Percentage) {
mergedDomain = computeContinuousDataDomain([0, 1], type, customDomain);
} else {
const stackedDomain = computeYDomain(stacked, hasZeroBaselineSpecs, type, newCustomDomain);
Expand Down
4 changes: 2 additions & 2 deletions packages/charts/src/chart_types/xy_chart/legend/legend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
getSeriesName,
DataSeries,
getSeriesKey,
isDataSeriesBanded,
isBandedSpec,
getSeriesIdentifierFromDataSeries,
} from '../utils/series';
import {
Expand Down Expand Up @@ -111,7 +111,7 @@ export function computeLegend(

dataSeries.forEach((series) => {
const { specId, yAccessor } = series;
const banded = isDataSeriesBanded(series);
const banded = isBandedSpec(series.spec);
const key = getSeriesKey(series, series.groupId);
const spec = getSpecsById<BasicSeriesSpec>(specs, specId);
const dataSeriesKey = getSeriesKey(
Expand Down
8 changes: 4 additions & 4 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,
seriesStyle: AreaSeriesStyle,
markSizeOptions: MarkSizeOptions,
Expand All @@ -57,14 +57,14 @@ export function renderArea(
.y1(y1Fn)
.y0(y0Fn)
.defined((datum) => {
return definedFn(datum, y1DatumAccessor) && (hasY0Accessors ? definedFn(datum, y0DatumAccessor) : true);
return definedFn(datum, y1DatumAccessor) && (isBandedSpec ? definedFn(datum, y0DatumAccessor) : true);
})
.curve(getCurveFactory(curve));

const clippedRanges = getClippedRanges(dataSeries.data, xScale, xScaleOffset);

const lines: string[] = [];
const y0Line = hasY0Accessors && 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 @@ -77,7 +77,7 @@ export function renderArea(
panel,
color,
seriesStyle.point,
hasY0Accessors,
isBandedSpec,
markSizeOptions,
pointStyleAccessor,
false,
Expand Down
8 changes: 4 additions & 4 deletions packages/charts/src/chart_types/xy_chart/rendering/points.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export function renderPoints(
panel: Dimensions,
color: Color,
pointStyle: PointStyle,
hasY0Accessors: boolean,
isBandChart: boolean,
markSizeOptions: MarkSizeOptions,
styleAccessor?: PointStyleAccessor,
spatial = false,
Expand Down Expand Up @@ -66,12 +66,12 @@ export function renderPoints(
if (Number.isNaN(x)) return acc;

const points: PointGeometry[] = [];
const yDatumKeyNames: Array<keyof Omit<FilledValues, 'x'>> = hasY0Accessors ? ['y0', 'y1'] : ['y1'];
const yDatumKeyNames: Array<keyof Omit<FilledValues, 'x'>> = isBandChart ? ['y0', 'y1'] : ['y1'];

yDatumKeyNames.forEach((yDatumKeyName, keyIndex) => {
const valueAccessor = getYDatumValueFn(yDatumKeyName);
const y = yDatumKeyName === 'y1' ? y1Fn(datum) : y0Fn(datum);
const originalY = getDatumYValue(datum, keyIndex === 0, hasY0Accessors, dataSeries.stackMode);
const originalY = getDatumYValue(datum, keyIndex === 0, isBandChart, dataSeries.stackMode);
const seriesIdentifier: XYChartSeriesIdentifier = {
key: dataSeries.key,
specId: dataSeries.specId,
Expand All @@ -98,7 +98,7 @@ export function renderPoints(
x: xValue,
y: originalY,
mark,
accessor: hasY0Accessors && keyIndex === 0 ? BandedAccessorType.Y0 : BandedAccessorType.Y1,
accessor: isBandChart && keyIndex === 0 ? BandedAccessorType.Y0 : BandedAccessorType.Y1,
datum: datum.datum,
},
transform: {
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
13 changes: 3 additions & 10 deletions packages/charts/src/chart_types/xy_chart/tooltip/tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,8 @@ import { getAccessorFormatLabel } from '../../../utils/accessor';
import { isDefined } from '../../../utils/common';
import { BandedAccessorType, IndexedGeometry } from '../../../utils/geometry';
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 @@ -66,7 +59,7 @@ export function formatTooltip(
): TooltipValue {
let label = getSeriesName(seriesIdentifier, hasSingleSeries, true, spec);

if (isBandedSpec(spec.y0Accessors) && (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
Loading