Skip to content

Commit

Permalink
feat(xy): add null bars to geometry indexing (#1226)
Browse files Browse the repository at this point in the history
The `showNullValues` prop is now available in the `Settings.tooltip` to allow displaying null Y values in the tooltip.
This should go hand in hand with the right tooltip formatter to catch and correctly format these null values.

fix #950
  • Loading branch information
markov00 authored Jul 14, 2021
1 parent be1fb3d commit 20b81a9
Show file tree
Hide file tree
Showing 18 changed files with 319 additions and 92 deletions.
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.
29 changes: 29 additions & 0 deletions integration/tests/interactions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,4 +341,33 @@ describe('Interactions', () => {
);
});
});
describe('should show null values in tooltip if configured', () => {
it('show N/A tooltip for areas', async () => {
await common.expectChartWithMouseAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/interactions--interaction-with-null-values&knob-Series type=area&knob-show null values=true',
{ left: 300, top: 80 },
{
screenshotSelector: '#story-root',
},
);
});
it('show N/A tooltip for bars', async () => {
await common.expectChartWithMouseAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/interactions--interaction-with-null-values&knob-Series type=bar&knob-show null values=true',
{ left: 300, top: 80 },
{
screenshotSelector: '#story-root',
},
);
});
it('hide tooltip if configured', async () => {
await common.expectChartWithMouseAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/interactions--interaction-with-null-values&knob-Series type=bar&knob-show null values=false',
{ left: 282, top: 80 },
{
screenshotSelector: '#story-root',
},
);
});
});
});
1 change: 1 addition & 0 deletions packages/charts/api/charts.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -2107,6 +2107,7 @@ export type TooltipProps = TooltipPortalSettings<'chart'> & {
unit?: string;
customTooltip?: CustomTooltip;
stickTo?: TooltipStickTo;
showNullValues?: boolean;
};

// @public
Expand Down
24 changes: 13 additions & 11 deletions packages/charts/src/chart_types/xy_chart/rendering/bars.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import { Scale } from '../../../scales';
import { ScaleType } from '../../../scales/constants';
import { CanvasTextBBoxCalculator } from '../../../utils/bbox/canvas_text_bbox_calculator';
import { clamp, Color, mergePartial } from '../../../utils/common';
import { clamp, Color, isNil, mergePartial } from '../../../utils/common';
import { Dimensions } from '../../../utils/dimensions';
import { BandedAccessorType, BarGeometry } from '../../../utils/geometry';
import { BarSeriesStyle, DisplayValueStyle } from '../../../utils/themes/theme';
Expand Down Expand Up @@ -46,10 +46,6 @@ export function renderBars(

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;
}
// don't create a bar if not within the xScale domain
if (!xScale.isValueInDomain(datum.x)) {
return;
Expand All @@ -70,12 +66,15 @@ export function renderBars(
y0Scaled = y0 === null ? yScale.scale(0) : yScale.scale(y0);
}

if (y === null || y0Scaled === null) {
return;
const absMinHeight = Math.abs(minBarHeight);

// safeguard against null y values
let height = isNil(y0Scaled) || isNil(y) ? 0 : y0Scaled - y;

if (isNil(y0Scaled) || isNil(y)) {
y = 0;
}

const absMinHeight = Math.abs(minBarHeight);
let height = y0Scaled - y;
if (absMinHeight !== undefined && height !== 0 && Math.abs(height) < absMinHeight) {
const heightDelta = absMinHeight - Math.abs(height);
if (height < 0) {
Expand Down Expand Up @@ -114,7 +113,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 ? (isNil(y1) ? null : y1 - (y0 ?? 0)) : initialY1;
const formattedDisplayValue = displayValueSettings?.valueFormatter?.(originalY1Value);

// only show displayValue for even bars if showOverlappingValue
Expand Down Expand Up @@ -184,7 +183,10 @@ export function renderBars(
panel,
};
indexedGeometryMap.set(barGeometry);
barGeometries.push(barGeometry);

if (y1 !== null && initialY1 !== null && filled?.y1 === undefined) {
barGeometries.push(barGeometry);
}
});

bboxCalculator.destroy();
Expand Down
13 changes: 3 additions & 10 deletions packages/charts/src/chart_types/xy_chart/rendering/points.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,6 @@ export function renderPoints(
return acc;
}
// don't create the point if it that point was filled
if (isDatumFilled(datum)) {
return acc;
}
const x = xScale.scale(xValue);

if (x === null) {
Expand All @@ -79,12 +76,8 @@ export function renderPoints(
let y: number | null;
try {
y = yDatumKeyName === 'y1' ? y1Fn(datum) : y0Fn(datum);
// skip rendering point if y1 is null
if (y === null) {
return;
}
} catch {
return;
y = null;
}

const originalY = getDatumYValue(datum, keyIndex === 0, hasY0Accessors, dataSeries.stackMode);
Expand All @@ -106,7 +99,7 @@ export function renderPoints(
: styleOverrides?.radius ?? pointStyle.radius;
const pointGeometry: PointGeometry = {
x,
y,
y: y === null ? NaN : y,
radius,
color,
style,
Expand All @@ -127,7 +120,7 @@ export function renderPoints(
};
indexedGeometryMap.set(pointGeometry, geometryType);
// use the geometry only if the yDatum in contained in the current yScale domain
if (yDefined(datum, valueAccessor)) {
if (y !== null && yDefined(datum, valueAccessor) && !isDatumFilled(datum)) {
points.push(pointGeometry);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -399,9 +399,9 @@ describe('Rendering points - areas', () => {
// all the points minus the undefined ones on a log scale
expect(points.length).toBe(7);
// all the points expect null geometries
expect(geometriesIndex.size).toEqual(8);
expect(geometriesIndex.size).toEqual(9);
const nullIndexdGeometry = geometriesIndex.find(2)!;
expect(nullIndexdGeometry).toEqual([]);
expect(nullIndexdGeometry).toHaveLength(1);

const zeroValueIndexdGeometry = geometriesIndex.find(5)!;
expect(zeroValueIndexdGeometry).toBeDefined();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -359,8 +359,8 @@ describe('Rendering points - bubble', () => {
] = bubbles;
// all the points minus the undefined ones on a log scale
expect(points.length).toBe(7);
// all the points expect null geometries
expect(geometriesIndex.size).toEqual(8);
// all the points including null geometries
expect(geometriesIndex.size).toEqual(9);

const zeroValueIndexdGeometry = geometriesIndex.find(null, {
x: 56.25,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -374,10 +374,10 @@ describe('Rendering points - line', () => {
] = lines;
// all the points minus the undefined ones on a log scale
expect(points.length).toBe(7);
// all the points expect null geometries
expect(geometriesIndex.size).toEqual(8);
// all the points including null geometries
expect(geometriesIndex.size).toEqual(9);
const nullIndexdGeometry = geometriesIndex.find(2)!;
expect(nullIndexdGeometry).toEqual([]);
expect(nullIndexdGeometry).toHaveLength(1);

const zeroValueIndexdGeometry = geometriesIndex.find(5)!;
expect(zeroValueIndexdGeometry).toBeDefined();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export const getTooltipSnapSelector = createCustomCachedSelector([getSettingsSpe
function getTooltipSnap(settings: SettingsSpec): boolean {
const { tooltip } = settings;
if (tooltip && isTooltipProps(tooltip)) {
return tooltip.snap || false;
return tooltip.snap ?? DEFAULT_TOOLTIP_SNAP;
}
return DEFAULT_TOOLTIP_SNAP;
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
isFollowTooltipType,
SettingsSpec,
getTooltipType,
getShowNullValues,
} from '../../../../specs';
import { TooltipType } from '../../../../specs/constants';
import { GlobalChartState } from '../../../../state/chart_state';
Expand Down Expand Up @@ -123,66 +124,67 @@ function getTooltipAndHighlightFromValue(
let header: TooltipValue | null = null;
const highlightedGeometries: IndexedGeometry[] = [];
const xValues = new Set<any>();
const hideNullValues = !getShowNullValues(settings);
const values = matchingGeoms.reduce<TooltipValue[]>((acc, indexedGeometry) => {
if (hideNullValues && indexedGeometry.value.y === null) {
return acc;
}
const {
seriesIdentifier: { specId },
} = indexedGeometry;
const spec = getSpecsById<BasicSeriesSpec>(seriesSpecs, specId);

// safe guard check
if (!spec) {
return acc;
}
const { xAxis, yAxis } = getAxesSpecForSpecId(axesSpecs, spec.groupId);

// yScales is ensured by the enclosing if
const yScale = scales.yScales.get(getSpecDomainGroupId(spec));
if (!yScale) {
return acc;
}

// check if the pointer is on the geometry (avoid checking if using external pointer event)
let isHighlighted = false;
if (
(!externalPointerEvent || isPointerOutEvent(externalPointerEvent)) &&
isPointOnGeometry(x, y, indexedGeometry, settings.pointBuffer)
) {
isHighlighted = true;
highlightedGeometries.push(indexedGeometry);
}

// if it's a follow tooltip, and no element is highlighted
// do _not_ add element into tooltip list
if (!isHighlighted && isFollowTooltipType(tooltipType)) {
return acc;
}

// format the tooltip values
const yAxisFormatSpec = [0, 180].includes(chartRotation) ? yAxis : xAxis;
const formattedTooltip = formatTooltip(
indexedGeometry,
spec,
false,
isHighlighted,
hasSingleSeries,
yAxisFormatSpec,
);

// format only one time the x value
if (!header) {
// if we have a tooltipHeaderFormatter, then don't pass in the xAxis as the user will define a formatter
const xAxisFormatSpec = [0, 180].includes(chartRotation) ? xAxis : yAxis;
const formatterAxis = tooltipHeaderFormatter ? undefined : xAxisFormatSpec;
header = formatTooltip(indexedGeometry, spec, true, false, hasSingleSeries, formatterAxis);
}

xValues.add(indexedGeometry.value.x);

const values = matchingGeoms
.filter(({ value: { y } }) => y !== null)
.reduce<TooltipValue[]>((acc, indexedGeometry) => {
const {
seriesIdentifier: { specId },
} = indexedGeometry;
const spec = getSpecsById<BasicSeriesSpec>(seriesSpecs, specId);

// safe guard check
if (!spec) {
return acc;
}
const { xAxis, yAxis } = getAxesSpecForSpecId(axesSpecs, spec.groupId);

// yScales is ensured by the enclosing if
const yScale = scales.yScales.get(getSpecDomainGroupId(spec));
if (!yScale) {
return acc;
}

// check if the pointer is on the geometry (avoid checking if using external pointer event)
let isHighlighted = false;
if (
(!externalPointerEvent || isPointerOutEvent(externalPointerEvent)) &&
isPointOnGeometry(x, y, indexedGeometry, settings.pointBuffer)
) {
isHighlighted = true;
highlightedGeometries.push(indexedGeometry);
}

// if it's a follow tooltip, and no element is highlighted
// do _not_ add element into tooltip list
if (!isHighlighted && isFollowTooltipType(tooltipType)) {
return acc;
}

// format the tooltip values
const yAxisFormatSpec = [0, 180].includes(chartRotation) ? yAxis : xAxis;
const formattedTooltip = formatTooltip(
indexedGeometry,
spec,
false,
isHighlighted,
hasSingleSeries,
yAxisFormatSpec,
);

// format only one time the x value
if (!header) {
// if we have a tooltipHeaderFormatter, then don't pass in the xAxis as the user will define a formatter
const xAxisFormatSpec = [0, 180].includes(chartRotation) ? xAxis : yAxis;
const formatterAxis = tooltipHeaderFormatter ? undefined : xAxisFormatSpec;
header = formatTooltip(indexedGeometry, spec, true, false, hasSingleSeries, formatterAxis);
}

xValues.add(indexedGeometry.value.x);

return [...acc, formattedTooltip];
}, []);
return [...acc, formattedTooltip];
}, []);

if (values.length > 1 && xValues.size === values.length) {
// TODO: remove after tooltip redesign
Expand Down
15 changes: 11 additions & 4 deletions packages/charts/src/specs/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,16 @@ export const DEFAULT_LEGEND_CONFIG = {
legendPosition: Position.Right,
};

/**
* Default tooltip config
* @internal
*/
export const DEFAULT_TOOLTIP_CONFIG = {
type: DEFAULT_TOOLTIP_TYPE,
snap: DEFAULT_TOOLTIP_SNAP,
showNullValues: false,
};

/** @public */
export const DEFAULT_SETTINGS_SPEC: SettingsSpec = {
id: '__global__settings___',
Expand All @@ -159,10 +169,7 @@ export const DEFAULT_SETTINGS_SPEC: SettingsSpec = {
animateData: true,
resizeDebounce: 10,
debug: false,
tooltip: {
type: DEFAULT_TOOLTIP_TYPE,
snap: DEFAULT_TOOLTIP_SNAP,
},
tooltip: DEFAULT_TOOLTIP_CONFIG,
pointerUpdateTrigger: PointerUpdateTrigger.X,
externalPointerEvents: {
tooltip: {
Expand Down
24 changes: 24 additions & 0 deletions packages/charts/src/specs/settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
BinAgg,
BrushAxis,
DEFAULT_SETTINGS_SPEC,
DEFAULT_TOOLTIP_CONFIG,
Direction,
PointerEventType,
PointerUpdateTrigger,
Expand Down Expand Up @@ -277,6 +278,12 @@ export type TooltipProps = TooltipPortalSettings<'chart'> & {
* @defaultValue mousePosition
*/
stickTo?: TooltipStickTo;

/**
* Show null values on the tooltip
* @defaultValue false
*/
showNullValues?: boolean;
};

/**
Expand Down Expand Up @@ -733,6 +740,23 @@ export function getTooltipType(settings: SettingsSpec, externalTooltip = false):
return defaultType;
}

/**
* Return snowNullValues or the default
* @internal
*/
export function getShowNullValues(settings: SettingsSpec): TooltipProps['showNullValues'] {
const { tooltip } = settings;
if (tooltip === undefined || tooltip === null) {
return DEFAULT_TOOLTIP_CONFIG.showNullValues;
}
if (isTooltipType(tooltip)) {
return DEFAULT_TOOLTIP_CONFIG.showNullValues;
}
if (isTooltipProps(tooltip)) {
return tooltip.showNullValues ?? DEFAULT_TOOLTIP_CONFIG.showNullValues;
}
}

/**
* Always return a Vertical Cursor for external pointer events or None if hidden
* @internal
Expand Down
Loading

0 comments on commit 20b81a9

Please sign in to comment.