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

feat: expose NaN values to tooltips #1206

Closed
Closed
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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
"test:tz-ny": "TZ=America/New_York jest --verbose --config=jest.tz.config.js",
"test:tz-jp": "TZ=Asia/Tokyo jest --verbose --config=jest.tz.config.js",
"test:integration": "./scripts/vrt.sh",
"test:integration:local": "LOCAL_VRT_SERVER=true PORT=9001 yarn test:integration",
"test:integration:local": "LOCAL_VRT_SERVER=true PORT=9002 yarn test:integration",
"test:integration:debug": "DEBUG=true yarn test:integration:local",
"test:integration:legacy": "LEGACY_VRT_SERVER=true yarn test:integration",
"test:integration:local:legacy": "LOCAL_VRT_SERVER=true PORT=9002 yarn test:integration:legacy",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -690,7 +690,7 @@ describe('Annotation utils', () => {

const emptyOutOfBoundsYDimensions = computeAnnotationDimensionsSelector(store.getState());

expect(emptyOutOfBoundsYDimensions.get('foo-line')).toHaveLength(0);
expect((emptyOutOfBoundsYDimensions.get('foo-line') as AnnotationLineProps[])[0].linePathPoints.y1).toBeNaN();

const outOfBoundsYLineAnnotation = MockAnnotationSpec.line({
id: annotationId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ function computeYDomainLineAnnotationDimensions(

vertical.domain.forEach((verticalValue) => {
horizontal.domain.forEach((horizontalValue) => {
const top = vertical.scaleOrThrow(verticalValue);
const left = horizontal.scaleOrThrow(horizontalValue);
const top = vertical.scale(verticalValue);
const left = horizontal.scale(horizontalValue);

const width = isHorizontalChartRotation ? horizontal.bandwidth : vertical.bandwidth;
const height = isHorizontalChartRotation ? vertical.bandwidth : horizontal.bandwidth;
Expand Down Expand Up @@ -193,8 +193,8 @@ function computeXDomainLineAnnotationDimensions(
return;
}

const top = vertical.scaleOrThrow(verticalValue);
const left = horizontal.scaleOrThrow(horizontalValue);
const top = vertical.scale(verticalValue);
const left = horizontal.scale(horizontalValue);
const width = isHorizontalChartRotation ? horizontal.bandwidth : vertical.bandwidth;
const height = isHorizontalChartRotation ? vertical.bandwidth : horizontal.bandwidth;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ export function computeRectAnnotationDimensions(
smallMultiplesScales.horizontal.domain.forEach((hDomainValue) => {
const panel = {
...panelSize,
top: smallMultiplesScales.vertical.scaleOrThrow(vDomainValue),
left: smallMultiplesScales.horizontal.scaleOrThrow(hDomainValue),
top: smallMultiplesScales.vertical.scale(vDomainValue),
left: smallMultiplesScales.horizontal.scale(hDomainValue),
};
duplicated.push({ ...props, panel });
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ describe('Crosshair utils ordinal scales', () => {
expect(snappedPosition?.position).toEqual(80);

snappedPosition = getSnapPosition('x', barSeriesScale);
expect(snappedPosition).toBeUndefined();
expect(snappedPosition).toEqual({ band: 40, position: NaN });

snappedPosition = getSnapPosition('a', multiBarSeriesScale, 2);
expect(snappedPosition?.band).toEqual(40);
Expand Down Expand Up @@ -197,7 +197,7 @@ describe('Crosshair utils ordinal scales', () => {
expect(snappedPosition?.position).toEqual(80);

snappedPosition = getSnapPosition('x', lineSeriesScale);
expect(snappedPosition).toBeUndefined();
expect(snappedPosition).toEqual({ band: 40, position: NaN });

snappedPosition = getSnapPosition('a', multiLineSeriesScale, 2);
expect(snappedPosition?.band).toEqual(40);
Expand Down Expand Up @@ -226,6 +226,6 @@ describe('Crosshair utils ordinal scales', () => {
expect(snappedPosition?.position).toEqual(80);

snappedPosition = getSnapPosition('x', mixedLinesBarsSeriesScale, 4);
expect(snappedPosition).toBeUndefined();
expect(snappedPosition).toEqual({ band: 40, position: NaN });
});
});
53 changes: 53 additions & 0 deletions packages/charts/src/chart_types/xy_chart/data/validate_datum.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/*
* 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 { Accessor, AccessorFn } from '../../../utils/accessor';
import { Datum, isNil } from '../../../utils/common';

/** @public */
export interface DatumMetadata<T> {
validated: T;
hasAccessor: boolean;
isNil: boolean;
isFilled: boolean;
value: any;
}
/** @internal */
export function getDatumNumericProperty(datum: Datum, accessor?: Accessor | AccessorFn): DatumMetadata<number> {
const value = !isNil(accessor) ? getValueViaAccessor(datum, accessor) : undefined;
return {
validated: parseFloat(value),
hasAccessor: !isNil(accessor),
isNil: isNil(value),
isFilled: false,
value,
};
}

/**
* Helper function to get accessor value from string, number or function
* @internal
*/
function getValueViaAccessor(datum: Datum, accessor: Accessor | AccessorFn) {
if (typeof accessor === 'function') {
return accessor(datum);
}

return datum[accessor];
}
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ describe('Legends', () => {
});
it('should return correct legendSizingLabel with linear scale and showExtraLegend set to true', () => {
const formatter = (d: string | number) => `${Number(d).toFixed(2)} dogs`;
const lastValues = { y0: null, y1: 14 };
const lastValues = { y0: NaN, y1: 14 };
const showExtraLegend = true;
const xScaleIsLinear = ScaleType.Linear;

Expand All @@ -423,7 +423,7 @@ describe('Legends', () => {
});
it('should return formatted to null with ordinal scale and showExtraLegend set to true', () => {
const formatter = (d: string | number) => `${Number(d).toFixed(2)} dogs`;
const lastValues = { y0: null, y1: 14 };
const lastValues = { y0: NaN, y1: 14 };

expect(getLegendExtra(true, ScaleType.Ordinal, formatter, 'y1', lastValues)).toMatchObject({
raw: 14,
Expand All @@ -433,7 +433,7 @@ describe('Legends', () => {
});
it('should return legendSizingLabel null with showLegendExtra set to false', () => {
const formatter = (d: string | number) => `${Number(d).toFixed(2)} dogs`;
const lastValues = { y0: null, y1: 14 };
const lastValues = { y0: NaN, y1: 14 };
const showLegendExtra = false;

expect(getLegendExtra(showLegendExtra, ScaleType.Ordinal, formatter, 'y1', lastValues)).toMatchObject({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,10 @@ export function renderAreas(ctx: CanvasRenderingContext2D, imgCanvas: HTMLCanvas
(ctx) => {
renderPoints(ctx, visiblePoints, geometryStateStyle);
},
{ area: clippings, shouldClip: points[0]?.value.mark !== null },
{
area: clippings,
shouldClip: points.length > 0 && points[0].value.mark !== null && !isNaN(points[0].value.mark),
},
);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export function renderLines(ctx: CanvasRenderingContext2D, props: LineGeometries
if (visiblePoints.length === 0) {
return;
}

const geometryStyle = getGeometryStateStyle(line.seriesIdentifier, sharedStyle, highlightedLegendItem);
withPanelTransform(
ctx,
Expand All @@ -75,7 +76,7 @@ export function renderLines(ctx: CanvasRenderingContext2D, props: LineGeometries
renderPoints(ctx, visiblePoints, geometryStyle);
},
// TODO: add padding over clipping
{ area: clippings, shouldClip: line.points[0]?.value.mark !== null },
{ area: clippings, shouldClip: points.length > 0 && !isNaN(points[0].metadata.radius.validated) },
);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ import { withPanelTransform } from './utils/panel_transform';
*/
export function renderPoints(ctx: CanvasRenderingContext2D, points: PointGeometry[], { opacity }: GeometryStateStyle) {
points
// do not render non-finite y values and the filled ones
.filter(({ metadata, y }) => isFinite(y) && !metadata.y.isFilled)
.map<[Circle, Fill, Stroke, PointShape]>(({ x, y, radius, transform, style }) => {
const fill: Fill = {
color: applyOpacity(style.fill.color, opacity),
Expand All @@ -49,7 +51,6 @@ export function renderPoints(ctx: CanvasRenderingContext2D, points: PointGeometr
y: y + transform.y,
radius,
};

return [coordinates, fill, stroke, style.shape];
})
.sort(([{ radius: a }], [{ radius: b }]) => b - a)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ function renderPathFill(ctx: CanvasRenderingContext2D, path: string, fill: Fill)

ctx.fillStyle = fill.texture.pattern;

// Use oversized rect to fill rotation/offset beyond path
// Use oversize rect to fill rotation/offset beyond path
const rotationRectFillSize = ctx.canvas.clientWidth * ctx.canvas.clientHeight;
ctx.translate(-rotationRectFillSize / 2, -rotationRectFillSize / 2);
ctx.fillRect(0, 0, rotationRectFillSize, rotationRectFillSize);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ export function renderShape(
withContext(ctx, (ctx) => {
const [pathFn, rotation] = ShapeRendererFn[shape];
const { x, y, radius } = coordinates;
if (isNaN(x) || isNaN(y)) {
return;
}
ctx.translate(x, y);
ctx.rotate(getRadians(rotation));
ctx.beginPath();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ export function renderBarValues(ctx: CanvasRenderingContext2D, props: BarValuesP
barSeriesStyle.displayValue,
alignment,
);
if (isNaN(bars[i].value.y)) {
continue;
}

if (displayValue.isValueContainedInElement) {
const width = rotation === 0 || rotation === 180 ? bars[i].width : bars[i].height;
Expand Down
10 changes: 5 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 @@ -31,8 +31,8 @@ import { PointStyleAccessor } from '../utils/specs';
import { renderPoints } from './points';
import {
getClippedRanges,
getY0ScaledValueOrThrowFn,
getY1ScaledValueOrThrowFn,
getY0ScaledValue,
getY1ScaledValue,
getYDatumValueFn,
isYValueDefinedFn,
MarkSizeOptions,
Expand All @@ -58,13 +58,13 @@ export function renderArea(
areaGeometry: AreaGeometry;
indexedGeometryMap: IndexedGeometryMap;
} {
const y1Fn = getY1ScaledValueOrThrowFn(yScale);
const y0Fn = getY0ScaledValueOrThrowFn(yScale);
const y1Fn = getY1ScaledValue(yScale);
const y0Fn = getY0ScaledValue(yScale);
const definedFn = isYValueDefinedFn(yScale, xScale);
const y1DatumAccessor = getYDatumValueFn();
const y0DatumAccessor = getYDatumValueFn('y0');
const pathGenerator = area<DataSeriesDatum>()
.x(({ x }) => xScale.scaleOrThrow(x) - xScaleOffset)
.x(({ x }) => xScale.scale(x) - xScaleOffset)
.y1(y1Fn)
.y0(y0Fn)
.defined((datum) => {
Expand Down
34 changes: 10 additions & 24 deletions packages/charts/src/chart_types/xy_chart/rendering/bars.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,38 +56,28 @@ export function renderBars(
const { fontSize, fontFamily } = sharedSeriesStyle.displayValue;

dataSeries.data.forEach((datum) => {
const { y0, y1, initialY1, filled } = datum;
// don't create a bar if the initialY1 value is null.
if (y1 === null || initialY1 === null || (filled && filled.y1 !== undefined)) {
return;
}
const { y0, y1, metadata } = datum;

// don't create a bar if not within the xScale domain
if (!xScale.isValueInDomain(datum.x)) {
return;
}

let y: number | null;
let y: number;
let y0Scaled;
if (yScale.type === ScaleType.Log) {
y = y1 === 0 || y1 === null ? yScale.range[0] : yScale.scale(y1);
if (yScale.isInverted) {
y0Scaled = y0 === 0 || y0 === null ? yScale.range[1] : yScale.scale(y0);
} else {
y0Scaled = y0 === 0 || y0 === null ? yScale.range[0] : yScale.scale(y0);
}
const minRangeIndex = !yScale.isInverted ? 0 : 1;
y = y1 === 0 || isNaN(y1) ? yScale.range[minRangeIndex] : yScale.scale(y1);
y0Scaled = y0 === 0 || isNaN(y0) ? yScale.range[minRangeIndex] : yScale.scale(y0);
} else {
y = yScale.scale(y1);
// use always zero as baseline if y0 is null
y0Scaled = y0 === null ? yScale.scale(0) : yScale.scale(y0);
}

if (y === null || y0Scaled === null) {
return;
y0Scaled = isNaN(y0) ? yScale.scale(0) : yScale.scale(y0);
}

const absMinHeight = Math.abs(minBarHeight);
let height = y0Scaled - y;
if (absMinHeight !== undefined && height !== 0 && Math.abs(height) < absMinHeight) {
if (height !== 0 && Math.abs(height) < absMinHeight) {
const heightDelta = absMinHeight - Math.abs(height);
if (height < 0) {
height = -absMinHeight;
Expand All @@ -103,10 +93,6 @@ export function renderBars(

const xScaled = xScale.scale(datum.x);

if (xScaled === null) {
return;
}

const seriesIdentifier: XYChartSeriesIdentifier = {
key: dataSeries.key,
specId: dataSeries.specId,
Expand All @@ -125,7 +111,7 @@ export function renderBars(
const width = clamp(seriesStyle.rect.widthPixel ?? xScale.bandwidth, minPixelWidth, maxPixelWidth);
const x = xScaled + xScale.bandwidth * orderIndex + xScale.bandwidth / 2 - width / 2;

const originalY1Value = stackMode === StackMode.Percentage ? y1 - (y0 ?? 0) : initialY1;
const originalY1Value = stackMode === StackMode.Percentage ? y1 - y0 : metadata.y1.validated;
const formattedDisplayValue =
displayValueSettings && displayValueSettings.valueFormatter
? displayValueSettings.valueFormatter(originalY1Value)
Expand Down Expand Up @@ -186,7 +172,7 @@ export function renderBars(
value: {
x: datum.x,
y: originalY1Value,
mark: null,
mark: NaN,
accessor: BandedAccessorType.Y1,
datum: datum.datum,
},
Expand Down
13 changes: 3 additions & 10 deletions packages/charts/src/chart_types/xy_chart/rendering/line.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,7 @@ import { IndexedGeometryMap } from '../utils/indexed_geometry_map';
import { DataSeries, DataSeriesDatum } from '../utils/series';
import { PointStyleAccessor } from '../utils/specs';
import { renderPoints } from './points';
import {
getClippedRanges,
getY1ScaledValueOrThrowFn,
getYDatumValueFn,
isYValueDefinedFn,
MarkSizeOptions,
} from './utils';
import { getClippedRanges, getY1ScaledValue, getYDatumValueFn, isYValueDefinedFn, MarkSizeOptions } from './utils';

/** @internal */
export function renderLine(
Expand All @@ -56,12 +50,12 @@ export function renderLine(
lineGeometry: LineGeometry;
indexedGeometryMap: IndexedGeometryMap;
} {
const y1Fn = getY1ScaledValueOrThrowFn(yScale);
const y1Fn = getY1ScaledValue(yScale);
const definedFn = isYValueDefinedFn(yScale, xScale);
const y1Accessor = getYDatumValueFn();

const pathGenerator = line<DataSeriesDatum>()
.x(({ x }) => xScale.scaleOrThrow(x) - xScaleOffset)
.x(({ x }) => xScale.scale(x) - xScaleOffset)
.y(y1Fn)
.defined((datum) => {
return definedFn(datum, y1Accessor);
Expand All @@ -83,7 +77,6 @@ export function renderLine(

const clippedRanges = getClippedRanges(dataSeries.data, xScale, xScaleOffset);
let linePath: string;

try {
linePath = pathGenerator(dataSeries.data) || '';
} catch {
Expand Down
Loading