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

refactor: decouple tooltip from XY chart #553

Merged
merged 18 commits into from
Feb 20, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
8 changes: 0 additions & 8 deletions src/chart_types/xy_chart/rendering/rendering.ts
Original file line number Diff line number Diff line change
Expand Up @@ -564,11 +564,3 @@ export function isPointOnGeometry(
const { width, height } = indexedGeometry;
return yCoordinate >= y && yCoordinate <= y + height && xCoordinate >= x && xCoordinate <= x + width;
}

export function getSeriesIdentifierPrefixedKey(
seriesIdentifier: XYChartSeriesIdentifier,
prefix?: string,
postfix?: string,
) {
return `${prefix || ''}${seriesIdentifier.key}${postfix || ''}`;
}
54 changes: 36 additions & 18 deletions src/chart_types/xy_chart/state/chart_state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -772,12 +772,15 @@ describe.skip('Chart Store', () => {

test.skip('can update the tooltip visibility', () => {
const tooltipValue: TooltipValue = {
name: 'a',
label: 'a',
value: 'a',
color: 'a',
isHighlighted: false,
seriesKey: 'a',
yAccessor: 'y',
seriesIdentifier: {
specId: 'a',
key: 'a',
},
valueAccessor: 'y',
isVisible: true,
};
store.cursorPosition.x = -1;
Expand Down Expand Up @@ -885,12 +888,15 @@ describe.skip('Chart Store', () => {
store.addSeriesSpec(spec);
store.setOnBrushEndListener(() => ({}));
const tooltipValue: TooltipValue = {
name: 'a',
label: 'a',
value: 'a',
color: 'a',
isHighlighted: false,
seriesKey: 'a',
yAccessor: 'y',
seriesIdentifier: {
specId: 'a',
key: 'a',
},
valueAccessor: 'y',
isVisible: true,
};
store.xScale = new ScaleContinuous({ type: ScaleType.Linear, domain: [0, 100], range: [0, 100] });
Expand Down Expand Up @@ -1028,21 +1034,27 @@ describe.skip('Chart Store', () => {
store.annotationDimensions.set(rectAnnotationSpec.id, annotationDimensions);

const highlightedTooltipValue: TooltipValue = {
name: 'foo',
label: 'foo',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we pick name or label to be consistent? I am having trouble choosing between the two. If we are going to have a name prop where this value is derived from, this should be name not label right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean the name prop of the Series component?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, this values is determined from the customSeriesLabel, soon to be name prop.

#539 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok let me switch to name then

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from our yesterday call, we will keep the label naming to make the tooltip a bit more abstract from the series. The Label should refer to the descriptive part of the value

value: 1,
color: 'color',
isHighlighted: true,
seriesKey: 'foo',
yAccessor: 'y',
seriesIdentifier: {
specId: 'a',
key: 'a',
},
valueAccessor: 'y',
isVisible: true,
};
const unhighlightedTooltipValue: TooltipValue = {
name: 'foo',
label: 'foo',
value: 1,
color: 'color',
isHighlighted: false,
seriesKey: 'foo',
yAccessor: 'y',
seriesIdentifier: {
specId: 'foo',
key: 'foo',
},
valueAccessor: 'y',
isVisible: true,
};

Expand All @@ -1066,26 +1078,32 @@ describe.skip('Chart Store', () => {
expect(store.legendItemTooltipValues.get()).toEqual(new Map());

const headerValue: TooltipValue = {
name: 'header',
label: 'header',
value: 'foo',
color: 'a',
isHighlighted: false,
seriesKey: 'headerSeries',
seriesIdentifier: {
specId: 'headerSeries',
key: 'headerSeries',
},
valueAccessor: BandedAccessorType.Y0,
isVisible: true,
yAccessor: BandedAccessorType.Y0,
};

store.tooltipData.replace([headerValue]);
expect(store.legendItemTooltipValues.get()).toEqual(new Map());

const tooltipValue: TooltipValue = {
name: 'a',
label: 'a',
value: 123,
color: 'a',
isHighlighted: false,
seriesKey: 'seriesKeys',
seriesIdentifier: {
specId: 'seriesKeys',
key: 'seriesKeys',
},
valueAccessor: BandedAccessorType.Y1,
isVisible: true,
yAccessor: BandedAccessorType.Y1,
};
store.tooltipData.replace([headerValue, tooltipValue]);

Expand Down
32 changes: 16 additions & 16 deletions src/chart_types/xy_chart/tooltip/tooltip.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,24 +100,24 @@ describe('Tooltip formatting', () => {
test('format simple tooltip', () => {
const tooltipValue = formatTooltip(indexedGeometry, SPEC_1, false, false, false, YAXIS_SPEC);
expect(tooltipValue).toBeDefined();
expect(tooltipValue.yAccessor).toBe('y1');
expect(tooltipValue.name).toBe('bar_1');
expect(tooltipValue.valueAccessor).toBe('y1');
expect(tooltipValue.label).toBe('bar_1');
expect(tooltipValue.isHighlighted).toBe(false);
expect(tooltipValue.color).toBe('blue');
expect(tooltipValue.value).toBe('10');
});
it('should set name as spec name when provided', () => {
const name = 'test - spec';
const tooltipValue = formatTooltip(indexedBandedGeometry, { ...SPEC_1, name }, false, false, false, YAXIS_SPEC);
expect(tooltipValue.name).toBe(name);
expect(tooltipValue.label).toBe(name);
});
it('should set name as spec id when name is not provided', () => {
const tooltipValue = formatTooltip(indexedBandedGeometry, SPEC_1, false, false, false, YAXIS_SPEC);
expect(tooltipValue.name).toBe(SPEC_1.id);
expect(tooltipValue.label).toBe(SPEC_1.id);
});
test('format banded tooltip - upper', () => {
const tooltipValue = formatTooltip(indexedBandedGeometry, bandedSpec, false, false, false, YAXIS_SPEC);
expect(tooltipValue.name).toBe('bar_1 - upper');
expect(tooltipValue.label).toBe('bar_1 - upper');
});
test('format banded tooltip - y1AccessorFormat', () => {
const tooltipValue = formatTooltip(
Expand All @@ -128,7 +128,7 @@ describe('Tooltip formatting', () => {
false,
YAXIS_SPEC,
);
expect(tooltipValue.name).toBe('bar_1 [max]');
expect(tooltipValue.label).toBe('bar_1 [max]');
});
test('format banded tooltip - y1AccessorFormat as function', () => {
const tooltipValue = formatTooltip(
Expand All @@ -139,7 +139,7 @@ describe('Tooltip formatting', () => {
false,
YAXIS_SPEC,
);
expect(tooltipValue.name).toBe('[max] bar_1');
expect(tooltipValue.label).toBe('[max] bar_1');
});
test('format banded tooltip - lower', () => {
const tooltipValue = formatTooltip(
Expand All @@ -156,7 +156,7 @@ describe('Tooltip formatting', () => {
false,
YAXIS_SPEC,
);
expect(tooltipValue.name).toBe('bar_1 - lower');
expect(tooltipValue.label).toBe('bar_1 - lower');
});
test('format banded tooltip - y0AccessorFormat', () => {
const tooltipValue = formatTooltip(
Expand All @@ -173,7 +173,7 @@ describe('Tooltip formatting', () => {
false,
YAXIS_SPEC,
);
expect(tooltipValue.name).toBe('bar_1 [min]');
expect(tooltipValue.label).toBe('bar_1 [min]');
});
test('format banded tooltip - y0AccessorFormat as function', () => {
const tooltipValue = formatTooltip(
Expand All @@ -190,7 +190,7 @@ describe('Tooltip formatting', () => {
false,
YAXIS_SPEC,
);
expect(tooltipValue.name).toBe('[min] bar_1');
expect(tooltipValue.label).toBe('[min] bar_1');
});
test('format tooltip with seriesKeys name', () => {
const geometry: BarGeometry = {
Expand All @@ -205,8 +205,8 @@ describe('Tooltip formatting', () => {
};
const tooltipValue = formatTooltip(geometry, SPEC_1, false, false, false, YAXIS_SPEC);
expect(tooltipValue).toBeDefined();
expect(tooltipValue.yAccessor).toBe('y1');
expect(tooltipValue.name).toBe('bar_1');
expect(tooltipValue.valueAccessor).toBe('y1');
expect(tooltipValue.label).toBe('bar_1');
expect(tooltipValue.isHighlighted).toBe(false);
expect(tooltipValue.color).toBe('blue');
expect(tooltipValue.value).toBe('10');
Expand All @@ -221,8 +221,8 @@ describe('Tooltip formatting', () => {
};
const tooltipValue = formatTooltip(geometry, SPEC_1, false, false, false, YAXIS_SPEC);
expect(tooltipValue).toBeDefined();
expect(tooltipValue.yAccessor).toBe('y0');
expect(tooltipValue.name).toBe('bar_1');
expect(tooltipValue.valueAccessor).toBe('y0');
expect(tooltipValue.label).toBe('bar_1');
expect(tooltipValue.isHighlighted).toBe(false);
expect(tooltipValue.color).toBe('blue');
expect(tooltipValue.value).toBe('10');
Expand All @@ -237,8 +237,8 @@ describe('Tooltip formatting', () => {
};
let tooltipValue = formatTooltip(geometry, SPEC_1, true, false, false, YAXIS_SPEC);
expect(tooltipValue).toBeDefined();
expect(tooltipValue.yAccessor).toBe('y0');
expect(tooltipValue.name).toBe('bar_1');
expect(tooltipValue.valueAccessor).toBe('y0');
expect(tooltipValue.label).toBe('bar_1');
expect(tooltipValue.isHighlighted).toBe(false);
expect(tooltipValue.color).toBe('blue');
expect(tooltipValue.value).toBe('1');
Expand Down
23 changes: 11 additions & 12 deletions src/chart_types/xy_chart/tooltip/tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
} from '../utils/specs';
import { IndexedGeometry, BandedAccessorType } from '../../../utils/geometry';
import { getAccessorFormatLabel } from '../../../utils/accessor';
import { getSeriesKey, getSeriesLabel } from '../utils/series';
import { getSeriesLabel } from '../utils/series';

export interface TooltipLegendValue {
y0: any;
Expand All @@ -26,15 +26,15 @@ export function getSeriesTooltipValues(
// map from seriesKey to TooltipLegendValue
const seriesTooltipValues = new Map<string, TooltipLegendValue>();

tooltipValues.forEach(({ seriesKey, value, yAccessor }) => {
tooltipValues.forEach(({ value, seriesIdentifier, valueAccessor }) => {
const seriesValue = defaultValue ? defaultValue : value;
const current = seriesTooltipValues.get(seriesKey) || {};
const current = seriesTooltipValues.get(seriesIdentifier.key) || {};

seriesTooltipValues.set(seriesKey, {
seriesTooltipValues.set(seriesIdentifier.key, {
y0: defaultValue,
y1: defaultValue,
...current,
[yAccessor]: seriesValue,
[valueAccessor]: seriesValue,
});
});
return seriesTooltipValues;
Expand All @@ -48,26 +48,25 @@ export function formatTooltip(
hasSingleSeries: boolean,
axisSpec?: AxisSpec,
): TooltipValue {
const seriesKey = getSeriesKey(seriesIdentifier);
let displayName = getSeriesLabel(seriesIdentifier, hasSingleSeries, true, spec);
let label = getSeriesLabel(seriesIdentifier, hasSingleSeries, true, spec);

if (isBandedSpec(spec.y0Accessors) && (isAreaSeriesSpec(spec) || isBarSeriesSpec(spec))) {
const { y0AccessorFormat = Y0_ACCESSOR_POSTFIX, y1AccessorFormat = Y1_ACCESSOR_POSTFIX } = spec;
const formatter = accessor === BandedAccessorType.Y0 ? y0AccessorFormat : y1AccessorFormat;
displayName = getAccessorFormatLabel(formatter, displayName);
label = getAccessorFormatLabel(formatter, label);
}
const isFiltered = spec.filterSeriesInTooltip !== undefined ? spec.filterSeriesInTooltip(seriesIdentifier) : true;
const isVisible = displayName === '' ? false : isFiltered;
const isVisible = label === '' ? false : isFiltered;

const value = isHeader ? x : y;
const tickFormatOptions: TickFormatterOptions | undefined = spec.timeZone ? { timeZone: spec.timeZone } : undefined;
return {
seriesKey,
name: displayName,
seriesIdentifier,
valueAccessor: accessor,
label,
value: axisSpec ? axisSpec.tickFormat(value, tickFormatOptions) : emptyFormatter(value),
color,
isHighlighted: isHeader ? false : isHighlighted,
yAccessor: accessor,
isVisible,
};
}
Expand Down
30 changes: 26 additions & 4 deletions src/chart_types/xy_chart/utils/interactions.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { $Values } from 'utility-types';
import { Rotation } from '../../../utils/commons';
import { Dimensions } from '../../../utils/dimensions';
import { Accessor } from '../../../utils/accessor';
import { BarGeometry, PointGeometry, IndexedGeometry, isPointGeometry, isBarGeometry } from '../../../utils/geometry';
import { TooltipProps } from '../../../specs';
import { SeriesIdentifier } from './series';
import { Accessor } from '../../../utils/accessor';

/** The type of tooltip to use */
export const TooltipType = Object.freeze({
Expand All @@ -20,13 +21,34 @@ export const TooltipType = Object.freeze({
export type TooltipType = $Values<typeof TooltipType>;

export interface TooltipValue {
name: string;
/**
* The label of the tooltip value
*/
label: string;
/**
* The value to display
*/
value: any;
/**
* The color of the graphic mark (by default the color of the series)
*/
color: string;
/**
* True if the mouse is over the graphic mark connected to the tooltip
*/
isHighlighted: boolean;
seriesKey: string;
yAccessor: Accessor;
/**
* True if the tooltip is visible, false otherwise
*/
isVisible: boolean;
/**
* The idenfitier of the related series
*/
seriesIdentifier: SeriesIdentifier;
/**
* The accessor linked to the current tooltip value
*/
valueAccessor: Accessor;
}

export type TooltipValueFormatter = (data: TooltipValue) => JSX.Element | string;
Expand Down
44 changes: 23 additions & 21 deletions src/components/tooltip/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,27 +100,29 @@ class TooltipComponent extends React.Component<TooltipProps> {
<div className="echTooltip" ref={this.tooltipRef}>
<div className="echTooltip__header">{this.renderHeader(tooltip.header, tooltipHeaderFormatter)}</div>
<div className="echTooltip__list">
{tooltip.values.map(({ name, value, color, isHighlighted, seriesKey, yAccessor, isVisible }) => {
if (!isVisible) {
return null;
}
const classes = classNames('echTooltip__item', {
/* eslint @typescript-eslint/camelcase:0 */
echTooltip__rowHighlighted: isHighlighted,
});
return (
<div
key={`${seriesKey}--${yAccessor}`}
className={classes}
style={{
borderLeftColor: color,
}}
>
<span className="echTooltip__label">{name}</span>
<span className="echTooltip__value">{value}</span>
</div>
);
})}
{tooltip.values.map(
({ seriesIdentifier, valueAccessor, label, value, color, isHighlighted, isVisible }) => {
if (!isVisible) {
return null;
}
const classes = classNames('echTooltip__item', {
/* eslint @typescript-eslint/camelcase:0 */
echTooltip__rowHighlighted: isHighlighted,
});
return (
<div
key={`${seriesIdentifier.key}__${valueAccessor}`}
className={classes}
style={{
borderLeftColor: color,
}}
>
<span className="echTooltip__label">{label}</span>
<span className="echTooltip__value">{value}</span>
</div>
);
},
)}
</div>
</div>
);
Expand Down