From e8814e1db0daf44602668f09792dfdd20e58d47e Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Thu, 13 Feb 2020 18:13:32 +0100 Subject: [PATCH 01/17] chore: remove unused interface --- src/chart_types/xy_chart/utils/interactions.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/chart_types/xy_chart/utils/interactions.ts b/src/chart_types/xy_chart/utils/interactions.ts index 73dff95d9a..c82ebda883 100644 --- a/src/chart_types/xy_chart/utils/interactions.ts +++ b/src/chart_types/xy_chart/utils/interactions.ts @@ -37,16 +37,6 @@ export interface TooltipProps { export type TooltipValueFormatter = (data: TooltipValue) => JSX.Element | string; -export interface HighlightedElement { - position: { - x: number; - y: number; - width: number; - height: number; - type: 'rect' | 'circle'; - }; - value: Datum; -} /** * Get the cursor position depending on the chart rotation * @param xPos x position relative to chart From f3680e37360af87728d703b202559b59b239c9ac Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Thu, 13 Feb 2020 18:32:45 +0100 Subject: [PATCH 02/17] chore: remove unused interfaces --- src/utils/domain.ts | 28 +--------------------------- 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/src/utils/domain.ts b/src/utils/domain.ts index 46cbfe43bd..22d9dbdd29 100644 --- a/src/utils/domain.ts +++ b/src/utils/domain.ts @@ -1,35 +1,9 @@ import { extent, sum } from 'd3-array'; import { nest } from 'd3-collection'; -import { Accessor, AccessorFn } from './accessor'; -import { ScaleType } from '../scales'; +import { AccessorFn } from './accessor'; export type Domain = any[]; -export interface SpecDomain { - accessor: Accessor; - level: number; - domain: Domain; - scaleType: ScaleType; - isStacked?: boolean; -} - -export interface ColorDomain { - accessors: Accessor[]; - yAccessors?: Accessor[]; - domain: string[]; - scaleType: ScaleType; -} - -export interface SeriesScales { - groupLevel: number; - xDomain: Domain; - yDomain?: Domain; - xScaleType: ScaleType; - yScaleType?: ScaleType; - xAccessor: Accessor; - yAccessor?: Accessor; -} - export function computeOrdinalDataDomain( data: any[], accessor: AccessorFn, From 4344c738f8bb823a36fec329a26e30e98c38ba13 Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Fri, 14 Feb 2020 15:12:55 +0100 Subject: [PATCH 03/17] chore: move tooltips component to global folder --- .../xy_chart/state/chart_state.tsx | 4 +-- src/components/_index.scss | 2 +- src/components/tooltip/_index.scss | 1 + src/components/{ => tooltip}/_tooltip.scss | 0 .../tooltip/index.tsx} | 25 +++++++++++-------- 5 files changed, 18 insertions(+), 14 deletions(-) create mode 100644 src/components/tooltip/_index.scss rename src/components/{ => tooltip}/_tooltip.scss (100%) rename src/{chart_types/xy_chart/renderer/dom/tooltips.tsx => components/tooltip/index.tsx} (81%) diff --git a/src/chart_types/xy_chart/state/chart_state.tsx b/src/chart_types/xy_chart/state/chart_state.tsx index 83932a45ae..ef88a90609 100644 --- a/src/chart_types/xy_chart/state/chart_state.tsx +++ b/src/chart_types/xy_chart/state/chart_state.tsx @@ -1,7 +1,7 @@ import React, { RefObject } from 'react'; import { InternalChartState, GlobalChartState, BackwardRef } from '../../../state/chart_state'; import { ChartTypes } from '../..'; -import { Tooltips } from '../renderer/dom/tooltips'; +import { Tooltip } from '../../../components/tooltip'; import { htmlIdGenerator } from '../../../utils/commons'; import { Highlighter } from '../renderer/dom/highlighter'; import { Crosshair } from '../renderer/dom/crosshair'; @@ -39,7 +39,7 @@ export class XYAxisChartState implements InternalChartState { - + diff --git a/src/components/_index.scss b/src/components/_index.scss index bae81d02d7..7c487177a2 100644 --- a/src/components/_index.scss +++ b/src/components/_index.scss @@ -1,7 +1,7 @@ @import 'global'; @import 'container'; @import 'annotation'; -@import 'tooltip'; +@import 'tooltip/index'; @import 'icons/index'; @import 'legend/index'; @import 'unavailable_chart'; diff --git a/src/components/tooltip/_index.scss b/src/components/tooltip/_index.scss new file mode 100644 index 0000000000..0a06e6f1bb --- /dev/null +++ b/src/components/tooltip/_index.scss @@ -0,0 +1 @@ +@import 'tooltip'; diff --git a/src/components/_tooltip.scss b/src/components/tooltip/_tooltip.scss similarity index 100% rename from src/components/_tooltip.scss rename to src/components/tooltip/_tooltip.scss diff --git a/src/chart_types/xy_chart/renderer/dom/tooltips.tsx b/src/components/tooltip/index.tsx similarity index 81% rename from src/chart_types/xy_chart/renderer/dom/tooltips.tsx rename to src/components/tooltip/index.tsx index 8d3d14b200..ad444622e9 100644 --- a/src/chart_types/xy_chart/renderer/dom/tooltips.tsx +++ b/src/components/tooltip/index.tsx @@ -1,16 +1,19 @@ import classNames from 'classnames'; import React from 'react'; import { connect } from 'react-redux'; -import { TooltipValue, TooltipValueFormatter } from '../../utils/interactions'; -import { GlobalChartState, BackwardRef } from '../../../../state/chart_state'; -import { isTooltipVisibleSelector } from '../../state/selectors/is_tooltip_visible'; -import { getTooltipHeaderFormatterSelector } from '../../state/selectors/get_tooltip_header_formatter'; -import { getTooltipPositionSelector } from '../../state/selectors/get_tooltip_position'; -import { getTooltipValuesSelector, TooltipData } from '../../state/selectors/get_tooltip_values_highlighted_geoms'; -import { isInitialized } from '../../../../state/selectors/is_initialized'; +import { TooltipValue, TooltipValueFormatter } from '../../chart_types/xy_chart/utils/interactions'; +import { GlobalChartState, BackwardRef } from '../../state/chart_state'; +import { isTooltipVisibleSelector } from '../../chart_types/xy_chart/state/selectors/is_tooltip_visible'; +import { getTooltipHeaderFormatterSelector } from '../../chart_types/xy_chart/state/selectors/get_tooltip_header_formatter'; +import { getTooltipPositionSelector } from '../../chart_types/xy_chart/state/selectors/get_tooltip_position'; +import { + getTooltipValuesSelector, + TooltipData, +} from '../../chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms'; +import { isInitialized } from '../../state/selectors/is_initialized'; import { createPortal } from 'react-dom'; -import { getFinalTooltipPosition, TooltipPosition } from '../../crosshair/crosshair_utils'; -import { isAnnotationTooltipVisibleSelector } from '../../state/selectors/is_annotation_tooltip_visible'; +import { getFinalTooltipPosition, TooltipPosition } from '../../chart_types/xy_chart/crosshair/crosshair_utils'; +import { isAnnotationTooltipVisibleSelector } from '../../chart_types/xy_chart/state/selectors/is_annotation_tooltip_visible'; interface TooltipStateProps { isTooltipVisible: boolean; @@ -24,7 +27,7 @@ interface TooltipOwnProps { } type TooltipProps = TooltipStateProps & TooltipOwnProps; -class TooltipsComponent extends React.Component { +class TooltipComponent extends React.Component { static displayName = 'Tooltips'; portalNode: HTMLDivElement | null = null; tooltipRef: React.RefObject; @@ -148,4 +151,4 @@ const mapStateToProps = (state: GlobalChartState): TooltipStateProps => { }; }; -export const Tooltips = connect(mapStateToProps)(TooltipsComponent); +export const Tooltip = connect(mapStateToProps)(TooltipComponent); From f56a0d335897b1a6f3c00d4382001cca8d192d4a Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Fri, 14 Feb 2020 15:29:45 +0100 Subject: [PATCH 04/17] chore: remove unused TooltipProps type --- src/chart_types/xy_chart/utils/interactions.ts | 9 ++------- src/specs/settings.tsx | 2 +- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/chart_types/xy_chart/utils/interactions.ts b/src/chart_types/xy_chart/utils/interactions.ts index c82ebda883..cae47553f0 100644 --- a/src/chart_types/xy_chart/utils/interactions.ts +++ b/src/chart_types/xy_chart/utils/interactions.ts @@ -1,8 +1,9 @@ import { $Values } from 'utility-types'; -import { Datum, Rotation } from '../../../utils/commons'; +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'; /** The type of tooltip to use */ export const TooltipType = Object.freeze({ @@ -29,12 +30,6 @@ export interface TooltipValue { isVisible: boolean; } -export interface TooltipProps { - type?: TooltipType; - snap?: boolean; - headerFormatter?: TooltipValueFormatter; -} - export type TooltipValueFormatter = (data: TooltipValue) => JSX.Element | string; /** diff --git a/src/specs/settings.tsx b/src/specs/settings.tsx index 388f4ed166..f90920df4b 100644 --- a/src/specs/settings.tsx +++ b/src/specs/settings.tsx @@ -57,7 +57,7 @@ export interface PointerOutEvent extends BasePointerEvent { export type PointerEvent = PointerOverEvent | PointerOutEvent; -interface TooltipProps { +export interface TooltipProps { type?: TooltipType; snap?: boolean; headerFormatter?: TooltipValueFormatter; From 58def8863be90696184549f98b86d3c5dff4dbb1 Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Fri, 14 Feb 2020 16:03:34 +0100 Subject: [PATCH 05/17] refactor: generalize SeriesIdentifier BREAKING CHANGE: the `SeriesIdentifier` type is generalized into a simple object with two common values: specId and key. A specialized `XYChartSeriesIdentifier` extends now the base `SeriesIdentifier` --- docs/0-Intro/1-Overview.mdx | 4 +-- src/chart_types/xy_chart/legend/legend.ts | 6 ++--- .../xy_chart/rendering/rendering.test.ts | 8 +++--- .../xy_chart/rendering/rendering.ts | 18 ++++++++----- .../selectors/on_element_click_caller.ts | 4 +-- .../state/selectors/on_element_over_caller.ts | 4 +-- src/chart_types/xy_chart/state/utils.ts | 13 ++++++---- src/chart_types/xy_chart/utils/series.test.ts | 4 +-- src/chart_types/xy_chart/utils/series.ts | 26 ++++++++++--------- src/chart_types/xy_chart/utils/specs.ts | 20 +++++++++----- src/components/legend/legend_item.tsx | 6 ++--- src/index.ts | 2 +- src/mocks/series/seriesIdentifiers.ts | 14 +++++++--- src/specs/settings.tsx | 8 +++--- src/state/actions/legend.ts | 6 ++--- src/state/chart_state.ts | 4 +-- src/state/reducers/interactions.ts | 7 +++-- src/utils/geometry.ts | 12 ++++----- wiki/overview.md | 4 +-- 19 files changed, 97 insertions(+), 73 deletions(-) diff --git a/docs/0-Intro/1-Overview.mdx b/docs/0-Intro/1-Overview.mdx index 82c5b0e6fd..62e4be3f14 100644 --- a/docs/0-Intro/1-Overview.mdx +++ b/docs/0-Intro/1-Overview.mdx @@ -233,7 +233,7 @@ Return types: ```ts type BarStyleAccessor = ( datum: RawDataSeriesDatum, - seriesIdentifier: SeriesIdentifier, + seriesIdentifier: XYChartSeriesIdentifier, ) => RecursivePartial | Color | null; ``` @@ -250,7 +250,7 @@ Return types: ```ts type PointStyleAccessor = ( datum: RawDataSeriesDatum, - seriesIdentifier: SeriesIdentifier, + seriesIdentifier: XYChartSeriesIdentifier, ) => RecursivePartial | Color | null; ``` diff --git a/src/chart_types/xy_chart/legend/legend.ts b/src/chart_types/xy_chart/legend/legend.ts index 42f4df4cf8..324a00edc6 100644 --- a/src/chart_types/xy_chart/legend/legend.ts +++ b/src/chart_types/xy_chart/legend/legend.ts @@ -4,8 +4,8 @@ import { SeriesCollectionValue, getSeriesIndex, getSortedDataSeriesColorsValuesMap, - SeriesIdentifier, getSeriesLabel, + XYChartSeriesIdentifier, } from '../utils/series'; import { AxisSpec, BasicSeriesSpec, Postfixes, isAreaSeriesSpec, isBarSeriesSpec } from '../utils/specs'; import { Y0_ACCESSOR_POSTFIX, Y1_ACCESSOR_POSTFIX } from '../tooltip/tooltip'; @@ -20,7 +20,7 @@ export type LegendItem = Postfixes & { key: string; color: string; label: string; - seriesIdentifier: SeriesIdentifier; + seriesIdentifier: XYChartSeriesIdentifier; isSeriesVisible?: boolean; banded?: boolean; isLegendItemVisible?: boolean; @@ -59,7 +59,7 @@ export function computeLegend( specs: BasicSeriesSpec[], defaultColor: string, axesSpecs: AxisSpec[], - deselectedDataSeries: SeriesIdentifier[] = [], + deselectedDataSeries: XYChartSeriesIdentifier[] = [], ): Map { const legendItems: Map = new Map(); const sortedCollection = getSortedDataSeriesColorsValuesMap(seriesCollection); diff --git a/src/chart_types/xy_chart/rendering/rendering.test.ts b/src/chart_types/xy_chart/rendering/rendering.test.ts index 3cd4abc0ef..4586163712 100644 --- a/src/chart_types/xy_chart/rendering/rendering.test.ts +++ b/src/chart_types/xy_chart/rendering/rendering.test.ts @@ -6,7 +6,7 @@ import { getClippedRanges, } from './rendering'; import { BarSeriesStyle, SharedGeometryStateStyle, PointStyle } from '../../../utils/themes/theme'; -import { DataSeriesDatum, SeriesIdentifier } from '../utils/series'; +import { DataSeriesDatum, XYChartSeriesIdentifier } from '../utils/series'; import { mergePartial, RecursivePartial } from '../../../utils/commons'; import { BarGeometry, PointGeometry } from '../../../utils/geometry'; import { MockDataSeries } from '../../../mocks'; @@ -95,7 +95,7 @@ describe('Rendering utils', () => { }); describe('should get common geometry style dependent on legend item highlight state', () => { - const seriesIdentifier: SeriesIdentifier = { + const seriesIdentifier: XYChartSeriesIdentifier = { specId: 'id', yAccessor: 'y1', splitAccessors: new Map(), @@ -222,7 +222,7 @@ describe('Rendering utils', () => { initialY1: 4, initialY0: 5, }; - const seriesIdentifier: SeriesIdentifier = { + const seriesIdentifier: XYChartSeriesIdentifier = { specId: 'test', yAccessor: 'test', splitAccessors: new Map(), @@ -314,7 +314,7 @@ describe('Rendering utils', () => { initialY1: 4, initialY0: 5, }; - const seriesIdentifier: SeriesIdentifier = { + const seriesIdentifier: XYChartSeriesIdentifier = { specId: 'test', yAccessor: 'test', splitAccessors: new Map(), diff --git a/src/chart_types/xy_chart/rendering/rendering.ts b/src/chart_types/xy_chart/rendering/rendering.ts index 3d006d0937..c4c450e733 100644 --- a/src/chart_types/xy_chart/rendering/rendering.ts +++ b/src/chart_types/xy_chart/rendering/rendering.ts @@ -10,7 +10,7 @@ import { } from '../../../utils/themes/theme'; import { Scale, ScaleType, isLogarithmicScale } from '../../../scales'; import { CurveType, getCurveFactory } from '../../../utils/curves'; -import { DataSeriesDatum, SeriesIdentifier, DataSeries } from '../utils/series'; +import { DataSeriesDatum, DataSeries, XYChartSeriesIdentifier } from '../utils/series'; import { DisplayValueSpec, PointStyleAccessor, BarStyleAccessor } from '../utils/specs'; import { IndexedGeometry, @@ -41,7 +41,7 @@ export function mutableIndexedGeometryMapUpsert( export function getPointStyleOverrides( datum: DataSeriesDatum, - seriesIdentifier: SeriesIdentifier, + seriesIdentifier: XYChartSeriesIdentifier, pointStyleAccessor?: PointStyleAccessor, ): Partial | undefined { const styleOverride = pointStyleAccessor && pointStyleAccessor(datum, seriesIdentifier); @@ -61,7 +61,7 @@ export function getPointStyleOverrides( export function getBarStyleOverrides( datum: DataSeriesDatum, - seriesIdentifier: SeriesIdentifier, + seriesIdentifier: XYChartSeriesIdentifier, seriesStyle: BarSeriesStyle, styleAccessor?: BarStyleAccessor, ): BarSeriesStyle { @@ -125,7 +125,7 @@ function renderPoints( y = yScale.scale(yDatum); } const originalY = hasY0Accessors && index === 0 ? initialY0 : initialY1; - const seriesIdentifier: SeriesIdentifier = { + const seriesIdentifier: XYChartSeriesIdentifier = { key: dataSeries.key, specId: dataSeries.specId, yAccessor: dataSeries.yAccessor, @@ -268,7 +268,7 @@ export function renderBars( } : undefined; - const seriesIdentifier: SeriesIdentifier = { + const seriesIdentifier: XYChartSeriesIdentifier = { key: dataSeries.key, specId: dataSeries.specId, yAccessor: dataSeries.yAccessor, @@ -522,7 +522,7 @@ export function getClippedRanges(dataset: DataSeriesDatum[], xScale: Scale, xSca } export function getGeometryStateStyle( - seriesIdentifier: SeriesIdentifier, + seriesIdentifier: XYChartSeriesIdentifier, highlightedLegendItem: LegendItem | null, sharedGeometryStyle: SharedGeometryStateStyle, individualHighlight?: { [key: string]: boolean }, @@ -565,6 +565,10 @@ export function isPointOnGeometry( return yCoordinate >= y && yCoordinate <= y + height && xCoordinate >= x && xCoordinate <= x + width; } -export function getSeriesIdentifierPrefixedKey(seriesIdentifier: SeriesIdentifier, prefix?: string, postfix?: string) { +export function getSeriesIdentifierPrefixedKey( + seriesIdentifier: XYChartSeriesIdentifier, + prefix?: string, + postfix?: string, +) { return `${prefix || ''}${seriesIdentifier.key}${postfix || ''}`; } diff --git a/src/chart_types/xy_chart/state/selectors/on_element_click_caller.ts b/src/chart_types/xy_chart/state/selectors/on_element_click_caller.ts index 89a84e8456..a190cbedae 100644 --- a/src/chart_types/xy_chart/state/selectors/on_element_click_caller.ts +++ b/src/chart_types/xy_chart/state/selectors/on_element_click_caller.ts @@ -6,7 +6,7 @@ import { getHighlightedGeomsSelector } from './get_tooltip_values_highlighted_ge import { SettingsSpec } from '../../../../specs'; import { IndexedGeometry, GeometryValue } from '../../../../utils/geometry'; import { ChartTypes } from '../../../index'; -import { SeriesIdentifier } from '../../utils/series'; +import { XYChartSeriesIdentifier } from '../../utils/series'; const getLastClickSelector = (state: GlobalChartState) => state.interactions.pointer.lastClick; @@ -57,7 +57,7 @@ export function createOnElementClickCaller(): (state: GlobalChartState) => void if (isClicking(prevProps, nextProps)) { if (settings && settings.onElementClick) { - const elements = indexedGeometries.map<[GeometryValue, SeriesIdentifier]>( + const elements = indexedGeometries.map<[GeometryValue, XYChartSeriesIdentifier]>( ({ value, seriesIdentifier }) => [value, seriesIdentifier], ); settings.onElementClick(elements); diff --git a/src/chart_types/xy_chart/state/selectors/on_element_over_caller.ts b/src/chart_types/xy_chart/state/selectors/on_element_over_caller.ts index 707fb2c972..496a0367f3 100644 --- a/src/chart_types/xy_chart/state/selectors/on_element_over_caller.ts +++ b/src/chart_types/xy_chart/state/selectors/on_element_over_caller.ts @@ -10,7 +10,7 @@ import { IndexedGeometry, GeometryValue } from '../../../../utils/geometry'; import { Selector } from 'react-redux'; import { ChartTypes } from '../../../index'; import { getChartIdSelector } from '../../../../state/selectors/get_chart_id'; -import { SeriesIdentifier } from '../../utils/series'; +import { XYChartSeriesIdentifier } from '../../utils/series'; interface Props { settings: SettingsSpec | undefined; @@ -58,7 +58,7 @@ export function createOnElementOverCaller(): (state: GlobalChartState) => void { }; if (isOverElement(prevProps, nextProps) && settings.onElementOver) { - const elements = highlightedGeometries.map<[GeometryValue, SeriesIdentifier]>( + const elements = highlightedGeometries.map<[GeometryValue, XYChartSeriesIdentifier]>( ({ value, seriesIdentifier }) => [value, seriesIdentifier], ); settings.onElementOver(elements); diff --git a/src/chart_types/xy_chart/state/utils.ts b/src/chart_types/xy_chart/state/utils.ts index 2bc7ab826e..1814bcbc1a 100644 --- a/src/chart_types/xy_chart/state/utils.ts +++ b/src/chart_types/xy_chart/state/utils.ts @@ -12,8 +12,8 @@ import { getFormattedDataseries, getSplittedSeries, getSeriesKey, - SeriesIdentifier, RawDataSeries, + XYChartSeriesIdentifier, } from '../utils/series'; import { AreaSeriesSpec, @@ -99,7 +99,10 @@ export interface SeriesDomainsAndData { * @param series * @param target */ -export function updateDeselectedDataSeries(series: SeriesIdentifier[], target: SeriesIdentifier): SeriesIdentifier[] { +export function updateDeselectedDataSeries( + series: XYChartSeriesIdentifier[], + target: XYChartSeriesIdentifier, +): XYChartSeriesIdentifier[] { const seriesIndex = getSeriesIndex(series, target); const updatedSeries = series ? [...series] : []; @@ -167,7 +170,7 @@ function getLastValues(formattedDataSeries: { // we need to get the latest formattedDataSeries.stacked.forEach((ds) => { ds.dataSeries.forEach((series) => { - const seriesKey = getSeriesKey(series as SeriesIdentifier); + const seriesKey = getSeriesKey(series as XYChartSeriesIdentifier); if (series.data.length > 0) { const last = series.data[series.data.length - 1]; if (last !== null) { @@ -182,7 +185,7 @@ function getLastValues(formattedDataSeries: { }); formattedDataSeries.nonStacked.forEach((ds) => { ds.dataSeries.forEach((series) => { - const seriesKey = getSeriesKey(series as SeriesIdentifier); + const seriesKey = getSeriesKey(series as XYChartSeriesIdentifier); if (series.data.length > 0) { const last = series.data[series.data.length - 1]; if (last !== null) { @@ -210,7 +213,7 @@ function getLastValues(formattedDataSeries: { export function computeSeriesDomains( seriesSpecs: BasicSeriesSpec[], customYDomainsByGroupId: Map = new Map(), - deselectedDataSeries: SeriesIdentifier[] = [], + deselectedDataSeries: XYChartSeriesIdentifier[] = [], customXDomain?: DomainRange | Domain, ): SeriesDomainsAndData { const { splittedSeries, xValues, seriesCollection } = deselectedDataSeries diff --git a/src/chart_types/xy_chart/utils/series.test.ts b/src/chart_types/xy_chart/utils/series.test.ts index b00f3aa8fc..a671414d68 100644 --- a/src/chart_types/xy_chart/utils/series.test.ts +++ b/src/chart_types/xy_chart/utils/series.test.ts @@ -8,7 +8,7 @@ import { getSplittedSeries, RawDataSeries, splitSeries, - SeriesIdentifier, + XYChartSeriesIdentifier, cleanDatum, } from './series'; import { BasicSeriesSpec, LineSeriesSpec, SeriesTypes } from './specs'; @@ -555,7 +555,7 @@ describe('Series', () => { const emptyDeselected = getSplittedSeries([splitSpec]); expect(emptyDeselected.splittedSeries.get(specId)!.length).toBe(2); - const deselectedDataSeries: SeriesIdentifier[] = [ + const deselectedDataSeries: XYChartSeriesIdentifier[] = [ { specId, yAccessor: splitSpec.yAccessors[0], diff --git a/src/chart_types/xy_chart/utils/series.ts b/src/chart_types/xy_chart/utils/series.ts index 40d9503d94..4c4dd068d0 100644 --- a/src/chart_types/xy_chart/utils/series.ts +++ b/src/chart_types/xy_chart/utils/series.ts @@ -45,21 +45,23 @@ export interface DataSeriesDatum { /** the list of filled values because missing or nulls */ filled?: FilledValues; } - -export interface SeriesIdentifier { +export type SeriesIdentifier = { specId: SpecId; + key: string; +}; + +export interface XYChartSeriesIdentifier extends SeriesIdentifier { yAccessor: string | number; splitAccessors: Map; // does the map have a size vs making it optional seriesKeys: (string | number)[]; - key: string; } -export type DataSeries = SeriesIdentifier & { +export type DataSeries = XYChartSeriesIdentifier & { // seriesColorKey: string; data: DataSeriesDatum[]; }; -export type RawDataSeries = SeriesIdentifier & { +export type RawDataSeries = XYChartSeriesIdentifier & { // seriesColorKey: string; data: RawDataSeriesDatum[]; }; @@ -80,10 +82,10 @@ export type SeriesCollectionValue = { banded?: boolean; lastValue?: LastValues; specSortIndex?: number; - seriesIdentifier: SeriesIdentifier; + seriesIdentifier: XYChartSeriesIdentifier; }; -export function getSeriesIndex(series: SeriesIdentifier[], target: SeriesIdentifier): number { +export function getSeriesIndex(series: XYChartSeriesIdentifier[], target: XYChartSeriesIdentifier): number { if (!series) { return -1; } @@ -149,7 +151,7 @@ export function getSeriesKey({ specId, yAccessor, splitAccessors, -}: Pick): string { +}: Pick): string { const joinedAccessors = [...splitAccessors.entries()] .sort(([a], [b]) => (a > b ? 1 : -1)) .map(([key, value]) => `${key}-${value}`) @@ -332,7 +334,7 @@ function getRawDataSeries( */ export function getSplittedSeries( seriesSpecs: BasicSeriesSpec[], - deselectedDataSeries: SeriesIdentifier[] = [], + deselectedDataSeries: XYChartSeriesIdentifier[] = [], ): { splittedSeries: Map; seriesCollection: Map; @@ -362,7 +364,7 @@ export function getSplittedSeries( seriesCollection.set(series.key, { banded, specSortIndex: spec.sortIndex, - seriesIdentifier: series as SeriesIdentifier, + seriesIdentifier: series as XYChartSeriesIdentifier, }); }); @@ -404,7 +406,7 @@ const getCustomSubSeriesName = (() => { const getSeriesLabelKeys = ( spec: BasicSeriesSpec, - seriesIdentifier: SeriesIdentifier, + seriesIdentifier: XYChartSeriesIdentifier, isTooltip: boolean, ): (string | number)[] => { const isMultipleY = spec.yAccessors.length > 1; @@ -426,7 +428,7 @@ const getSeriesLabelKeys = ( * Get series label based on `SeriesIdentifier` */ export function getSeriesLabel( - seriesIdentifier: SeriesIdentifier, + seriesIdentifier: XYChartSeriesIdentifier, hasSingleSeries: boolean, isTooltip: boolean, spec?: BasicSeriesSpec, diff --git a/src/chart_types/xy_chart/utils/specs.ts b/src/chart_types/xy_chart/utils/specs.ts index 026ee0fbc9..c85724ecc8 100644 --- a/src/chart_types/xy_chart/utils/specs.ts +++ b/src/chart_types/xy_chart/utils/specs.ts @@ -13,7 +13,7 @@ import { RecursivePartial, Color, Position, Datum } from '../../../utils/commons import { AxisId, GroupId } from '../../../utils/ids'; import { ScaleContinuousType, ScaleType } from '../../../scales'; import { CurveType } from '../../../utils/curves'; -import { RawDataSeriesDatum, SeriesIdentifier } from './series'; +import { RawDataSeriesDatum, XYChartSeriesIdentifier } from './series'; import { AnnotationTooltipFormatter } from '../annotations/annotation_utils'; import { Spec, SpecTypes } from '../../..'; import { ChartTypes } from '../..'; @@ -37,7 +37,10 @@ export type SeriesTypes = $Values; * - `RecursivePartial`: Style values to be merged with base bar styles * - `null`: Keep existing bar style */ -export type BarStyleAccessor = (datum: RawDataSeriesDatum, seriesIdentifier: SeriesIdentifier) => BarStyleOverride; +export type BarStyleAccessor = ( + datum: RawDataSeriesDatum, + seriesIdentifier: XYChartSeriesIdentifier, +) => BarStyleOverride; /** * Override for bar styles per datum * @@ -46,11 +49,14 @@ export type BarStyleAccessor = (datum: RawDataSeriesDatum, seriesIdentifier: Ser * - `RecursivePartial`: Style values to be merged with base point styles * - `null`: Keep existing point style */ -export type PointStyleAccessor = (datum: RawDataSeriesDatum, seriesIdentifier: SeriesIdentifier) => PointStyleOverride; +export type PointStyleAccessor = ( + datum: RawDataSeriesDatum, + seriesIdentifier: XYChartSeriesIdentifier, +) => PointStyleOverride; export const DEFAULT_GLOBAL_ID = '__global__'; -export type FilterPredicate = (series: SeriesIdentifier) => boolean; -export type SeriesStringPredicate = (series: SeriesIdentifier, isTooltip: boolean) => string | null; +export type FilterPredicate = (series: XYChartSeriesIdentifier) => boolean; +export type SeriesStringPredicate = (series: XYChartSeriesIdentifier, isTooltip: boolean) => string | null; export type SubSeriesStringPredicate = ( accessorLabel: string | number, accessorKey: string | number | null, @@ -239,7 +245,7 @@ export interface SeriesSpec extends Spec { * * This takes precedence over `customSubSeriesLabel` * - * @param series - `SeriesIdentifier` + * @param series - `XYChartSeriesIdentifier` * @param isTooltip - true if tooltip label, otherwise legend label */ customSeriesLabel?: SeriesStringPredicate; @@ -267,7 +273,7 @@ export interface Postfixes { } export type SeriesColorsArray = string[]; -export type SeriesColorAccessorFn = (seriesIdentifier: SeriesIdentifier) => string | null; +export type SeriesColorAccessorFn = (seriesIdentifier: XYChartSeriesIdentifier) => string | null; export type CustomSeriesColors = SeriesColorsArray | SeriesColorAccessorFn; export interface SeriesAccessors { diff --git a/src/components/legend/legend_item.tsx b/src/components/legend/legend_item.tsx index c9decc5e99..ebb2407ae2 100644 --- a/src/components/legend/legend_item.tsx +++ b/src/components/legend/legend_item.tsx @@ -6,7 +6,7 @@ import { LegendItemListener, BasicListener } from '../../specs/settings'; import { LegendItem } from '../../chart_types/xy_chart/legend/legend'; import { onLegendItemOutAction, onLegendItemOverAction } from '../../state/actions/legend'; import { Position } from '../../utils/commons'; -import { SeriesIdentifier } from '../../chart_types/xy_chart/utils/series'; +import { XYChartSeriesIdentifier } from '../../chart_types/xy_chart/utils/series'; interface LegendItemProps { selectedLegendItem?: LegendItem | null; @@ -20,7 +20,7 @@ interface LegendItemProps { onLegendItemOverListener?: LegendItemListener; legendItemOutAction: typeof onLegendItemOutAction; legendItemOverAction: typeof onLegendItemOverAction; - toggleDeselectSeriesAction: (legendItemId: SeriesIdentifier) => void; + toggleDeselectSeriesAction: (legendItemId: XYChartSeriesIdentifier) => void; } /** @@ -138,7 +138,7 @@ export class LegendListItem extends React.Component { }; // TODO handle shift key - onVisibilityClick = (legendItemId: SeriesIdentifier) => () => { + onVisibilityClick = (legendItemId: XYChartSeriesIdentifier) => () => { const { onLegendItemClickListener, toggleDeselectSeriesAction } = this.props; if (onLegendItemClickListener) { onLegendItemClickListener(legendItemId); diff --git a/src/index.ts b/src/index.ts index 2e6d548c6b..fd9581903d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -17,7 +17,7 @@ export { ChartTypes } from './chart_types'; export { Datum, Position, Rendering, Rotation } from './utils/commons'; export { TickFormatter } from './chart_types/xy_chart/utils/specs'; export { TooltipType, TooltipValue, TooltipValueFormatter } from './chart_types/xy_chart/utils/interactions'; -export { SeriesIdentifier } from './chart_types/xy_chart/utils/series'; +export { SeriesIdentifier, XYChartSeriesIdentifier } from './chart_types/xy_chart/utils/series'; export { AnnotationDomainType, AnnotationDomainTypes, diff --git a/src/mocks/series/seriesIdentifiers.ts b/src/mocks/series/seriesIdentifiers.ts index 38f3b07671..f471994f51 100644 --- a/src/mocks/series/seriesIdentifiers.ts +++ b/src/mocks/series/seriesIdentifiers.ts @@ -1,6 +1,10 @@ import { BasicSeriesSpec } from '../../chart_types/xy_chart/utils/specs'; import { getSpecId } from '../..'; -import { SeriesCollectionValue, getSplittedSeries, SeriesIdentifier } from '../../chart_types/xy_chart/utils/series'; +import { + SeriesCollectionValue, + getSplittedSeries, + XYChartSeriesIdentifier, +} from '../../chart_types/xy_chart/utils/series'; import { mergePartial } from '../../utils/commons'; type SeriesCollection = Map; @@ -18,7 +22,7 @@ export class MockSeriesCollection { } export class MockSeriesIdentifier { - private static readonly base: SeriesIdentifier = { + private static readonly base: XYChartSeriesIdentifier = { specId: getSpecId('bars'), yAccessor: 'y', seriesKeys: ['a'], @@ -26,7 +30,9 @@ export class MockSeriesIdentifier { key: 'spec{bars}yAccessor{y}splitAccessors{g-a}', }; - static default(partial?: Partial) { - return mergePartial(MockSeriesIdentifier.base, partial, { mergeOptionalPartialValues: true }); + static default(partial?: Partial) { + return mergePartial(MockSeriesIdentifier.base, partial, { + mergeOptionalPartialValues: true, + }); } } diff --git a/src/specs/settings.tsx b/src/specs/settings.tsx index f90920df4b..9d51142d50 100644 --- a/src/specs/settings.tsx +++ b/src/specs/settings.tsx @@ -8,14 +8,14 @@ import { Spec } from '.'; import { LIGHT_THEME } from '../utils/themes/light_theme'; import { ChartTypes } from '../chart_types'; import { GeometryValue } from '../utils/geometry'; -import { SeriesIdentifier } from '../chart_types/xy_chart/utils/series'; +import { XYChartSeriesIdentifier } from '../chart_types/xy_chart/utils/series'; import { Position, Rendering, Rotation } from '../utils/commons'; import { ScaleContinuousType, ScaleOrdinalType } from '../scales'; -export type ElementClickListener = (elements: Array<[GeometryValue, SeriesIdentifier]>) => void; -export type ElementOverListener = (elements: Array<[GeometryValue, SeriesIdentifier]>) => void; +export type ElementClickListener = (elements: Array<[GeometryValue, XYChartSeriesIdentifier]>) => void; +export type ElementOverListener = (elements: Array<[GeometryValue, XYChartSeriesIdentifier]>) => void; export type BrushEndListener = (min: number, max: number) => void; -export type LegendItemListener = (series: SeriesIdentifier | null) => void; +export type LegendItemListener = (series: XYChartSeriesIdentifier | null) => void; export type PointerUpdateListener = (event: PointerEvent) => void; /** * Listener to be called when chart render state changes diff --git a/src/state/actions/legend.ts b/src/state/actions/legend.ts index 3defd22d79..4166e62e11 100644 --- a/src/state/actions/legend.ts +++ b/src/state/actions/legend.ts @@ -1,4 +1,4 @@ -import { SeriesIdentifier } from '../../chart_types/xy_chart/utils/series'; +import { XYChartSeriesIdentifier } from '../../chart_types/xy_chart/utils/series'; export const ON_TOGGLE_LEGEND = 'ON_TOGGLE_LEGEND'; export const ON_LEGEND_ITEM_OVER = 'ON_LEGEND_ITEM_OVER'; @@ -18,7 +18,7 @@ interface LegendItemOutAction { interface ToggleDeselectSeriesAction { type: typeof ON_TOGGLE_DESELECT_SERIES; - legendItemId: SeriesIdentifier; + legendItemId: XYChartSeriesIdentifier; } export function onToggleLegend(): ToggleLegendAction { @@ -33,7 +33,7 @@ export function onLegendItemOutAction(): LegendItemOutAction { return { type: ON_LEGEND_ITEM_OUT }; } -export function onToggleDeselectSeriesAction(legendItemId: SeriesIdentifier): ToggleDeselectSeriesAction { +export function onToggleDeselectSeriesAction(legendItemId: XYChartSeriesIdentifier): ToggleDeselectSeriesAction { return { type: ON_TOGGLE_DESELECT_SERIES, legendItemId }; } diff --git a/src/state/chart_state.ts b/src/state/chart_state.ts index 09cb75e537..6b536f7cb3 100644 --- a/src/state/chart_state.ts +++ b/src/state/chart_state.ts @@ -2,7 +2,7 @@ import { SPEC_PARSED, SPEC_UNMOUNTED, UPSERT_SPEC, REMOVE_SPEC, SPEC_PARSING } f import { interactionsReducer } from './reducers/interactions'; import { ChartTypes } from '../chart_types'; import { XYAxisChartState } from '../chart_types/xy_chart/state/chart_state'; -import { SeriesIdentifier } from '../chart_types/xy_chart/utils/series'; +import { XYChartSeriesIdentifier } from '../chart_types/xy_chart/utils/series'; import { Spec, PointerEvent } from '../specs'; import { DEFAULT_SETTINGS_SPEC } from '../specs/settings'; import { Dimensions } from '../utils/dimensions'; @@ -67,7 +67,7 @@ export interface InteractionsState { highlightedLegendItemKey: string | null; legendCollapsed: boolean; invertDeselect: boolean; - deselectedDataSeries: SeriesIdentifier[]; + deselectedDataSeries: XYChartSeriesIdentifier[]; } export interface ExternalEventsState { diff --git a/src/state/reducers/interactions.ts b/src/state/reducers/interactions.ts index 203ade3392..3d672a3044 100644 --- a/src/state/reducers/interactions.ts +++ b/src/state/reducers/interactions.ts @@ -7,7 +7,7 @@ import { LegendActions, } from '../actions/legend'; import { ON_MOUSE_DOWN, ON_MOUSE_UP, ON_POINTER_MOVE, MouseActions } from '../actions/mouse'; -import { getSeriesIndex, SeriesIdentifier } from '../../chart_types/xy_chart/utils/series'; +import { getSeriesIndex, XYChartSeriesIdentifier } from '../../chart_types/xy_chart/utils/series'; export function interactionsReducer(state: InteractionsState, action: LegendActions | MouseActions): InteractionsState { switch (action.type) { @@ -107,7 +107,10 @@ export function interactionsReducer(state: InteractionsState, action: LegendActi } } -function toggleDeselectedDataSeries(legendItem: SeriesIdentifier, deselectedDataSeries: SeriesIdentifier[]) { +function toggleDeselectedDataSeries( + legendItem: XYChartSeriesIdentifier, + deselectedDataSeries: XYChartSeriesIdentifier[], +) { const index = getSeriesIndex(deselectedDataSeries, legendItem); if (index > -1) { return [...deselectedDataSeries.slice(0, index), ...deselectedDataSeries.slice(index + 1)]; diff --git a/src/utils/geometry.ts b/src/utils/geometry.ts index 00c21ed340..8c0cded6c7 100644 --- a/src/utils/geometry.ts +++ b/src/utils/geometry.ts @@ -1,6 +1,6 @@ import { $Values } from 'utility-types'; import { BarSeriesStyle, PointStyle, AreaStyle, LineStyle, ArcStyle } from './themes/theme'; -import { SeriesIdentifier } from '../chart_types/xy_chart/utils/series'; +import { XYChartSeriesIdentifier } from '../chart_types/xy_chart/utils/series'; /** * The accessor type @@ -36,7 +36,7 @@ export interface PointGeometry { x: number; y: number; }; - seriesIdentifier: SeriesIdentifier; + seriesIdentifier: XYChartSeriesIdentifier; value: GeometryValue; styleOverrides?: Partial; } @@ -53,7 +53,7 @@ export interface BarGeometry { hideClippedValue?: boolean; isValueContainedInElement?: boolean; }; - seriesIdentifier: SeriesIdentifier; + seriesIdentifier: XYChartSeriesIdentifier; value: GeometryValue; seriesStyle: BarSeriesStyle; } @@ -66,7 +66,7 @@ export interface LineGeometry { x: number; y: number; }; - seriesIdentifier: SeriesIdentifier; + seriesIdentifier: XYChartSeriesIdentifier; seriesLineStyle: LineStyle; seriesPointStyle: PointStyle; /** @@ -84,7 +84,7 @@ export interface AreaGeometry { x: number; y: number; }; - seriesIdentifier: SeriesIdentifier; + seriesIdentifier: XYChartSeriesIdentifier; seriesAreaStyle: AreaStyle; seriesAreaLineStyle: LineStyle; seriesPointStyle: PointStyle; @@ -98,7 +98,7 @@ export interface AreaGeometry { export interface ArcGeometry { arc: string; color: string; - seriesIdentifier: SeriesIdentifier; + seriesIdentifier: XYChartSeriesIdentifier; seriesArcStyle: ArcStyle; transform: { x: number; diff --git a/wiki/overview.md b/wiki/overview.md index c997fc6a19..1105c2d0c0 100644 --- a/wiki/overview.md +++ b/wiki/overview.md @@ -229,7 +229,7 @@ Return types: ```ts type BarStyleAccessor = ( datum: RawDataSeriesDatum, - seriesIdentifier: SeriesIdentifier, + seriesIdentifier: XYChartSeriesIdentifier, ) => RecursivePartial | Color | null; ``` @@ -246,7 +246,7 @@ Return types: ```ts type PointStyleAccessor = ( datum: RawDataSeriesDatum, - seriesIdentifier: SeriesIdentifier, + seriesIdentifier: XYChartSeriesIdentifier, ) => RecursivePartial | Color | null; ``` From 4a8c2189cf55cb0155ea6f591fe46555e8410e1e Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Fri, 14 Feb 2020 19:01:46 +0100 Subject: [PATCH 06/17] refactor(legend): rename displayValue to extra The commit change the naming for few variables in the legend item object, removing some unnecessary code BREAKING CHANGE: The `SettingsSpec` prop `showLegendDisplayValue` is renamed to `showLegendExtra` --- src/components/legend/_legend_item.scss | 12 ++-- src/components/legend/legend.test.tsx | 4 +- src/components/legend/legend.tsx | 11 ++-- src/components/legend/legend_item.tsx | 84 ++++++++++++------------- src/mocks/specs/specs.ts | 2 +- src/specs/settings.test.tsx | 6 +- src/specs/settings.tsx | 9 ++- src/state/selectors/get_legend_size.ts | 8 +-- stories/legend.tsx | 2 +- 9 files changed, 68 insertions(+), 70 deletions(-) diff --git a/src/components/legend/_legend_item.scss b/src/components/legend/_legend_item.scss index e312e19eb3..70a62d079c 100644 --- a/src/components/legend/_legend_item.scss +++ b/src/components/legend/_legend_item.scss @@ -10,7 +10,7 @@ $legendItemVerticalPadding: $echLegendRowGap / 2; width: 100%; &:hover { - .echLegendItem__title { + .echLegendItem__label { text-decoration: underline; } } @@ -27,7 +27,7 @@ $legendItemVerticalPadding: $echLegendRowGap / 2; } } - &__title { + &__label { @include euiFontSizeXS; @include euiTextTruncate; flex: 1 1 auto; @@ -37,17 +37,13 @@ $legendItemVerticalPadding: $echLegendRowGap / 2; } } - &__title--selected { - text-decoration: underline; - } - - &__title--hasClickListener { + &__label--hasClickListener { &:hover { cursor: pointer; } } - &__displayValue { + &__extra { @include euiFontSizeXS; text-align: right; margin-left: $euiSizeXS; diff --git a/src/components/legend/legend.test.tsx b/src/components/legend/legend.test.tsx index 68dd197d4f..e2e24dfa66 100644 --- a/src/components/legend/legend.test.tsx +++ b/src/components/legend/legend.test.tsx @@ -42,7 +42,7 @@ describe('Legend', () => { it('shall render the all the series names without the data value', () => { const wrapper = mount( - + { expect(legendItems).toHaveLength(4); legendItems.forEach((legendItem, i) => { // the click is only enabled on the title - legendItem.find('.echLegendItem__title').simulate('click'); + legendItem.find('.echLegendItem__label').simulate('click'); expect(onLegendItemClick).toBeCalledTimes(i + 1); }); }); diff --git a/src/components/legend/legend.tsx b/src/components/legend/legend.tsx index 7ea0c0bc06..91f6d14e2f 100644 --- a/src/components/legend/legend.tsx +++ b/src/components/legend/legend.tsx @@ -138,19 +138,18 @@ class LegendComponent extends React.Component { } const { key, displayValue, banded } = item; const { legendItemTooltipValues, settings } = this.props; - const { showLegendDisplayValue, legendPosition } = settings; + const { showLegendExtra, legendPosition } = settings; const legendValues = this.getLegendValues(legendItemTooltipValues, key, banded); return legendValues.map((value, index) => { const yAccessor: BandedAccessorType = index === 0 ? BandedAccessorType.Y1 : BandedAccessorType.Y0; return ( - {displayValue} +
+ {extra}
); } /** - * Create a div for the title - * @param title - * @param onTitleClick - * @param hasTitleClickListener - * @param isSelected - * @param showLegendDisplayValue + * Create a div for the label + * @param label + * @param onLabelClick + * @param hasLabelClickListener */ -function renderTitle( - title: string, - onTitleClick: (event: React.MouseEvent) => void, - hasTitleClickListener: boolean, - isSelected: boolean, - showLegendDisplayValue: boolean, +function renderLabel( + onLabelClick: (event: React.MouseEvent) => void, + hasLabelClickListener: boolean, + label?: string, ) { - // TODO add contextual menu panel on click - const titleClassNames = classNames('echLegendItem__title', { - ['echLegendItem__title--hasClickListener']: hasTitleClickListener, - ['echLegendItem__title--selected']: isSelected, - ['echLegendItem__title--hasDisplayValue']: showLegendDisplayValue, + if (!label) { + return null; + } + const labelClassNames = classNames('echLegendItem__label', { + ['echLegendItem__label--hasClickListener']: hasLabelClickListener, }); return ( -
- {title} +
+ {label}
); } @@ -72,7 +67,10 @@ function renderTitle( * @param color * @param isSeriesVisible */ -function renderColor(color: string, isSeriesVisible = true) { +function renderColor(color?: string, isSeriesVisible = true) { + if (!color) { + return null; + } // TODO add color picker if (isSeriesVisible) { return ( @@ -97,24 +95,26 @@ export class LegendListItem extends React.Component { } render() { - const { displayValue, legendItem, legendPosition, label } = this.props; + const { extra, legendItem, legendPosition, label, showExtra, onLegendItemClickListener } = this.props; const { color, isSeriesVisible, seriesIdentifier, isLegendItemVisible } = legendItem; - const onTitleClick = this.onVisibilityClick(seriesIdentifier); - const { showLegendDisplayValue, selectedLegendItem, onLegendItemClickListener } = this.props; - const isSelected = - selectedLegendItem == null ? false : selectedLegendItem.seriesIdentifier.key === seriesIdentifier.key; - const hasTitleClickListener = Boolean(onLegendItemClickListener); - const itemClasses = classNames('echLegendItem', `echLegendItem--${legendPosition}`, { + const onLabelClick = this.onVisibilityClick(seriesIdentifier); + const hasLabelClickListener = Boolean(onLegendItemClickListener); + + const itemClassNames = classNames('echLegendItem', `echLegendItem--${legendPosition}`, { 'echLegendItem-isHidden': !isSeriesVisible, - 'echLegendItem__displayValue--hidden': !isLegendItemVisible, + 'echLegendItem__extra--hidden': !isLegendItemVisible, }); return ( -
- {color && renderColor(color, isSeriesVisible)} - {label && renderTitle(label, onTitleClick, hasTitleClickListener, isSelected, showLegendDisplayValue)} - {showLegendDisplayValue && renderDisplayValue(displayValue, isSeriesVisible)} +
+ {renderColor(color, isSeriesVisible)} + {renderLabel(onLabelClick, hasLabelClickListener, label)} + {showExtra && renderExtra(extra, isSeriesVisible)}
); } diff --git a/src/mocks/specs/specs.ts b/src/mocks/specs/specs.ts index 572a8d7915..d3b1c6c08f 100644 --- a/src/mocks/specs/specs.ts +++ b/src/mocks/specs/specs.ts @@ -149,7 +149,7 @@ export class MockGlobalSpec { snap: true, }, legendPosition: Position.Right, - showLegendDisplayValue: true, + showLegendExtra: true, hideDuplicateAxes: false, theme: LIGHT_THEME, }; diff --git a/src/specs/settings.test.tsx b/src/specs/settings.test.tsx index 7c1180a0b7..acb3223241 100644 --- a/src/specs/settings.test.tsx +++ b/src/specs/settings.test.tsx @@ -68,7 +68,7 @@ describe('Settings spec component', () => { snap: false, }, legendPosition: Position.Bottom, - showLegendDisplayValue: false, + showLegendExtra: false, debug: true, xDomain: { min: 0, max: 10 }, }, @@ -84,7 +84,7 @@ describe('Settings spec component', () => { snap: false, }); expect(settingSpec.legendPosition).toBe(Position.Bottom); - expect(settingSpec.showLegendDisplayValue).toEqual(false); + expect(settingSpec.showLegendExtra).toEqual(false); expect(settingSpec.debug).toBe(true); expect(settingSpec.xDomain).toEqual({ min: 0, max: 10 }); }); @@ -176,7 +176,7 @@ describe('Settings spec component', () => { snap: false, }, legendPosition: Position.Bottom, - showLegendDisplayValue: false, + showLegendExtra: false, hideDuplicateAxes: false, debug: true, xDomain: { min: 0, max: 10 }, diff --git a/src/specs/settings.tsx b/src/specs/settings.tsx index 9d51142d50..4ed5371545 100644 --- a/src/specs/settings.tsx +++ b/src/specs/settings.tsx @@ -90,7 +90,10 @@ export interface SettingsSpec extends Spec { tooltip: TooltipType | TooltipProps; debug: boolean; legendPosition: Position; - showLegendDisplayValue: boolean; + /** + * Show an extra parameter on each legend item defined by the chart type + */ + showLegendExtra: boolean; /** * Removes duplicate axes * @@ -123,7 +126,7 @@ export type DefaultSettingsProps = | 'showLegend' | 'debug' | 'tooltip' - | 'showLegendDisplayValue' + | 'showLegendExtra' | 'theme' | 'legendPosition' | 'hideDuplicateAxes'; @@ -155,7 +158,7 @@ export const DEFAULT_SETTINGS_SPEC: SettingsSpec = { snap: DEFAULT_TOOLTIP_SNAP, }, legendPosition: Position.Right, - showLegendDisplayValue: true, + showLegendExtra: true, hideDuplicateAxes: false, theme: LIGHT_THEME, }; diff --git a/src/state/selectors/get_legend_size.ts b/src/state/selectors/get_legend_size.ts index 18dd4b534e..d641d7051f 100644 --- a/src/state/selectors/get_legend_size.ts +++ b/src/state/selectors/get_legend_size.ts @@ -15,18 +15,18 @@ const legendItemLabelsSelector = createCachedSelector( [getSettingsSpecSelector, getLegendItemsSelector], (settings, legendItems): string[] => { const labels: string[] = []; - const { showLegendDisplayValue } = settings; + const { showLegendExtra } = settings; legendItems.forEach((item) => { const labelY1 = getItemLabel(item, 'y1'); if (item.displayValue.formatted.y1 !== null) { - labels.push(`${labelY1}${showLegendDisplayValue ? item.displayValue.formatted.y1 : ''}`); + labels.push(`${labelY1}${showLegendExtra ? item.displayValue.formatted.y1 : ''}`); } else { labels.push(labelY1); } if (item.banded) { const labelY0 = getItemLabel(item, 'y0'); if (item.displayValue.formatted.y0 !== null) { - labels.push(`${labelY0}${showLegendDisplayValue ? item.displayValue.formatted.y0 : ''}`); + labels.push(`${labelY0}${showLegendExtra ? item.displayValue.formatted.y0 : ''}`); } else { labels.push(labelY0); } @@ -68,7 +68,7 @@ export const getLegendSizeSelector = createCachedSelector( ); bboxCalculator.destroy(); - const { showLegend, showLegendDisplayValue, legendPosition } = settings; + const { showLegend, showLegendExtra: showLegendDisplayValue, legendPosition } = settings; const { legend: { verticalWidth, spacingBuffer }, } = theme; diff --git a/stories/legend.tsx b/stories/legend.tsx index 614215b481..7c729be812 100644 --- a/stories/legend.tsx +++ b/stories/legend.tsx @@ -264,7 +264,7 @@ export const displayValuesInLegendElements = () => { }); return ( - + Date: Fri, 14 Feb 2020 19:17:19 +0100 Subject: [PATCH 07/17] refactor(tooltip): remove isXValue prop from TooltipValue The isXValue is no longer required inside the TooltipValue object because we are already defining the header and the values inside two distinct objects --- src/chart_types/xy_chart/state/chart_state.test.ts | 6 ------ src/chart_types/xy_chart/tooltip/tooltip.test.ts | 4 ---- src/chart_types/xy_chart/tooltip/tooltip.ts | 7 +++---- src/chart_types/xy_chart/utils/interactions.ts | 1 - 4 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/chart_types/xy_chart/state/chart_state.test.ts b/src/chart_types/xy_chart/state/chart_state.test.ts index cff92ad127..277510b2dc 100644 --- a/src/chart_types/xy_chart/state/chart_state.test.ts +++ b/src/chart_types/xy_chart/state/chart_state.test.ts @@ -776,7 +776,6 @@ describe.skip('Chart Store', () => { value: 'a', color: 'a', isHighlighted: false, - isXValue: false, seriesKey: 'a', yAccessor: 'y', isVisible: true, @@ -890,7 +889,6 @@ describe.skip('Chart Store', () => { value: 'a', color: 'a', isHighlighted: false, - isXValue: false, seriesKey: 'a', yAccessor: 'y', isVisible: true, @@ -1034,7 +1032,6 @@ describe.skip('Chart Store', () => { value: 1, color: 'color', isHighlighted: true, - isXValue: false, seriesKey: 'foo', yAccessor: 'y', isVisible: true, @@ -1044,7 +1041,6 @@ describe.skip('Chart Store', () => { value: 1, color: 'color', isHighlighted: false, - isXValue: false, seriesKey: 'foo', yAccessor: 'y', isVisible: true, @@ -1074,7 +1070,6 @@ describe.skip('Chart Store', () => { value: 'foo', color: 'a', isHighlighted: false, - isXValue: true, seriesKey: 'headerSeries', isVisible: true, yAccessor: BandedAccessorType.Y0, @@ -1088,7 +1083,6 @@ describe.skip('Chart Store', () => { value: 123, color: 'a', isHighlighted: false, - isXValue: false, seriesKey: 'seriesKeys', isVisible: true, yAccessor: BandedAccessorType.Y1, diff --git a/src/chart_types/xy_chart/tooltip/tooltip.test.ts b/src/chart_types/xy_chart/tooltip/tooltip.test.ts index a43bc62da8..5038f2bb9b 100644 --- a/src/chart_types/xy_chart/tooltip/tooltip.test.ts +++ b/src/chart_types/xy_chart/tooltip/tooltip.test.ts @@ -102,7 +102,6 @@ describe('Tooltip formatting', () => { expect(tooltipValue).toBeDefined(); expect(tooltipValue.yAccessor).toBe('y1'); expect(tooltipValue.name).toBe('bar_1'); - expect(tooltipValue.isXValue).toBe(false); expect(tooltipValue.isHighlighted).toBe(false); expect(tooltipValue.color).toBe('blue'); expect(tooltipValue.value).toBe('10'); @@ -208,7 +207,6 @@ describe('Tooltip formatting', () => { expect(tooltipValue).toBeDefined(); expect(tooltipValue.yAccessor).toBe('y1'); expect(tooltipValue.name).toBe('bar_1'); - expect(tooltipValue.isXValue).toBe(false); expect(tooltipValue.isHighlighted).toBe(false); expect(tooltipValue.color).toBe('blue'); expect(tooltipValue.value).toBe('10'); @@ -225,7 +223,6 @@ describe('Tooltip formatting', () => { expect(tooltipValue).toBeDefined(); expect(tooltipValue.yAccessor).toBe('y0'); expect(tooltipValue.name).toBe('bar_1'); - expect(tooltipValue.isXValue).toBe(false); expect(tooltipValue.isHighlighted).toBe(false); expect(tooltipValue.color).toBe('blue'); expect(tooltipValue.value).toBe('10'); @@ -242,7 +239,6 @@ describe('Tooltip formatting', () => { expect(tooltipValue).toBeDefined(); expect(tooltipValue.yAccessor).toBe('y0'); expect(tooltipValue.name).toBe('bar_1'); - expect(tooltipValue.isXValue).toBe(true); expect(tooltipValue.isHighlighted).toBe(false); expect(tooltipValue.color).toBe('blue'); expect(tooltipValue.value).toBe('1'); diff --git a/src/chart_types/xy_chart/tooltip/tooltip.ts b/src/chart_types/xy_chart/tooltip/tooltip.ts index 77ecaf985b..575e8a35b6 100644 --- a/src/chart_types/xy_chart/tooltip/tooltip.ts +++ b/src/chart_types/xy_chart/tooltip/tooltip.ts @@ -43,7 +43,7 @@ export function getSeriesTooltipValues( export function formatTooltip( { color, value: { x, y, accessor }, seriesIdentifier }: IndexedGeometry, spec: BasicSeriesSpec, - isXValue: boolean, + isHeader: boolean, isHighlighted: boolean, hasSingleSeries: boolean, axisSpec?: AxisSpec, @@ -59,15 +59,14 @@ export function formatTooltip( const isFiltered = spec.filterSeriesInTooltip !== undefined ? spec.filterSeriesInTooltip(seriesIdentifier) : true; const isVisible = displayName === '' ? false : isFiltered; - const value = isXValue ? x : y; + const value = isHeader ? x : y; const tickFormatOptions: TickFormatterOptions | undefined = spec.timeZone ? { timeZone: spec.timeZone } : undefined; return { seriesKey, name: displayName, value: axisSpec ? axisSpec.tickFormat(value, tickFormatOptions) : emptyFormatter(value), color, - isHighlighted: isXValue ? false : isHighlighted, - isXValue, + isHighlighted: isHeader ? false : isHighlighted, yAccessor: accessor, isVisible, }; diff --git a/src/chart_types/xy_chart/utils/interactions.ts b/src/chart_types/xy_chart/utils/interactions.ts index cae47553f0..b1ce6b5c4b 100644 --- a/src/chart_types/xy_chart/utils/interactions.ts +++ b/src/chart_types/xy_chart/utils/interactions.ts @@ -24,7 +24,6 @@ export interface TooltipValue { value: any; color: string; isHighlighted: boolean; - isXValue: boolean; seriesKey: string; yAccessor: Accessor; isVisible: boolean; From 2fa3fb03d8b65129e130bf0937aba8c6f594c353 Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Tue, 18 Feb 2020 10:25:43 +0100 Subject: [PATCH 08/17] refactor: update TooltipValue type with decoupled names I've renamed the yAccessor field in favour of a more general valueAccessor. I've also added the seriesIdentifier to each tooltipValue --- .../xy_chart/rendering/rendering.ts | 8 --- .../xy_chart/state/chart_state.test.ts | 54 ++++++++++++------- .../xy_chart/tooltip/tooltip.test.ts | 32 +++++------ src/chart_types/xy_chart/tooltip/tooltip.ts | 23 ++++---- .../xy_chart/utils/interactions.ts | 30 +++++++++-- src/components/tooltip/index.tsx | 44 +++++++-------- 6 files changed, 112 insertions(+), 79 deletions(-) diff --git a/src/chart_types/xy_chart/rendering/rendering.ts b/src/chart_types/xy_chart/rendering/rendering.ts index c4c450e733..f74410db0b 100644 --- a/src/chart_types/xy_chart/rendering/rendering.ts +++ b/src/chart_types/xy_chart/rendering/rendering.ts @@ -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 || ''}`; -} diff --git a/src/chart_types/xy_chart/state/chart_state.test.ts b/src/chart_types/xy_chart/state/chart_state.test.ts index 277510b2dc..67ebc024bb 100644 --- a/src/chart_types/xy_chart/state/chart_state.test.ts +++ b/src/chart_types/xy_chart/state/chart_state.test.ts @@ -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; @@ -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] }); @@ -1028,21 +1034,27 @@ describe.skip('Chart Store', () => { store.annotationDimensions.set(rectAnnotationSpec.id, annotationDimensions); const highlightedTooltipValue: TooltipValue = { - name: 'foo', + label: 'foo', 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, }; @@ -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]); diff --git a/src/chart_types/xy_chart/tooltip/tooltip.test.ts b/src/chart_types/xy_chart/tooltip/tooltip.test.ts index 5038f2bb9b..08fe890c75 100644 --- a/src/chart_types/xy_chart/tooltip/tooltip.test.ts +++ b/src/chart_types/xy_chart/tooltip/tooltip.test.ts @@ -100,8 +100,8 @@ 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'); @@ -109,15 +109,15 @@ describe('Tooltip formatting', () => { 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( @@ -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( @@ -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( @@ -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( @@ -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( @@ -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 = { @@ -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'); @@ -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'); @@ -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'); diff --git a/src/chart_types/xy_chart/tooltip/tooltip.ts b/src/chart_types/xy_chart/tooltip/tooltip.ts index 575e8a35b6..de8ff0233f 100644 --- a/src/chart_types/xy_chart/tooltip/tooltip.ts +++ b/src/chart_types/xy_chart/tooltip/tooltip.ts @@ -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; @@ -26,15 +26,15 @@ export function getSeriesTooltipValues( // map from seriesKey to TooltipLegendValue const seriesTooltipValues = new Map(); - 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; @@ -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, }; } diff --git a/src/chart_types/xy_chart/utils/interactions.ts b/src/chart_types/xy_chart/utils/interactions.ts index b1ce6b5c4b..2eb1755385 100644 --- a/src/chart_types/xy_chart/utils/interactions.ts +++ b/src/chart_types/xy_chart/utils/interactions.ts @@ -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({ @@ -20,13 +21,34 @@ export const TooltipType = Object.freeze({ export type TooltipType = $Values; 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; diff --git a/src/components/tooltip/index.tsx b/src/components/tooltip/index.tsx index ad444622e9..8c5d2c3848 100644 --- a/src/components/tooltip/index.tsx +++ b/src/components/tooltip/index.tsx @@ -100,27 +100,29 @@ class TooltipComponent extends React.Component {
{this.renderHeader(tooltip.header, tooltipHeaderFormatter)}
- {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 ( -
- {name} - {value} -
- ); - })} + {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 ( +
+ {label} + {value} +
+ ); + }, + )}
); From 7c2d9647f5ce74e2e76a0605d878b9dcbe30cf92 Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Tue, 18 Feb 2020 10:49:43 +0100 Subject: [PATCH 09/17] refactor: decoupled tooltipHeaderFormatter selector --- .playground/playground.tsx | 8 ++- .../xy_chart/renderer/dom/crosshair.tsx | 4 +- .../state/chart_state.interactions.test.ts | 3 +- .../xy_chart/state/chart_state.test.ts | 3 +- .../state/chart_state.tooltip.test.ts | 2 +- .../state/selectors/get_tooltip_snap.ts | 3 +- .../state/selectors/get_tooltip_type.ts | 11 ++- .../get_tooltip_values_highlighted_geoms.ts | 14 ++-- .../state/selectors/is_tooltip_visible.ts | 19 ++---- src/chart_types/xy_chart/tooltip/tooltip.ts | 2 +- .../xy_chart/utils/interactions.test.ts | 4 +- .../xy_chart/utils/interactions.ts | 66 ------------------ src/components/tooltip/index.tsx | 8 +-- src/index.ts | 1 - src/mocks/specs/specs.ts | 3 +- src/specs/settings.test.tsx | 3 +- src/specs/settings.tsx | 67 ++++++++++++++++++- .../selectors/get_tooltip_header_formatter.ts | 7 +- 18 files changed, 109 insertions(+), 119 deletions(-) rename src/{chart_types/xy_chart => }/state/selectors/get_tooltip_header_formatter.ts (58%) diff --git a/.playground/playground.tsx b/.playground/playground.tsx index c3bd5e6f2b..aba458f9ad 100644 --- a/.playground/playground.tsx +++ b/.playground/playground.tsx @@ -1,5 +1,5 @@ import React from 'react'; -import { Chart, ScaleType, Position, Axis, getAxisId, timeFormatter, getSpecId, AreaSeries } from '../src'; +import { Chart, ScaleType, Position, Axis, getAxisId, timeFormatter, getSpecId, AreaSeries, Settings } from '../src'; import { KIBANA_METRICS } from '../src/utils/data_samples/test_dataset_kibana'; export class Playground extends React.Component { chartRef: React.RefObject = React.createRef(); @@ -13,6 +13,7 @@ export class Playground extends React.Component { <>
+ { + return [...d, d[1] - 10]; + })} />
diff --git a/src/chart_types/xy_chart/renderer/dom/crosshair.tsx b/src/chart_types/xy_chart/renderer/dom/crosshair.tsx index eef4518d06..edb043e647 100644 --- a/src/chart_types/xy_chart/renderer/dom/crosshair.tsx +++ b/src/chart_types/xy_chart/renderer/dom/crosshair.tsx @@ -1,6 +1,5 @@ import React, { CSSProperties } from 'react'; import { connect } from 'react-redux'; -import { TooltipType } from '../../utils/interactions'; import { isHorizontalRotation } from '../../state/utils'; import { Dimensions } from '../../../../utils/dimensions'; import { Theme } from '../../../../utils/themes/theme'; @@ -13,6 +12,7 @@ import { getCursorLinePositionSelector } from '../../state/selectors/get_cursor_ import { getTooltipTypeSelector } from '../../state/selectors/get_tooltip_type'; import { getChartThemeSelector } from '../../../../state/selectors/get_chart_theme'; import { LIGHT_THEME } from '../../../../utils/themes/light_theme'; +import { TooltipType } from '../../../../specs'; interface CrosshairProps { theme: Theme; @@ -108,7 +108,7 @@ const mapStateToProps = (state: GlobalChartState): CrosshairProps => { chartRotation: getChartRotationSelector(state), cursorBandPosition: getCursorBandPositionSelector(state), cursorLinePosition: getCursorLinePositionSelector(state), - tooltipType: getTooltipTypeSelector(state), + tooltipType: getTooltipTypeSelector(state) || TooltipType.VerticalCursor, }; }; diff --git a/src/chart_types/xy_chart/state/chart_state.interactions.test.ts b/src/chart_types/xy_chart/state/chart_state.interactions.test.ts index 888c334603..85ff1e2dd1 100644 --- a/src/chart_types/xy_chart/state/chart_state.interactions.test.ts +++ b/src/chart_types/xy_chart/state/chart_state.interactions.test.ts @@ -1,10 +1,9 @@ import { createStore, Store } from 'redux'; import { BarSeriesSpec, BasicSeriesSpec, AxisSpec, SeriesTypes } from '../utils/specs'; import { Position } from '../../../utils/commons'; -import { TooltipType } from '../utils/interactions'; import { ScaleType } from '../../../scales'; import { chartStoreReducer, GlobalChartState } from '../../../state/chart_state'; -import { SettingsSpec, DEFAULT_SETTINGS_SPEC, SpecTypes } from '../../../specs'; +import { SettingsSpec, DEFAULT_SETTINGS_SPEC, SpecTypes, TooltipType } from '../../../specs'; import { computeSeriesGeometriesSelector } from './selectors/compute_series_geometries'; import { getProjectedPointerPositionSelector } from './selectors/get_projected_pointer_position'; import { diff --git a/src/chart_types/xy_chart/state/chart_state.test.ts b/src/chart_types/xy_chart/state/chart_state.test.ts index 67ebc024bb..eadbb35557 100644 --- a/src/chart_types/xy_chart/state/chart_state.test.ts +++ b/src/chart_types/xy_chart/state/chart_state.test.ts @@ -8,14 +8,13 @@ import { SeriesTypes, } from '../utils/specs'; import { Position } from '../../../utils/commons'; -import { TooltipType, TooltipValue } from '../utils/interactions'; import { ScaleType, ScaleContinuous, ScaleBand } from '../../../scales'; import { IndexedGeometry, GeometryValue, BandedAccessorType } from '../../../utils/geometry'; import { AxisTicksDimensions, isDuplicateAxis } from '../utils/axis_utils'; import { AxisId } from '../../../utils/ids'; import { LegendItem } from '../legend/legend'; import { ChartTypes } from '../..'; -import { SpecTypes } from '../../../specs/settings'; +import { SpecTypes, TooltipValue, TooltipType } from '../../../specs/settings'; describe.skip('Chart Store', () => { let store: any = null; // diff --git a/src/chart_types/xy_chart/state/chart_state.tooltip.test.ts b/src/chart_types/xy_chart/state/chart_state.tooltip.test.ts index c4c703f103..3ebab7d57b 100644 --- a/src/chart_types/xy_chart/state/chart_state.tooltip.test.ts +++ b/src/chart_types/xy_chart/state/chart_state.tooltip.test.ts @@ -5,7 +5,7 @@ import { MockSeriesSpec, MockGlobalSpec } from '../../../mocks/specs'; import { updateParentDimensions } from '../../../state/actions/chart_settings'; import { getTooltipValuesAndGeometriesSelector } from './selectors/get_tooltip_values_highlighted_geoms'; import { onPointerMove } from '../../../state/actions/mouse'; -import { TooltipType } from '../utils/interactions'; +import { TooltipType } from '../../../specs'; describe('XYChart - State tooltips', () => { let store: Store; diff --git a/src/chart_types/xy_chart/state/selectors/get_tooltip_snap.ts b/src/chart_types/xy_chart/state/selectors/get_tooltip_snap.ts index 534ec2b033..366b7a3d7c 100644 --- a/src/chart_types/xy_chart/state/selectors/get_tooltip_snap.ts +++ b/src/chart_types/xy_chart/state/selectors/get_tooltip_snap.ts @@ -1,7 +1,6 @@ import createCachedSelector from 're-reselect'; -import { isTooltipProps } from '../../utils/interactions'; import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs'; -import { SettingsSpec } from '../../../../specs/settings'; +import { SettingsSpec, isTooltipProps } from '../../../../specs/settings'; import { DEFAULT_TOOLTIP_SNAP } from '../../../../specs/settings'; import { getChartIdSelector } from '../../../../state/selectors/get_chart_id'; diff --git a/src/chart_types/xy_chart/state/selectors/get_tooltip_type.ts b/src/chart_types/xy_chart/state/selectors/get_tooltip_type.ts index 95a6e0570e..7207474d6e 100644 --- a/src/chart_types/xy_chart/state/selectors/get_tooltip_type.ts +++ b/src/chart_types/xy_chart/state/selectors/get_tooltip_type.ts @@ -1,7 +1,6 @@ import createCachedSelector from 're-reselect'; -import { TooltipType, isTooltipProps, isTooltipType } from '../../utils/interactions'; import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs'; -import { SettingsSpec } from '../../../../specs/settings'; +import { SettingsSpec, TooltipType, isTooltipType, isTooltipProps } from '../../../../specs/settings'; import { getChartIdSelector } from '../../../../state/selectors/get_chart_id'; export const getTooltipTypeSelector = createCachedSelector( @@ -9,16 +8,16 @@ export const getTooltipTypeSelector = createCachedSelector( getTooltipType, )(getChartIdSelector); -function getTooltipType(settings: SettingsSpec): TooltipType { +export function getTooltipType(settings: SettingsSpec): TooltipType | undefined { const { tooltip } = settings; if (tooltip === undefined || tooltip === null) { - return TooltipType.VerticalCursor; + return undefined; } if (isTooltipType(tooltip)) { return tooltip; } if (isTooltipProps(tooltip)) { - return tooltip.type || TooltipType.VerticalCursor; + return tooltip.type || undefined; } - return TooltipType.VerticalCursor; + return undefined; } diff --git a/src/chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms.ts b/src/chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms.ts index 99d50da730..64a7520032 100644 --- a/src/chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms.ts +++ b/src/chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms.ts @@ -1,5 +1,4 @@ import createCachedSelector from 're-reselect'; -import { TooltipValue, isFollowTooltipType, TooltipType, TooltipValueFormatter } from '../../utils/interactions'; import { getProjectedPointerPositionSelector } from './get_projected_pointer_position'; import { Point } from '../../../../utils/point'; import { getOrientedProjectedPointerPositionSelector } from './get_oriented_projected_pointer_position'; @@ -12,10 +11,17 @@ import { BasicSeriesSpec, AxisSpec } from '../../utils/specs'; import { Rotation } from '../../../../utils/commons'; import { getTooltipTypeSelector } from './get_tooltip_type'; import { formatTooltip } from '../../tooltip/tooltip'; -import { getTooltipHeaderFormatterSelector } from './get_tooltip_header_formatter'; +import { getTooltipHeaderFormatterSelector } from '../../../../state/selectors/get_tooltip_header_formatter'; import { isPointOnGeometry } from '../../rendering/rendering'; import { GlobalChartState } from '../../../../state/chart_state'; -import { PointerEvent, isPointerOutEvent } from '../../../../specs'; +import { + PointerEvent, + isPointerOutEvent, + TooltipValue, + TooltipType, + TooltipValueFormatter, + isFollowTooltipType, +} from '../../../../specs'; import { isValidPointerOverEvent } from '../../../../utils/events'; import { getChartRotationSelector } from '../../../../state/selectors/get_chart_rotation'; import { getChartIdSelector } from '../../../../state/selectors/get_chart_id'; @@ -68,7 +74,7 @@ function getTooltipAndHighlightFromXValue( hasSingleSeries: boolean, scales: ComputedScales, xMatchingGeoms: IndexedGeometry[], - tooltipType: TooltipType, + tooltipType: TooltipType = TooltipType.VerticalCursor, externalPointerEvent: PointerEvent | null, tooltipHeaderFormatter?: TooltipValueFormatter, ): TooltipAndHighlightedGeoms { diff --git a/src/chart_types/xy_chart/state/selectors/is_tooltip_visible.ts b/src/chart_types/xy_chart/state/selectors/is_tooltip_visible.ts index d9a659b796..e0ecee7527 100644 --- a/src/chart_types/xy_chart/state/selectors/is_tooltip_visible.ts +++ b/src/chart_types/xy_chart/state/selectors/is_tooltip_visible.ts @@ -1,28 +1,21 @@ import createCachedSelector from 're-reselect'; -import { TooltipType, isTooltipType, isTooltipProps } from '../../utils/interactions'; import { Point } from '../../../../utils/point'; import { GlobalChartState, PointerStates } from '../../../../state/chart_state'; import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs'; import { getProjectedPointerPositionSelector } from './get_projected_pointer_position'; import { getTooltipValuesSelector, TooltipData } from './get_tooltip_values_highlighted_geoms'; import { getChartIdSelector } from '../../../../state/selectors/get_chart_id'; +import { getTooltipType } from './get_tooltip_type'; +import { TooltipType } from '../../../../specs'; -const getTooltipType = (state: GlobalChartState): TooltipType | undefined => { - const tooltip = getSettingsSpecSelector(state).tooltip; - if (!tooltip) { - return undefined; - } - if (isTooltipType(tooltip)) { - return tooltip; - } - if (isTooltipProps(tooltip)) { - return tooltip.type; - } +const hasTooltipTypeDefinedSelector = (state: GlobalChartState): TooltipType | undefined => { + return getTooltipType(getSettingsSpecSelector(state)); }; + const getPointerSelector = (state: GlobalChartState) => state.interactions.pointer; export const isTooltipVisibleSelector = createCachedSelector( - [getTooltipType, getPointerSelector, getProjectedPointerPositionSelector, getTooltipValuesSelector], + [hasTooltipTypeDefinedSelector, getPointerSelector, getProjectedPointerPositionSelector, getTooltipValuesSelector], isTooltipVisible, )(getChartIdSelector); diff --git a/src/chart_types/xy_chart/tooltip/tooltip.ts b/src/chart_types/xy_chart/tooltip/tooltip.ts index de8ff0233f..87cb5bf711 100644 --- a/src/chart_types/xy_chart/tooltip/tooltip.ts +++ b/src/chart_types/xy_chart/tooltip/tooltip.ts @@ -1,4 +1,3 @@ -import { TooltipValue } from '../utils/interactions'; import { AxisSpec, BasicSeriesSpec, @@ -10,6 +9,7 @@ import { import { IndexedGeometry, BandedAccessorType } from '../../../utils/geometry'; import { getAccessorFormatLabel } from '../../../utils/accessor'; import { getSeriesLabel } from '../utils/series'; +import { TooltipValue } from '../../../specs'; export interface TooltipLegendValue { y0: any; diff --git a/src/chart_types/xy_chart/utils/interactions.test.ts b/src/chart_types/xy_chart/utils/interactions.test.ts index 355407a2b4..4ae692b32f 100644 --- a/src/chart_types/xy_chart/utils/interactions.test.ts +++ b/src/chart_types/xy_chart/utils/interactions.test.ts @@ -4,11 +4,9 @@ import { areIndexedGeomsEquals, getOrientedXPosition, getOrientedYPosition, - isCrosshairTooltipType, - isFollowTooltipType, - TooltipType, } from './interactions'; import { IndexedGeometry, PointGeometry } from '../../../utils/geometry'; +import { TooltipType, isCrosshairTooltipType, isFollowTooltipType } from '../../../specs'; const seriesStyle = { rect: { diff --git a/src/chart_types/xy_chart/utils/interactions.ts b/src/chart_types/xy_chart/utils/interactions.ts index 2eb1755385..ac57b67b03 100644 --- a/src/chart_types/xy_chart/utils/interactions.ts +++ b/src/chart_types/xy_chart/utils/interactions.ts @@ -1,57 +1,6 @@ -import { $Values } from 'utility-types'; import { Rotation } from '../../../utils/commons'; import { Dimensions } from '../../../utils/dimensions'; 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({ - /** Vertical cursor parallel to x axis */ - VerticalCursor: 'vertical' as 'vertical', - /** Vertical and horizontal cursors */ - Crosshairs: 'cross' as 'cross', - /** Follor the mouse coordinates */ - Follow: 'follow' as 'follow', - /** Hide every tooltip */ - None: 'none' as 'none', -}); - -export type TooltipType = $Values; - -export interface TooltipValue { - /** - * 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; - /** - * 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; /** * Get the cursor position depending on the chart rotation @@ -85,13 +34,6 @@ export function getOrientedYPosition(xPos: number, yPos: number, chartRotation: } } -export function isCrosshairTooltipType(type: TooltipType) { - return type === TooltipType.VerticalCursor || type === TooltipType.Crosshairs; -} -export function isFollowTooltipType(type: TooltipType) { - return type === TooltipType.Follow; -} - export function areIndexedGeometryArraysEquals(arr1: IndexedGeometry[], arr2: IndexedGeometry[]) { if (arr1.length !== arr2.length) { return false; @@ -133,11 +75,3 @@ function areBarEqual(ig1: BarGeometry, ig2: BarGeometry) { ig1.height === ig2.height ); } - -export function isTooltipProps(config: TooltipType | TooltipProps): config is TooltipProps { - return typeof config === 'object'; -} - -export function isTooltipType(config: TooltipType | TooltipProps): config is TooltipType { - return typeof config === 'string'; -} diff --git a/src/components/tooltip/index.tsx b/src/components/tooltip/index.tsx index 8c5d2c3848..29de5cfbcd 100644 --- a/src/components/tooltip/index.tsx +++ b/src/components/tooltip/index.tsx @@ -1,17 +1,17 @@ import classNames from 'classnames'; import React from 'react'; +import { createPortal } from 'react-dom'; import { connect } from 'react-redux'; -import { TooltipValue, TooltipValueFormatter } from '../../chart_types/xy_chart/utils/interactions'; +import { TooltipValueFormatter, TooltipValue } from '../../specs'; import { GlobalChartState, BackwardRef } from '../../state/chart_state'; +import { isInitialized } from '../../state/selectors/is_initialized'; +import { getTooltipHeaderFormatterSelector } from '../../state/selectors/get_tooltip_header_formatter'; import { isTooltipVisibleSelector } from '../../chart_types/xy_chart/state/selectors/is_tooltip_visible'; -import { getTooltipHeaderFormatterSelector } from '../../chart_types/xy_chart/state/selectors/get_tooltip_header_formatter'; import { getTooltipPositionSelector } from '../../chart_types/xy_chart/state/selectors/get_tooltip_position'; import { getTooltipValuesSelector, TooltipData, } from '../../chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms'; -import { isInitialized } from '../../state/selectors/is_initialized'; -import { createPortal } from 'react-dom'; import { getFinalTooltipPosition, TooltipPosition } from '../../chart_types/xy_chart/crosshair/crosshair_utils'; import { isAnnotationTooltipVisibleSelector } from '../../chart_types/xy_chart/state/selectors/is_annotation_tooltip_visible'; diff --git a/src/index.ts b/src/index.ts index fd9581903d..fd39490345 100644 --- a/src/index.ts +++ b/src/index.ts @@ -16,7 +16,6 @@ export { SeriesCollectionValue } from './chart_types/xy_chart/utils/series'; export { ChartTypes } from './chart_types'; export { Datum, Position, Rendering, Rotation } from './utils/commons'; export { TickFormatter } from './chart_types/xy_chart/utils/specs'; -export { TooltipType, TooltipValue, TooltipValueFormatter } from './chart_types/xy_chart/utils/interactions'; export { SeriesIdentifier, XYChartSeriesIdentifier } from './chart_types/xy_chart/utils/series'; export { AnnotationDomainType, diff --git a/src/mocks/specs/specs.ts b/src/mocks/specs/specs.ts index d3b1c6c08f..e8437b427a 100644 --- a/src/mocks/specs/specs.ts +++ b/src/mocks/specs/specs.ts @@ -13,8 +13,7 @@ import { import { getSpecId, getGroupId } from '../../utils/ids'; import { ScaleType } from '../../scales'; import { ChartTypes } from '../../chart_types'; -import { SettingsSpec, SpecTypes } from '../../specs'; -import { TooltipType } from '../../chart_types/xy_chart/utils/interactions'; +import { SettingsSpec, SpecTypes, TooltipType } from '../../specs'; import { LIGHT_THEME } from '../../utils/themes/light_theme'; export class MockSeriesSpec { diff --git a/src/specs/settings.test.tsx b/src/specs/settings.test.tsx index acb3223241..5cab312fd4 100644 --- a/src/specs/settings.test.tsx +++ b/src/specs/settings.test.tsx @@ -2,8 +2,7 @@ import { mount } from 'enzyme'; import * as React from 'react'; import { Position, Rendering, Rotation } from '../utils/commons'; import { DARK_THEME } from '../utils/themes/dark_theme'; -import { TooltipType } from '../chart_types/xy_chart/utils/interactions'; -import { Settings, SettingsSpec } from './settings'; +import { Settings, SettingsSpec, TooltipType } from './settings'; import { PartialTheme } from '../utils/themes/theme'; import { LIGHT_THEME } from '../utils/themes/light_theme'; import { chartStoreReducer, GlobalChartState } from '../state/chart_state'; diff --git a/src/specs/settings.tsx b/src/specs/settings.tsx index 4ed5371545..e982f4749f 100644 --- a/src/specs/settings.tsx +++ b/src/specs/settings.tsx @@ -2,15 +2,15 @@ import { $Values } from 'utility-types'; import { DomainRange } from '../chart_types/xy_chart/utils/specs'; import { PartialTheme, Theme } from '../utils/themes/theme'; import { Domain } from '../utils/domain'; -import { TooltipType, TooltipValueFormatter } from '../chart_types/xy_chart/utils/interactions'; import { getConnect, specComponentFactory } from '../state/spec_factory'; import { Spec } from '.'; import { LIGHT_THEME } from '../utils/themes/light_theme'; import { ChartTypes } from '../chart_types'; import { GeometryValue } from '../utils/geometry'; -import { XYChartSeriesIdentifier } from '../chart_types/xy_chart/utils/series'; +import { XYChartSeriesIdentifier, SeriesIdentifier } from '../chart_types/xy_chart/utils/series'; import { Position, Rendering, Rotation } from '../utils/commons'; import { ScaleContinuousType, ScaleOrdinalType } from '../scales'; +import { Accessor } from '../utils/accessor'; export type ElementClickListener = (elements: Array<[GeometryValue, XYChartSeriesIdentifier]>) => void; export type ElementOverListener = (elements: Array<[GeometryValue, XYChartSeriesIdentifier]>) => void; @@ -57,6 +57,53 @@ export interface PointerOutEvent extends BasePointerEvent { export type PointerEvent = PointerOverEvent | PointerOutEvent; +/** The type of tooltip to use */ +export const TooltipType = Object.freeze({ + /** Vertical cursor parallel to x axis */ + VerticalCursor: 'vertical' as 'vertical', + /** Vertical and horizontal cursors */ + Crosshairs: 'cross' as 'cross', + /** Follor the mouse coordinates */ + Follow: 'follow' as 'follow', + /** Hide every tooltip */ + None: 'none' as 'none', +}); + +export type TooltipType = $Values; + +export interface TooltipValue { + /** + * 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; + /** + * 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; + export interface TooltipProps { type?: TooltipType; snap?: boolean; @@ -176,3 +223,19 @@ export function isPointerOutEvent(event: PointerEvent | null | undefined): event export function isPointerOverEvent(event: PointerEvent | null | undefined): event is PointerOverEvent { return event !== null && event !== undefined && event.type === PointerEventType.Over; } + +export function isTooltipProps(config: TooltipType | TooltipProps): config is TooltipProps { + return typeof config === 'object'; +} + +export function isTooltipType(config: TooltipType | TooltipProps): config is TooltipType { + return typeof config === 'string'; +} + +export function isCrosshairTooltipType(type: TooltipType) { + return type === TooltipType.VerticalCursor || type === TooltipType.Crosshairs; +} + +export function isFollowTooltipType(type: TooltipType) { + return type === TooltipType.Follow; +} diff --git a/src/chart_types/xy_chart/state/selectors/get_tooltip_header_formatter.ts b/src/state/selectors/get_tooltip_header_formatter.ts similarity index 58% rename from src/chart_types/xy_chart/state/selectors/get_tooltip_header_formatter.ts rename to src/state/selectors/get_tooltip_header_formatter.ts index 5f9f0cfff8..2dafa8bddd 100644 --- a/src/chart_types/xy_chart/state/selectors/get_tooltip_header_formatter.ts +++ b/src/state/selectors/get_tooltip_header_formatter.ts @@ -1,8 +1,7 @@ import createCachedSelector from 're-reselect'; -import { isTooltipProps, TooltipValueFormatter } from '../../utils/interactions'; -import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs'; -import { SettingsSpec } from '../../../../specs/settings'; -import { getChartIdSelector } from '../../../../state/selectors/get_chart_id'; +import { getSettingsSpecSelector } from './get_settings_specs'; +import { SettingsSpec, TooltipValueFormatter, isTooltipProps } from '../../specs/settings'; +import { getChartIdSelector } from './get_chart_id'; export const getTooltipHeaderFormatterSelector = createCachedSelector( [getSettingsSpecSelector], From 0fd00422eabc2111f10c967ca578f293ca5706bd Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Tue, 18 Feb 2020 11:00:09 +0100 Subject: [PATCH 10/17] refactor: move annotation vis check inside tooltip vis check --- .../xy_chart/state/selectors/is_tooltip_visible.ts | 13 +++++++++++-- src/components/tooltip/index.tsx | 10 +++------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/chart_types/xy_chart/state/selectors/is_tooltip_visible.ts b/src/chart_types/xy_chart/state/selectors/is_tooltip_visible.ts index e0ecee7527..2032dfd6c1 100644 --- a/src/chart_types/xy_chart/state/selectors/is_tooltip_visible.ts +++ b/src/chart_types/xy_chart/state/selectors/is_tooltip_visible.ts @@ -7,6 +7,7 @@ import { getTooltipValuesSelector, TooltipData } from './get_tooltip_values_high import { getChartIdSelector } from '../../../../state/selectors/get_chart_id'; import { getTooltipType } from './get_tooltip_type'; import { TooltipType } from '../../../../specs'; +import { isAnnotationTooltipVisibleSelector } from './is_annotation_tooltip_visible'; const hasTooltipTypeDefinedSelector = (state: GlobalChartState): TooltipType | undefined => { return getTooltipType(getSettingsSpecSelector(state)); @@ -15,7 +16,13 @@ const hasTooltipTypeDefinedSelector = (state: GlobalChartState): TooltipType | u const getPointerSelector = (state: GlobalChartState) => state.interactions.pointer; export const isTooltipVisibleSelector = createCachedSelector( - [hasTooltipTypeDefinedSelector, getPointerSelector, getProjectedPointerPositionSelector, getTooltipValuesSelector], + [ + hasTooltipTypeDefinedSelector, + getPointerSelector, + getProjectedPointerPositionSelector, + getTooltipValuesSelector, + isAnnotationTooltipVisibleSelector, + ], isTooltipVisible, )(getChartIdSelector); @@ -24,12 +31,14 @@ function isTooltipVisible( pointer: PointerStates, projectedPointerPosition: Point, tooltip: TooltipData, + isAnnotationTooltipVisible: boolean, ) { return ( tooltipType !== TooltipType.None && pointer.down === null && projectedPointerPosition.x > -1 && projectedPointerPosition.y > -1 && - tooltip.values.length > 0 + tooltip.values.length > 0 && + !isAnnotationTooltipVisible ); } diff --git a/src/components/tooltip/index.tsx b/src/components/tooltip/index.tsx index 29de5cfbcd..c5eeadddc8 100644 --- a/src/components/tooltip/index.tsx +++ b/src/components/tooltip/index.tsx @@ -13,11 +13,9 @@ import { TooltipData, } from '../../chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms'; import { getFinalTooltipPosition, TooltipPosition } from '../../chart_types/xy_chart/crosshair/crosshair_utils'; -import { isAnnotationTooltipVisibleSelector } from '../../chart_types/xy_chart/state/selectors/is_annotation_tooltip_visible'; interface TooltipStateProps { isTooltipVisible: boolean; - isAnnotationTooltipVisible: boolean; tooltip: TooltipData; tooltipPosition: TooltipPosition | null; tooltipHeaderFormatter?: TooltipValueFormatter; @@ -28,7 +26,7 @@ interface TooltipOwnProps { type TooltipProps = TooltipStateProps & TooltipOwnProps; class TooltipComponent extends React.Component { - static displayName = 'Tooltips'; + static displayName = 'Tooltip'; portalNode: HTMLDivElement | null = null; tooltipRef: React.RefObject; @@ -86,14 +84,14 @@ class TooltipComponent extends React.Component { } render() { - const { isTooltipVisible, tooltip, tooltipHeaderFormatter, isAnnotationTooltipVisible } = this.props; + const { isTooltipVisible, tooltip, tooltipHeaderFormatter } = this.props; if (!this.portalNode) { return null; } const { getChartContainerRef } = this.props; const chartContainerRef = getChartContainerRef(); let tooltipComponent; - if (chartContainerRef.current === null || !isTooltipVisible || isAnnotationTooltipVisible) { + if (chartContainerRef.current === null || !isTooltipVisible) { return null; } else { tooltipComponent = ( @@ -135,7 +133,6 @@ const mapStateToProps = (state: GlobalChartState): TooltipStateProps => { if (!isInitialized(state)) { return { isTooltipVisible: false, - isAnnotationTooltipVisible: false, tooltip: { header: null, values: [], @@ -146,7 +143,6 @@ const mapStateToProps = (state: GlobalChartState): TooltipStateProps => { } return { isTooltipVisible: isTooltipVisibleSelector(state), - isAnnotationTooltipVisible: isAnnotationTooltipVisibleSelector(state), tooltip: getTooltipValuesSelector(state), tooltipPosition: getTooltipPositionSelector(state), tooltipHeaderFormatter: getTooltipHeaderFormatterSelector(state), From 6f34ca7172c01dc206ff54453b41425048258e62 Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Tue, 18 Feb 2020 11:52:42 +0100 Subject: [PATCH 11/17] refactor: move TooltipData type near to Tooltip component --- .../state/selectors/get_annotation_tooltip_state.ts | 3 ++- .../selectors/get_tooltip_values_highlighted_geoms.ts | 5 +---- .../xy_chart/state/selectors/is_tooltip_visible.ts | 3 ++- src/components/tooltip/index.tsx | 6 ++---- src/components/tooltip/types.ts | 6 ++++++ stories/interactions.tsx | 8 ++++---- 6 files changed, 17 insertions(+), 14 deletions(-) create mode 100644 src/components/tooltip/types.ts diff --git a/src/chart_types/xy_chart/state/selectors/get_annotation_tooltip_state.ts b/src/chart_types/xy_chart/state/selectors/get_annotation_tooltip_state.ts index d3dde59ab8..56c7e044a6 100644 --- a/src/chart_types/xy_chart/state/selectors/get_annotation_tooltip_state.ts +++ b/src/chart_types/xy_chart/state/selectors/get_annotation_tooltip_state.ts @@ -15,9 +15,10 @@ import { getChartRotationSelector } from '../../../../state/selectors/get_chart_ import { AnnotationId } from '../../../../utils/ids'; import { computeSeriesGeometriesSelector } from './compute_series_geometries'; import { ComputedGeometries } from '../utils'; -import { getTooltipValuesSelector, TooltipData } from './get_tooltip_values_highlighted_geoms'; +import { getTooltipValuesSelector } from './get_tooltip_values_highlighted_geoms'; import { getChartIdSelector } from '../../../../state/selectors/get_chart_id'; import { GlobalChartState } from '../../../../state/chart_state'; +import { TooltipData } from '../../../../components/tooltip/types'; const getCurrentPointerPosition = (state: GlobalChartState) => state.interactions.pointer.current.position; diff --git a/src/chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms.ts b/src/chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms.ts index 64a7520032..5353e96d29 100644 --- a/src/chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms.ts +++ b/src/chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms.ts @@ -26,6 +26,7 @@ import { isValidPointerOverEvent } from '../../../../utils/events'; import { getChartRotationSelector } from '../../../../state/selectors/get_chart_rotation'; import { getChartIdSelector } from '../../../../state/selectors/get_chart_id'; import { hasSingleSeriesSelector } from './has_single_series'; +import { TooltipData } from '../../../../components/tooltip/types'; const EMPTY_VALUES = Object.freeze({ tooltip: { @@ -35,10 +36,6 @@ const EMPTY_VALUES = Object.freeze({ highlightedGeometries: [], }); -export interface TooltipData { - header: TooltipValue | null; - values: TooltipValue[]; -} export interface TooltipAndHighlightedGeoms { tooltip: TooltipData; highlightedGeometries: IndexedGeometry[]; diff --git a/src/chart_types/xy_chart/state/selectors/is_tooltip_visible.ts b/src/chart_types/xy_chart/state/selectors/is_tooltip_visible.ts index 2032dfd6c1..3b87f19c6c 100644 --- a/src/chart_types/xy_chart/state/selectors/is_tooltip_visible.ts +++ b/src/chart_types/xy_chart/state/selectors/is_tooltip_visible.ts @@ -3,11 +3,12 @@ import { Point } from '../../../../utils/point'; import { GlobalChartState, PointerStates } from '../../../../state/chart_state'; import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs'; import { getProjectedPointerPositionSelector } from './get_projected_pointer_position'; -import { getTooltipValuesSelector, TooltipData } from './get_tooltip_values_highlighted_geoms'; +import { getTooltipValuesSelector } from './get_tooltip_values_highlighted_geoms'; import { getChartIdSelector } from '../../../../state/selectors/get_chart_id'; import { getTooltipType } from './get_tooltip_type'; import { TooltipType } from '../../../../specs'; import { isAnnotationTooltipVisibleSelector } from './is_annotation_tooltip_visible'; +import { TooltipData } from '../../../../components/tooltip/types'; const hasTooltipTypeDefinedSelector = (state: GlobalChartState): TooltipType | undefined => { return getTooltipType(getSettingsSpecSelector(state)); diff --git a/src/components/tooltip/index.tsx b/src/components/tooltip/index.tsx index c5eeadddc8..384be01379 100644 --- a/src/components/tooltip/index.tsx +++ b/src/components/tooltip/index.tsx @@ -8,11 +8,9 @@ import { isInitialized } from '../../state/selectors/is_initialized'; import { getTooltipHeaderFormatterSelector } from '../../state/selectors/get_tooltip_header_formatter'; import { isTooltipVisibleSelector } from '../../chart_types/xy_chart/state/selectors/is_tooltip_visible'; import { getTooltipPositionSelector } from '../../chart_types/xy_chart/state/selectors/get_tooltip_position'; -import { - getTooltipValuesSelector, - TooltipData, -} from '../../chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms'; +import { getTooltipValuesSelector } from '../../chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms'; import { getFinalTooltipPosition, TooltipPosition } from '../../chart_types/xy_chart/crosshair/crosshair_utils'; +import { TooltipData } from './types'; interface TooltipStateProps { isTooltipVisible: boolean; diff --git a/src/components/tooltip/types.ts b/src/components/tooltip/types.ts new file mode 100644 index 0000000000..d283c5a5d5 --- /dev/null +++ b/src/components/tooltip/types.ts @@ -0,0 +1,6 @@ +import { TooltipValue } from '../../specs'; + +export interface TooltipData { + header: TooltipValue | null; + values: TooltipValue[]; +} diff --git a/stories/interactions.tsx b/stories/interactions.tsx index a87b026171..97c69c1b66 100644 --- a/stories/interactions.tsx +++ b/stories/interactions.tsx @@ -57,17 +57,17 @@ export default { }; export const barClicksAndHovers = () => { - const headerFormatter: TooltipValueFormatter = (tooltipData: TooltipValue) => { - if (tooltipData.value % 2 === 0) { + const headerFormatter: TooltipValueFormatter = (tooltip: TooltipValue) => { + if (tooltip.value % 2 === 0) { return (

special header for even x values

-

{tooltipData.value}

+

{tooltip.value}

); } - return tooltipData.value; + return tooltip.value; }; const tooltipProps = { From 126532b6ee1be6c7921687fad83b68b50cfcd878 Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Tue, 18 Feb 2020 12:13:45 +0100 Subject: [PATCH 12/17] refactor: move isTooltipVisible to internal state --- .../partition_chart/state/chart_state.tsx | 3 +++ src/chart_types/xy_chart/state/chart_state.tsx | 18 +++++++++++------- src/components/tooltip/index.tsx | 4 ++-- src/state/chart_state.ts | 2 ++ .../get_internal_is_tooltip_visible.ts | 9 +++++++++ 5 files changed, 27 insertions(+), 9 deletions(-) create mode 100644 src/state/selectors/get_internal_is_tooltip_visible.ts diff --git a/src/chart_types/partition_chart/state/chart_state.tsx b/src/chart_types/partition_chart/state/chart_state.tsx index efcd81e594..426f72ad37 100644 --- a/src/chart_types/partition_chart/state/chart_state.tsx +++ b/src/chart_types/partition_chart/state/chart_state.tsx @@ -27,4 +27,7 @@ export class PartitionState implements InternalChartState { getPointerCursor() { return 'default'; } + isTooltipVisible() { + return false; + } } diff --git a/src/chart_types/xy_chart/state/chart_state.tsx b/src/chart_types/xy_chart/state/chart_state.tsx index ef88a90609..53c267a4f5 100644 --- a/src/chart_types/xy_chart/state/chart_state.tsx +++ b/src/chart_types/xy_chart/state/chart_state.tsx @@ -1,20 +1,21 @@ import React, { RefObject } from 'react'; -import { InternalChartState, GlobalChartState, BackwardRef } from '../../../state/chart_state'; -import { ChartTypes } from '../..'; -import { Tooltip } from '../../../components/tooltip'; -import { htmlIdGenerator } from '../../../utils/commons'; +import { XYChart } from '../renderer/canvas/xy_chart'; import { Highlighter } from '../renderer/dom/highlighter'; import { Crosshair } from '../renderer/dom/crosshair'; +import { BrushTool } from '../renderer/dom/brush'; +import { InternalChartState, GlobalChartState, BackwardRef } from '../../../state/chart_state'; +import { TooltipLegendValue } from '../tooltip/tooltip'; +import { ChartTypes } from '../..'; import { AnnotationTooltip } from '../renderer/dom/annotation_tooltips'; import { isBrushAvailableSelector } from './selectors/is_brush_available'; -import { BrushTool } from '../renderer/dom/brush'; import { isChartEmptySelector } from './selectors/is_chart_empty'; import { computeLegendSelector } from './selectors/compute_legend'; import { getLegendTooltipValuesSelector } from './selectors/get_legend_tooltip_values'; -import { TooltipLegendValue } from '../tooltip/tooltip'; import { getPointerCursorSelector } from './selectors/get_cursor_pointer'; import { isBrushingSelector } from './selectors/is_brushing'; -import { XYChart } from '../renderer/canvas/xy_chart'; +import { isTooltipVisibleSelector } from './selectors/is_tooltip_visible'; +import { htmlIdGenerator } from '../../../utils/commons'; +import { Tooltip } from '../../../components/tooltip'; export class XYAxisChartState implements InternalChartState { chartType = ChartTypes.XYAxis; @@ -49,4 +50,7 @@ export class XYAxisChartState implements InternalChartState { getPointerCursor(globalState: GlobalChartState) { return getPointerCursorSelector(globalState); } + isTooltipVisible(globalState: GlobalChartState) { + return isTooltipVisibleSelector(globalState); + } } diff --git a/src/components/tooltip/index.tsx b/src/components/tooltip/index.tsx index 384be01379..b170a87a35 100644 --- a/src/components/tooltip/index.tsx +++ b/src/components/tooltip/index.tsx @@ -5,8 +5,8 @@ import { connect } from 'react-redux'; import { TooltipValueFormatter, TooltipValue } from '../../specs'; import { GlobalChartState, BackwardRef } from '../../state/chart_state'; import { isInitialized } from '../../state/selectors/is_initialized'; +import { getInternalIsTooltipVisibleSelector } from '../../state/selectors/get_internal_is_tooltip_visible'; import { getTooltipHeaderFormatterSelector } from '../../state/selectors/get_tooltip_header_formatter'; -import { isTooltipVisibleSelector } from '../../chart_types/xy_chart/state/selectors/is_tooltip_visible'; import { getTooltipPositionSelector } from '../../chart_types/xy_chart/state/selectors/get_tooltip_position'; import { getTooltipValuesSelector } from '../../chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms'; import { getFinalTooltipPosition, TooltipPosition } from '../../chart_types/xy_chart/crosshair/crosshair_utils'; @@ -140,7 +140,7 @@ const mapStateToProps = (state: GlobalChartState): TooltipStateProps => { }; } return { - isTooltipVisible: isTooltipVisibleSelector(state), + isTooltipVisible: getInternalIsTooltipVisibleSelector(state), tooltip: getTooltipValuesSelector(state), tooltipPosition: getTooltipPositionSelector(state), tooltipHeaderFormatter: getTooltipHeaderFormatterSelector(state), diff --git a/src/state/chart_state.ts b/src/state/chart_state.ts index 6b536f7cb3..45788266d6 100644 --- a/src/state/chart_state.ts +++ b/src/state/chart_state.ts @@ -39,6 +39,8 @@ export interface InternalChartState { getLegendItemsValues(globalState: GlobalChartState): Map; // return the CSS pointer cursor depending on the internal chart state getPointerCursor(globalState: GlobalChartState): string; + // true if the tooltip is visible, false otherwise + isTooltipVisible(globalState: GlobalChartState): boolean; } export interface SpecList { diff --git a/src/state/selectors/get_internal_is_tooltip_visible.ts b/src/state/selectors/get_internal_is_tooltip_visible.ts new file mode 100644 index 0000000000..a6dbc06f77 --- /dev/null +++ b/src/state/selectors/get_internal_is_tooltip_visible.ts @@ -0,0 +1,9 @@ +import { GlobalChartState } from '../chart_state'; + +export const getInternalIsTooltipVisibleSelector = (state: GlobalChartState): boolean => { + if (state.internalChartState) { + return state.internalChartState.isTooltipVisible(state); + } else { + return false; + } +}; From ad1033a609a4b29b2931d76c1f2935831c476686 Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Tue, 18 Feb 2020 12:23:10 +0100 Subject: [PATCH 13/17] refactor: cleanup TooltipStateProps --- .../selectors/get_annotation_tooltip_state.ts | 4 +- .../get_tooltip_values_highlighted_geoms.ts | 6 +- .../state/selectors/is_tooltip_visible.ts | 4 +- src/components/tooltip/index.tsx | 100 ++++++++---------- src/components/tooltip/types.ts | 2 +- 5 files changed, 54 insertions(+), 62 deletions(-) diff --git a/src/chart_types/xy_chart/state/selectors/get_annotation_tooltip_state.ts b/src/chart_types/xy_chart/state/selectors/get_annotation_tooltip_state.ts index 56c7e044a6..d4fab8bacf 100644 --- a/src/chart_types/xy_chart/state/selectors/get_annotation_tooltip_state.ts +++ b/src/chart_types/xy_chart/state/selectors/get_annotation_tooltip_state.ts @@ -18,7 +18,7 @@ import { ComputedGeometries } from '../utils'; import { getTooltipValuesSelector } from './get_tooltip_values_highlighted_geoms'; import { getChartIdSelector } from '../../../../state/selectors/get_chart_id'; import { GlobalChartState } from '../../../../state/chart_state'; -import { TooltipData } from '../../../../components/tooltip/types'; +import { TooltipInfo } from '../../../../components/tooltip/types'; const getCurrentPointerPosition = (state: GlobalChartState) => state.interactions.pointer.current.position; @@ -48,7 +48,7 @@ function getAnnotationTooltipState( annotationSpecs: AnnotationSpec[], axesSpecs: AxisSpec[], annotationDimensions: Map, - tooltip: TooltipData, + tooltip: TooltipInfo, ): AnnotationTooltipState | null { // get positions relative to chart if (x < 0 || y < 0) { diff --git a/src/chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms.ts b/src/chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms.ts index 5353e96d29..a210d1cb09 100644 --- a/src/chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms.ts +++ b/src/chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms.ts @@ -26,7 +26,7 @@ import { isValidPointerOverEvent } from '../../../../utils/events'; import { getChartRotationSelector } from '../../../../state/selectors/get_chart_rotation'; import { getChartIdSelector } from '../../../../state/selectors/get_chart_id'; import { hasSingleSeriesSelector } from './has_single_series'; -import { TooltipData } from '../../../../components/tooltip/types'; +import { TooltipInfo } from '../../../../components/tooltip/types'; const EMPTY_VALUES = Object.freeze({ tooltip: { @@ -37,7 +37,7 @@ const EMPTY_VALUES = Object.freeze({ }); export interface TooltipAndHighlightedGeoms { - tooltip: TooltipData; + tooltip: TooltipInfo; highlightedGeometries: IndexedGeometry[]; } @@ -166,7 +166,7 @@ function getTooltipAndHighlightFromXValue( export const getTooltipValuesSelector = createCachedSelector( [getTooltipValuesAndGeometriesSelector], - ({ tooltip }): TooltipData => { + ({ tooltip }): TooltipInfo => { return tooltip; }, )(getChartIdSelector); diff --git a/src/chart_types/xy_chart/state/selectors/is_tooltip_visible.ts b/src/chart_types/xy_chart/state/selectors/is_tooltip_visible.ts index 3b87f19c6c..3452cc9b80 100644 --- a/src/chart_types/xy_chart/state/selectors/is_tooltip_visible.ts +++ b/src/chart_types/xy_chart/state/selectors/is_tooltip_visible.ts @@ -8,7 +8,7 @@ import { getChartIdSelector } from '../../../../state/selectors/get_chart_id'; import { getTooltipType } from './get_tooltip_type'; import { TooltipType } from '../../../../specs'; import { isAnnotationTooltipVisibleSelector } from './is_annotation_tooltip_visible'; -import { TooltipData } from '../../../../components/tooltip/types'; +import { TooltipInfo } from '../../../../components/tooltip/types'; const hasTooltipTypeDefinedSelector = (state: GlobalChartState): TooltipType | undefined => { return getTooltipType(getSettingsSpecSelector(state)); @@ -31,7 +31,7 @@ function isTooltipVisible( tooltipType: TooltipType | undefined, pointer: PointerStates, projectedPointerPosition: Point, - tooltip: TooltipData, + tooltip: TooltipInfo, isAnnotationTooltipVisible: boolean, ) { return ( diff --git a/src/components/tooltip/index.tsx b/src/components/tooltip/index.tsx index b170a87a35..b739c7459a 100644 --- a/src/components/tooltip/index.tsx +++ b/src/components/tooltip/index.tsx @@ -10,13 +10,13 @@ import { getTooltipHeaderFormatterSelector } from '../../state/selectors/get_too import { getTooltipPositionSelector } from '../../chart_types/xy_chart/state/selectors/get_tooltip_position'; import { getTooltipValuesSelector } from '../../chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms'; import { getFinalTooltipPosition, TooltipPosition } from '../../chart_types/xy_chart/crosshair/crosshair_utils'; -import { TooltipData } from './types'; +import { TooltipInfo } from './types'; interface TooltipStateProps { - isTooltipVisible: boolean; - tooltip: TooltipData; - tooltipPosition: TooltipPosition | null; - tooltipHeaderFormatter?: TooltipValueFormatter; + isVisible: boolean; + info: TooltipInfo; + position: TooltipPosition | null; + headerFormatter?: TooltipValueFormatter; } interface TooltipOwnProps { getChartContainerRef: BackwardRef; @@ -48,16 +48,16 @@ class TooltipComponent extends React.Component { componentDidUpdate() { this.createPortalNode(); - const { getChartContainerRef, tooltipPosition } = this.props; + const { getChartContainerRef, position } = this.props; const chartContainerRef = getChartContainerRef(); - if (!this.tooltipRef.current || !chartContainerRef.current || !this.portalNode || !tooltipPosition) { + if (!this.tooltipRef.current || !chartContainerRef.current || !this.portalNode || !position) { return; } const chartContainerBBox = chartContainerRef.current.getBoundingClientRect(); const tooltipBBox = this.tooltipRef.current.getBoundingClientRect(); - const tooltipStyle = getFinalTooltipPosition(chartContainerBBox, tooltipBBox, tooltipPosition); + const tooltipStyle = getFinalTooltipPosition(chartContainerBBox, tooltipBBox, position); if (tooltipStyle.left) { this.portalNode.style.left = tooltipStyle.left; @@ -82,47 +82,39 @@ class TooltipComponent extends React.Component { } render() { - const { isTooltipVisible, tooltip, tooltipHeaderFormatter } = this.props; - if (!this.portalNode) { - return null; - } - const { getChartContainerRef } = this.props; + const { isVisible, info, headerFormatter, getChartContainerRef } = this.props; const chartContainerRef = getChartContainerRef(); - let tooltipComponent; - if (chartContainerRef.current === null || !isTooltipVisible) { + if (!this.portalNode || chartContainerRef.current === null || !isVisible) { return null; - } else { - tooltipComponent = ( -
-
{this.renderHeader(tooltip.header, tooltipHeaderFormatter)}
-
- {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 ( -
- {label} - {value} -
- ); - }, - )} -
-
- ); } + const tooltipComponent = ( +
+
{this.renderHeader(info.header, headerFormatter)}
+
+ {info.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 ( +
+ {label} + {value} +
+ ); + })} +
+
+ ); return createPortal(tooltipComponent, this.portalNode); } } @@ -130,20 +122,20 @@ class TooltipComponent extends React.Component { const mapStateToProps = (state: GlobalChartState): TooltipStateProps => { if (!isInitialized(state)) { return { - isTooltipVisible: false, - tooltip: { + isVisible: false, + info: { header: null, values: [], }, - tooltipPosition: null, - tooltipHeaderFormatter: undefined, + position: null, + headerFormatter: undefined, }; } return { - isTooltipVisible: getInternalIsTooltipVisibleSelector(state), - tooltip: getTooltipValuesSelector(state), - tooltipPosition: getTooltipPositionSelector(state), - tooltipHeaderFormatter: getTooltipHeaderFormatterSelector(state), + isVisible: getInternalIsTooltipVisibleSelector(state), + info: getTooltipValuesSelector(state), + position: getTooltipPositionSelector(state), + headerFormatter: getTooltipHeaderFormatterSelector(state), }; }; diff --git a/src/components/tooltip/types.ts b/src/components/tooltip/types.ts index d283c5a5d5..17b5eb8f68 100644 --- a/src/components/tooltip/types.ts +++ b/src/components/tooltip/types.ts @@ -1,6 +1,6 @@ import { TooltipValue } from '../../specs'; -export interface TooltipData { +export interface TooltipInfo { header: TooltipValue | null; values: TooltipValue[]; } From e39fa4ef5ec11e5f479ebdd4631fc3dde89bfd9b Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Tue, 18 Feb 2020 12:42:07 +0100 Subject: [PATCH 14/17] refactor: move getTooltipValues in the internal state --- .../partition_chart/state/chart_state.tsx | 3 + .../state/chart_state.interactions.test.ts | 102 +++++++++--------- .../state/chart_state.timescales.test.ts | 20 ++-- .../state/chart_state.tooltip.test.ts | 4 +- .../xy_chart/state/chart_state.tsx | 4 + .../selectors/get_annotation_tooltip_state.ts | 4 +- .../selectors/get_legend_tooltip_values.ts | 4 +- .../get_tooltip_values_highlighted_geoms.ts | 20 ++-- .../state/selectors/is_tooltip_visible.ts | 4 +- .../state/selectors/on_element_out_caller.ts | 4 +- .../state/selectors/on_element_over_caller.ts | 4 +- src/components/tooltip/index.tsx | 25 +++-- src/state/chart_state.ts | 51 +++++++-- .../selectors/get_internal_tooltip_info.ts | 10 ++ 14 files changed, 154 insertions(+), 105 deletions(-) create mode 100644 src/state/selectors/get_internal_tooltip_info.ts diff --git a/src/chart_types/partition_chart/state/chart_state.tsx b/src/chart_types/partition_chart/state/chart_state.tsx index 426f72ad37..ff07027c51 100644 --- a/src/chart_types/partition_chart/state/chart_state.tsx +++ b/src/chart_types/partition_chart/state/chart_state.tsx @@ -30,4 +30,7 @@ export class PartitionState implements InternalChartState { isTooltipVisible() { return false; } + getTooltipInfo() { + return undefined; + } } diff --git a/src/chart_types/xy_chart/state/chart_state.interactions.test.ts b/src/chart_types/xy_chart/state/chart_state.interactions.test.ts index 85ff1e2dd1..b9d464ded8 100644 --- a/src/chart_types/xy_chart/state/chart_state.interactions.test.ts +++ b/src/chart_types/xy_chart/state/chart_state.interactions.test.ts @@ -8,7 +8,7 @@ import { computeSeriesGeometriesSelector } from './selectors/compute_series_geom import { getProjectedPointerPositionSelector } from './selectors/get_projected_pointer_position'; import { getHighlightedGeomsSelector, - getTooltipValuesAndGeometriesSelector, + getTooltipInfoAndGeometriesSelector, } from './selectors/get_tooltip_values_highlighted_geoms'; import { isTooltipVisibleSelector } from './selectors/is_tooltip_visible'; import { createOnBrushEndCaller } from './selectors/on_brush_end_caller'; @@ -220,9 +220,9 @@ describe('Chart state pointer interactions', () => { store.dispatch(upsertSpec(updatedSettings)); store.dispatch(specParsed()); store.dispatch(onPointerMove({ x: 10, y: 10 + 70 }, 0)); - const tooltipData = getTooltipValuesAndGeometriesSelector(store.getState()); + const tooltipInfo = getTooltipInfoAndGeometriesSelector(store.getState()); // no tooltip values exist if we have a TooltipType === None - expect(tooltipData.tooltip.values.length).toBe(0); + expect(tooltipInfo.tooltip.values.length).toBe(0); let isTooltipVisible = isTooltipVisibleSelector(store.getState()); expect(isTooltipVisible).toBe(false); @@ -283,8 +283,8 @@ function mouseOverTestSuite(scaleType: ScaleType) { onElementOverCaller(state); onPointerMoveCaller(state); }); - const tooltipData = getTooltipValuesAndGeometriesSelector(store.getState()); - expect(tooltipData.tooltip.values).toEqual([]); + const tooltipInfo = getTooltipInfoAndGeometriesSelector(store.getState()); + expect(tooltipInfo.tooltip.values).toEqual([]); }); test('store is correctly configured', () => { @@ -298,15 +298,15 @@ function mouseOverTestSuite(scaleType: ScaleType) { store.dispatch(onPointerMove({ x: chartLeft + 10, y: chartTop + 10 }, 0)); expect(onPointerUpdateListener).toBeCalledTimes(1); - const tooltipData1 = getTooltipValuesAndGeometriesSelector(store.getState()); - expect(tooltipData1.tooltip.values.length).toBe(1); + const tooltipInfo1 = getTooltipInfoAndGeometriesSelector(store.getState()); + expect(tooltipInfo1.tooltip.values.length).toBe(1); // avoid calls store.dispatch(onPointerMove({ x: chartLeft + 12, y: chartTop + 12 }, 1)); expect(onPointerUpdateListener).toBeCalledTimes(1); - const tooltipData2 = getTooltipValuesAndGeometriesSelector(store.getState()); - expect(tooltipData2.tooltip.values.length).toBe(1); - expect(tooltipData1).toEqual(tooltipData2); + const tooltipInfo2 = getTooltipInfoAndGeometriesSelector(store.getState()); + expect(tooltipInfo2.tooltip.values.length).toBe(1); + expect(tooltipInfo1).toEqual(tooltipInfo2); }); test('call pointer update listener on move', () => { @@ -384,8 +384,8 @@ function mouseOverTestSuite(scaleType: ScaleType) { }); test('can hover top-left corner of the first bar', () => { - let tooltipData = getTooltipValuesAndGeometriesSelector(store.getState()); - expect(tooltipData.tooltip.values).toEqual([]); + let tooltipInfo = getTooltipInfoAndGeometriesSelector(store.getState()); + expect(tooltipInfo.tooltip.values).toEqual([]); store.dispatch(onPointerMove({ x: chartLeft + 0, y: chartTop + 0 }, 0)); let projectedPointerPosition = getProjectedPointerPositionSelector(store.getState()); expect(projectedPointerPosition).toEqual({ x: 0, y: 0 }); @@ -395,9 +395,9 @@ function mouseOverTestSuite(scaleType: ScaleType) { expect(cursorBandPosition!.width).toBe(45); let isTooltipVisible = isTooltipVisibleSelector(store.getState()); expect(isTooltipVisible).toBe(true); - tooltipData = getTooltipValuesAndGeometriesSelector(store.getState()); - expect(tooltipData.tooltip.values.length).toBe(1); - expect(tooltipData.highlightedGeometries.length).toBe(1); + tooltipInfo = getTooltipInfoAndGeometriesSelector(store.getState()); + expect(tooltipInfo.tooltip.values.length).toBe(1); + expect(tooltipInfo.highlightedGeometries.length).toBe(1); expect(onOverListener).toBeCalledTimes(1); expect(onOutListener).toBeCalledTimes(0); expect(onOverListener.mock.calls[0][0]).toEqual([ @@ -422,9 +422,9 @@ function mouseOverTestSuite(scaleType: ScaleType) { expect(projectedPointerPosition).toEqual({ x: -1, y: -1 }); isTooltipVisible = isTooltipVisibleSelector(store.getState()); expect(isTooltipVisible).toBe(false); - tooltipData = getTooltipValuesAndGeometriesSelector(store.getState()); - expect(tooltipData.tooltip.values.length).toBe(0); - expect(tooltipData.highlightedGeometries.length).toBe(0); + tooltipInfo = getTooltipInfoAndGeometriesSelector(store.getState()); + expect(tooltipInfo.tooltip.values.length).toBe(0); + expect(tooltipInfo.highlightedGeometries.length).toBe(0); expect(onOverListener).toBeCalledTimes(1); expect(onOutListener).toBeCalledTimes(1); }); @@ -439,9 +439,9 @@ function mouseOverTestSuite(scaleType: ScaleType) { expect(cursorBandPosition!.width).toBe(45); let isTooltipVisible = isTooltipVisibleSelector(store.getState()); expect(isTooltipVisible).toBe(true); - let tooltipData = getTooltipValuesAndGeometriesSelector(store.getState()); - expect(tooltipData.highlightedGeometries.length).toBe(1); - expect(tooltipData.tooltip.values.length).toBe(1); + let tooltipInfo = getTooltipInfoAndGeometriesSelector(store.getState()); + expect(tooltipInfo.highlightedGeometries.length).toBe(1); + expect(tooltipInfo.tooltip.values.length).toBe(1); expect(onOverListener).toBeCalledTimes(1); expect(onOutListener).toBeCalledTimes(0); expect(onOverListener.mock.calls[0][0]).toEqual([ @@ -465,9 +465,9 @@ function mouseOverTestSuite(scaleType: ScaleType) { expect(projectedPointerPosition).toEqual({ x: -1, y: 89 }); isTooltipVisible = isTooltipVisibleSelector(store.getState()); expect(isTooltipVisible).toBe(false); - tooltipData = getTooltipValuesAndGeometriesSelector(store.getState()); - expect(tooltipData.tooltip.values.length).toBe(0); - expect(tooltipData.highlightedGeometries.length).toBe(0); + tooltipInfo = getTooltipInfoAndGeometriesSelector(store.getState()); + expect(tooltipInfo.tooltip.values.length).toBe(0); + expect(tooltipInfo.highlightedGeometries.length).toBe(0); expect(onOverListener).toBeCalledTimes(1); expect(onOutListener).toBeCalledTimes(1); }); @@ -486,9 +486,9 @@ function mouseOverTestSuite(scaleType: ScaleType) { expect(cursorBandPosition!.width).toBe(45); let isTooltipVisible = isTooltipVisibleSelector(store.getState()); expect(isTooltipVisible).toBe(true); - let tooltipData = getTooltipValuesAndGeometriesSelector(store.getState()); - expect(tooltipData.highlightedGeometries.length).toBe(1); - expect(tooltipData.tooltip.values.length).toBe(1); + let tooltipInfo = getTooltipInfoAndGeometriesSelector(store.getState()); + expect(tooltipInfo.highlightedGeometries.length).toBe(1); + expect(tooltipInfo.tooltip.values.length).toBe(1); expect(onOverListener).toBeCalledTimes(1); expect(onOutListener).toBeCalledTimes(0); expect(onOverListener.mock.calls[0][0]).toEqual([ @@ -517,9 +517,9 @@ function mouseOverTestSuite(scaleType: ScaleType) { expect(cursorBandPosition!.width).toBe(45); isTooltipVisible = isTooltipVisibleSelector(store.getState()); expect(isTooltipVisible).toBe(true); - tooltipData = getTooltipValuesAndGeometriesSelector(store.getState()); - expect(tooltipData.tooltip.values.length).toBe(1); - expect(tooltipData.highlightedGeometries.length).toBe(0); + tooltipInfo = getTooltipInfoAndGeometriesSelector(store.getState()); + expect(tooltipInfo.tooltip.values.length).toBe(1); + expect(tooltipInfo.highlightedGeometries.length).toBe(0); expect(onOverListener).toBeCalledTimes(1); expect(onOutListener).toBeCalledTimes(1); }); @@ -538,9 +538,9 @@ function mouseOverTestSuite(scaleType: ScaleType) { expect(cursorBandPosition!.width).toBe(45); let isTooltipVisible = isTooltipVisibleSelector(store.getState()); expect(isTooltipVisible).toBe(true); - let tooltipData = getTooltipValuesAndGeometriesSelector(store.getState()); - expect(tooltipData.highlightedGeometries.length).toBe(1); - expect(tooltipData.tooltip.values.length).toBe(1); + let tooltipInfo = getTooltipInfoAndGeometriesSelector(store.getState()); + expect(tooltipInfo.highlightedGeometries.length).toBe(1); + expect(tooltipInfo.tooltip.values.length).toBe(1); expect(onOverListener).toBeCalledTimes(1); expect(onOutListener).toBeCalledTimes(0); expect(onOverListener.mock.calls[0][0]).toEqual([ @@ -569,10 +569,10 @@ function mouseOverTestSuite(scaleType: ScaleType) { expect(cursorBandPosition!.width).toBe(45); isTooltipVisible = isTooltipVisibleSelector(store.getState()); expect(isTooltipVisible).toBe(true); - tooltipData = getTooltipValuesAndGeometriesSelector(store.getState()); - expect(tooltipData.tooltip.values.length).toBe(1); + tooltipInfo = getTooltipInfoAndGeometriesSelector(store.getState()); + expect(tooltipInfo.tooltip.values.length).toBe(1); // we are over the second bar here - expect(tooltipData.highlightedGeometries.length).toBe(1); + expect(tooltipInfo.highlightedGeometries.length).toBe(1); expect(onOverListener).toBeCalledTimes(2); expect(onOverListener.mock.calls[1][0]).toEqual([ [ @@ -599,9 +599,9 @@ function mouseOverTestSuite(scaleType: ScaleType) { test('can hover top-right corner of the chart', () => { expect(onOverListener).toBeCalledTimes(0); expect(onOutListener).toBeCalledTimes(0); - let tooltipData = getTooltipValuesAndGeometriesSelector(store.getState()); - expect(tooltipData.highlightedGeometries.length).toBe(0); - expect(tooltipData.tooltip.values.length).toBe(0); + let tooltipInfo = getTooltipInfoAndGeometriesSelector(store.getState()); + expect(tooltipInfo.highlightedGeometries.length).toBe(0); + expect(tooltipInfo.tooltip.values.length).toBe(0); store.dispatch(onPointerMove({ x: chartLeft + 89, y: chartTop + 0 }, 0)); const projectedPointerPosition = getProjectedPointerPositionSelector(store.getState()); @@ -613,9 +613,9 @@ function mouseOverTestSuite(scaleType: ScaleType) { const isTooltipVisible = isTooltipVisibleSelector(store.getState()); expect(isTooltipVisible).toBe(true); - tooltipData = getTooltipValuesAndGeometriesSelector(store.getState()); - expect(tooltipData.highlightedGeometries.length).toBe(0); - expect(tooltipData.tooltip.values.length).toBe(1); + tooltipInfo = getTooltipInfoAndGeometriesSelector(store.getState()); + expect(tooltipInfo.highlightedGeometries.length).toBe(0); + expect(tooltipInfo.tooltip.values.length).toBe(1); expect(onOverListener).toBeCalledTimes(0); expect(onOutListener).toBeCalledTimes(0); }); @@ -665,9 +665,9 @@ function mouseOverTestSuite(scaleType: ScaleType) { expect(cursorBandPosition!.width).toBe(45); const isTooltipVisible = isTooltipVisibleSelector(store.getState()); expect(isTooltipVisible).toBe(true); - const tooltipData = getTooltipValuesAndGeometriesSelector(store.getState()); - expect(tooltipData.highlightedGeometries.length).toBe(1); - expect(tooltipData.tooltip.values.length).toBe(1); + const tooltipInfo = getTooltipInfoAndGeometriesSelector(store.getState()); + expect(tooltipInfo.highlightedGeometries.length).toBe(1); + expect(tooltipInfo.tooltip.values.length).toBe(1); expect(onOverListener).toBeCalledTimes(1); expect(onOverListener.mock.calls[0][0]).toEqual([ [ @@ -743,9 +743,9 @@ function mouseOverTestSuite(scaleType: ScaleType) { }); test('chart 0 rotation', () => { store.dispatch(onPointerMove({ x: chartLeft + 0, y: chartTop + 89 }, 0)); - const tooltipData = getTooltipValuesAndGeometriesSelector(store.getState()); - expect(tooltipData.tooltip.header?.value).toBe('bottom 0'); - expect(tooltipData.tooltip.values[0].value).toBe('left 10'); + const tooltipInfo = getTooltipInfoAndGeometriesSelector(store.getState()); + expect(tooltipInfo.tooltip.header?.value).toBe('bottom 0'); + expect(tooltipInfo.tooltip.values[0].value).toBe('left 10'); }); test('chart 90 deg rotated', () => { @@ -757,9 +757,9 @@ function mouseOverTestSuite(scaleType: ScaleType) { store.dispatch(upsertSpec(updatedSettings)); store.dispatch(specParsed()); store.dispatch(onPointerMove({ x: chartLeft + 0, y: chartTop + 89 }, 0)); - const tooltipData = getTooltipValuesAndGeometriesSelector(store.getState()); - expect(tooltipData.tooltip.header?.value).toBe('left 1'); - expect(tooltipData.tooltip.values[0].value).toBe('bottom 5'); + const tooltipInfo = getTooltipInfoAndGeometriesSelector(store.getState()); + expect(tooltipInfo.tooltip.header?.value).toBe('left 1'); + expect(tooltipInfo.tooltip.values[0].value).toBe('bottom 5'); }); }); describe('brush', () => { diff --git a/src/chart_types/xy_chart/state/chart_state.timescales.test.ts b/src/chart_types/xy_chart/state/chart_state.timescales.test.ts index d6c0540ed5..7565f59546 100644 --- a/src/chart_types/xy_chart/state/chart_state.timescales.test.ts +++ b/src/chart_types/xy_chart/state/chart_state.timescales.test.ts @@ -9,7 +9,7 @@ import { LIGHT_THEME } from '../../../utils/themes/light_theme'; import { updateParentDimensions } from '../../../state/actions/chart_settings'; import { computeSeriesGeometriesSelector } from './selectors/compute_series_geometries'; import { onPointerMove } from '../../../state/actions/mouse'; -import { getTooltipValuesSelector } from './selectors/get_tooltip_values_highlighted_geoms'; +import { getTooltipInfoSelector } from './selectors/get_tooltip_values_highlighted_geoms'; import { DateTime } from 'luxon'; import { getComputedScalesSelector } from './selectors/get_computed_scales'; import { ChartTypes } from '../..'; @@ -69,17 +69,17 @@ describe('Render chart', () => { }); test('check mouse position correctly return inverted value', () => { store.dispatch(onPointerMove({ x: 15, y: 10 }, 0)); // check first valid tooltip - let tooltip = getTooltipValuesSelector(store.getState()); + let tooltip = getTooltipInfoSelector(store.getState()); expect(tooltip.values.length).toBe(1); expect(tooltip.header?.value).toBe(day1); expect(tooltip.values[0].value).toBe(10); store.dispatch(onPointerMove({ x: 35, y: 10 }, 1)); // check second valid tooltip - tooltip = getTooltipValuesSelector(store.getState()); + tooltip = getTooltipInfoSelector(store.getState()); expect(tooltip.values.length).toBe(1); expect(tooltip.header?.value).toBe(day2); expect(tooltip.values[0].value).toBe(22); store.dispatch(onPointerMove({ x: 76, y: 10 }, 2)); // check third valid tooltip - tooltip = getTooltipValuesSelector(store.getState()); + tooltip = getTooltipInfoSelector(store.getState()); expect(tooltip.values.length).toBe(1); expect(tooltip.header?.value).toBe(day3); expect(tooltip.values[0].value).toBe(6); @@ -138,17 +138,17 @@ describe('Render chart', () => { }); test('check mouse position correctly return inverted value', () => { store.dispatch(onPointerMove({ x: 15, y: 10 }, 0)); // check first valid tooltip - let tooltip = getTooltipValuesSelector(store.getState()); + let tooltip = getTooltipInfoSelector(store.getState()); expect(tooltip.values.length).toBe(1); expect(tooltip.header?.value).toBe(date1); expect(tooltip.values[0].value).toBe(10); store.dispatch(onPointerMove({ x: 35, y: 10 }, 1)); // check second valid tooltip - tooltip = getTooltipValuesSelector(store.getState()); + tooltip = getTooltipInfoSelector(store.getState()); expect(tooltip.values.length).toBe(1); expect(tooltip.header?.value).toBe(date2); expect(tooltip.values[0].value).toBe(22); store.dispatch(onPointerMove({ x: 76, y: 10 }, 2)); // check third valid tooltip - tooltip = getTooltipValuesSelector(store.getState()); + tooltip = getTooltipInfoSelector(store.getState()); expect(tooltip.values.length).toBe(1); expect(tooltip.header?.value).toBe(date3); expect(tooltip.values[0].value).toBe(6); @@ -225,17 +225,17 @@ describe('Render chart', () => { }); test('check mouse position correctly return inverted value', () => { store.dispatch(onPointerMove({ x: 15, y: 10 }, 0)); // check first valid tooltip - let tooltip = getTooltipValuesSelector(store.getState()); + let tooltip = getTooltipInfoSelector(store.getState()); expect(tooltip.values.length).toBe(1); expect(tooltip.header?.value).toBe(date1); expect(tooltip.values[0].value).toBe(10); store.dispatch(onPointerMove({ x: 35, y: 10 }, 1)); // check second valid tooltip - tooltip = getTooltipValuesSelector(store.getState()); + tooltip = getTooltipInfoSelector(store.getState()); expect(tooltip.values.length).toBe(1); expect(tooltip.header?.value).toBe(date2); expect(tooltip.values[0].value).toBe(22); store.dispatch(onPointerMove({ x: 76, y: 10 }, 2)); // check third valid tooltip - tooltip = getTooltipValuesSelector(store.getState()); + tooltip = getTooltipInfoSelector(store.getState()); expect(tooltip.values.length).toBe(1); expect(tooltip.header?.value).toBe(date3); expect(tooltip.values[0].value).toBe(6); diff --git a/src/chart_types/xy_chart/state/chart_state.tooltip.test.ts b/src/chart_types/xy_chart/state/chart_state.tooltip.test.ts index 3ebab7d57b..29938e954e 100644 --- a/src/chart_types/xy_chart/state/chart_state.tooltip.test.ts +++ b/src/chart_types/xy_chart/state/chart_state.tooltip.test.ts @@ -3,7 +3,7 @@ import { createStore, Store } from 'redux'; import { upsertSpec, specParsed } from '../../../state/actions/specs'; import { MockSeriesSpec, MockGlobalSpec } from '../../../mocks/specs'; import { updateParentDimensions } from '../../../state/actions/chart_settings'; -import { getTooltipValuesAndGeometriesSelector } from './selectors/get_tooltip_values_highlighted_geoms'; +import { getTooltipInfoAndGeometriesSelector } from './selectors/get_tooltip_values_highlighted_geoms'; import { onPointerMove } from '../../../state/actions/mouse'; import { TooltipType } from '../../../specs'; @@ -46,7 +46,7 @@ describe('XYChart - State tooltips', () => { ); store.dispatch(specParsed()); const state = store.getState(); - const tooltipValues = getTooltipValuesAndGeometriesSelector(state); + const tooltipValues = getTooltipInfoAndGeometriesSelector(state); expect(tooltipValues.tooltip.values).toHaveLength(expectedTooltipValuesLength); expect(tooltipValues.tooltip.header === null).toBe(expectHeader); expect(tooltipValues.highlightedGeometries).toHaveLength(expectedHgeomsLength); diff --git a/src/chart_types/xy_chart/state/chart_state.tsx b/src/chart_types/xy_chart/state/chart_state.tsx index 53c267a4f5..833131a1d4 100644 --- a/src/chart_types/xy_chart/state/chart_state.tsx +++ b/src/chart_types/xy_chart/state/chart_state.tsx @@ -14,6 +14,7 @@ import { getLegendTooltipValuesSelector } from './selectors/get_legend_tooltip_v import { getPointerCursorSelector } from './selectors/get_cursor_pointer'; import { isBrushingSelector } from './selectors/is_brushing'; import { isTooltipVisibleSelector } from './selectors/is_tooltip_visible'; +import { getTooltipInfoSelector } from './selectors/get_tooltip_values_highlighted_geoms'; import { htmlIdGenerator } from '../../../utils/commons'; import { Tooltip } from '../../../components/tooltip'; @@ -53,4 +54,7 @@ export class XYAxisChartState implements InternalChartState { isTooltipVisible(globalState: GlobalChartState) { return isTooltipVisibleSelector(globalState); } + getTooltipInfo(globalState: GlobalChartState) { + return getTooltipInfoSelector(globalState); + } } diff --git a/src/chart_types/xy_chart/state/selectors/get_annotation_tooltip_state.ts b/src/chart_types/xy_chart/state/selectors/get_annotation_tooltip_state.ts index d4fab8bacf..880fc2c11b 100644 --- a/src/chart_types/xy_chart/state/selectors/get_annotation_tooltip_state.ts +++ b/src/chart_types/xy_chart/state/selectors/get_annotation_tooltip_state.ts @@ -15,7 +15,7 @@ import { getChartRotationSelector } from '../../../../state/selectors/get_chart_ import { AnnotationId } from '../../../../utils/ids'; import { computeSeriesGeometriesSelector } from './compute_series_geometries'; import { ComputedGeometries } from '../utils'; -import { getTooltipValuesSelector } from './get_tooltip_values_highlighted_geoms'; +import { getTooltipInfoSelector } from './get_tooltip_values_highlighted_geoms'; import { getChartIdSelector } from '../../../../state/selectors/get_chart_id'; import { GlobalChartState } from '../../../../state/chart_state'; import { TooltipInfo } from '../../../../components/tooltip/types'; @@ -31,7 +31,7 @@ export const getAnnotationTooltipStateSelector = createCachedSelector( getAnnotationSpecsSelector, getAxisSpecsSelector, computeAnnotationDimensionsSelector, - getTooltipValuesSelector, + getTooltipInfoSelector, ], getAnnotationTooltipState, )(getChartIdSelector); diff --git a/src/chart_types/xy_chart/state/selectors/get_legend_tooltip_values.ts b/src/chart_types/xy_chart/state/selectors/get_legend_tooltip_values.ts index 4f580c3d05..35a72e898f 100644 --- a/src/chart_types/xy_chart/state/selectors/get_legend_tooltip_values.ts +++ b/src/chart_types/xy_chart/state/selectors/get_legend_tooltip_values.ts @@ -1,10 +1,10 @@ import createCachedSelector from 're-reselect'; import { getSeriesTooltipValues, TooltipLegendValue } from '../../tooltip/tooltip'; -import { getTooltipValuesSelector } from './get_tooltip_values_highlighted_geoms'; +import { getTooltipInfoSelector } from './get_tooltip_values_highlighted_geoms'; import { getChartIdSelector } from '../../../../state/selectors/get_chart_id'; export const getLegendTooltipValuesSelector = createCachedSelector( - [getTooltipValuesSelector], + [getTooltipInfoSelector], ({ values }): Map => { return getSeriesTooltipValues(values); }, diff --git a/src/chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms.ts b/src/chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms.ts index a210d1cb09..d237ec2ef1 100644 --- a/src/chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms.ts +++ b/src/chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms.ts @@ -43,7 +43,7 @@ export interface TooltipAndHighlightedGeoms { const getExternalPointerEventStateSelector = (state: GlobalChartState) => state.externalEvents.pointer; -export const getTooltipValuesAndGeometriesSelector = createCachedSelector( +export const getTooltipInfoAndGeometriesSelector = createCachedSelector( [ getSeriesSpecsSelector, getAxisSpecsSelector, @@ -95,9 +95,9 @@ function getTooltipAndHighlightFromXValue( } // build the tooltip value list - let tooltipHeader: TooltipValue | null = null; + let header: TooltipValue | null = null; const highlightedGeometries: IndexedGeometry[] = []; - const tooltipValues = xMatchingGeoms + const values = xMatchingGeoms .filter(({ value: { y } }) => y !== null) .reduce((acc, indexedGeometry) => { const { @@ -145,11 +145,11 @@ function getTooltipAndHighlightFromXValue( ); // format only one time the x value - if (!tooltipHeader) { + 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; - tooltipHeader = formatTooltip(indexedGeometry, spec, true, false, hasSingleSeries, formatterAxis); + header = formatTooltip(indexedGeometry, spec, true, false, hasSingleSeries, formatterAxis); } return [...acc, formattedTooltip]; @@ -157,22 +157,22 @@ function getTooltipAndHighlightFromXValue( return { tooltip: { - header: tooltipHeader, - values: tooltipValues, + header, + values, }, highlightedGeometries, }; } -export const getTooltipValuesSelector = createCachedSelector( - [getTooltipValuesAndGeometriesSelector], +export const getTooltipInfoSelector = createCachedSelector( + [getTooltipInfoAndGeometriesSelector], ({ tooltip }): TooltipInfo => { return tooltip; }, )(getChartIdSelector); export const getHighlightedGeomsSelector = createCachedSelector( - [getTooltipValuesAndGeometriesSelector], + [getTooltipInfoAndGeometriesSelector], (values): IndexedGeometry[] => { return values.highlightedGeometries; }, diff --git a/src/chart_types/xy_chart/state/selectors/is_tooltip_visible.ts b/src/chart_types/xy_chart/state/selectors/is_tooltip_visible.ts index 3452cc9b80..225b76a32a 100644 --- a/src/chart_types/xy_chart/state/selectors/is_tooltip_visible.ts +++ b/src/chart_types/xy_chart/state/selectors/is_tooltip_visible.ts @@ -3,7 +3,7 @@ import { Point } from '../../../../utils/point'; import { GlobalChartState, PointerStates } from '../../../../state/chart_state'; import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs'; import { getProjectedPointerPositionSelector } from './get_projected_pointer_position'; -import { getTooltipValuesSelector } from './get_tooltip_values_highlighted_geoms'; +import { getTooltipInfoSelector } from './get_tooltip_values_highlighted_geoms'; import { getChartIdSelector } from '../../../../state/selectors/get_chart_id'; import { getTooltipType } from './get_tooltip_type'; import { TooltipType } from '../../../../specs'; @@ -21,7 +21,7 @@ export const isTooltipVisibleSelector = createCachedSelector( hasTooltipTypeDefinedSelector, getPointerSelector, getProjectedPointerPositionSelector, - getTooltipValuesSelector, + getTooltipInfoSelector, isAnnotationTooltipVisibleSelector, ], isTooltipVisible, diff --git a/src/chart_types/xy_chart/state/selectors/on_element_out_caller.ts b/src/chart_types/xy_chart/state/selectors/on_element_out_caller.ts index cf4042073b..4cee610e4f 100644 --- a/src/chart_types/xy_chart/state/selectors/on_element_out_caller.ts +++ b/src/chart_types/xy_chart/state/selectors/on_element_out_caller.ts @@ -1,7 +1,7 @@ import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs'; import createCachedSelector from 're-reselect'; import { - getTooltipValuesAndGeometriesSelector, + getTooltipInfoAndGeometriesSelector, TooltipAndHighlightedGeoms, } from './get_tooltip_values_highlighted_geoms'; import { SettingsSpec } from '../../../../specs'; @@ -40,7 +40,7 @@ export function createOnElementOutCaller(): (state: GlobalChartState) => void { return (state: GlobalChartState) => { if (selector === null && state.chartType === ChartTypes.XYAxis) { selector = createCachedSelector( - [getTooltipValuesAndGeometriesSelector, getSettingsSpecSelector], + [getTooltipInfoAndGeometriesSelector, getSettingsSpecSelector], ({ highlightedGeometries }: TooltipAndHighlightedGeoms, settings: SettingsSpec): void => { const nextProps = { settings, diff --git a/src/chart_types/xy_chart/state/selectors/on_element_over_caller.ts b/src/chart_types/xy_chart/state/selectors/on_element_over_caller.ts index 496a0367f3..07b4cc7ea2 100644 --- a/src/chart_types/xy_chart/state/selectors/on_element_over_caller.ts +++ b/src/chart_types/xy_chart/state/selectors/on_element_over_caller.ts @@ -1,7 +1,7 @@ import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs'; import createCachedSelector from 're-reselect'; import { - getTooltipValuesAndGeometriesSelector, + getTooltipInfoAndGeometriesSelector, TooltipAndHighlightedGeoms, } from './get_tooltip_values_highlighted_geoms'; import { SettingsSpec } from '../../../../specs'; @@ -50,7 +50,7 @@ export function createOnElementOverCaller(): (state: GlobalChartState) => void { return (state: GlobalChartState) => { if (selector === null && state.chartType === ChartTypes.XYAxis) { selector = createCachedSelector( - [getTooltipValuesAndGeometriesSelector, getSettingsSpecSelector], + [getTooltipInfoAndGeometriesSelector, getSettingsSpecSelector], ({ highlightedGeometries }: TooltipAndHighlightedGeoms, settings: SettingsSpec): void => { const nextProps = { settings, diff --git a/src/components/tooltip/index.tsx b/src/components/tooltip/index.tsx index b739c7459a..8bc009094b 100644 --- a/src/components/tooltip/index.tsx +++ b/src/components/tooltip/index.tsx @@ -8,14 +8,14 @@ import { isInitialized } from '../../state/selectors/is_initialized'; import { getInternalIsTooltipVisibleSelector } from '../../state/selectors/get_internal_is_tooltip_visible'; import { getTooltipHeaderFormatterSelector } from '../../state/selectors/get_tooltip_header_formatter'; import { getTooltipPositionSelector } from '../../chart_types/xy_chart/state/selectors/get_tooltip_position'; -import { getTooltipValuesSelector } from '../../chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms'; import { getFinalTooltipPosition, TooltipPosition } from '../../chart_types/xy_chart/crosshair/crosshair_utils'; import { TooltipInfo } from './types'; +import { getInternalTooltipInfoSelector } from '../../state/selectors/get_internal_tooltip_info'; interface TooltipStateProps { isVisible: boolean; - info: TooltipInfo; position: TooltipPosition | null; + info?: TooltipInfo; headerFormatter?: TooltipValueFormatter; } interface TooltipOwnProps { @@ -84,7 +84,7 @@ class TooltipComponent extends React.Component { render() { const { isVisible, info, headerFormatter, getChartContainerRef } = this.props; const chartContainerRef = getChartContainerRef(); - if (!this.portalNode || chartContainerRef.current === null || !isVisible) { + if (!this.portalNode || chartContainerRef.current === null || !isVisible || !info) { return null; } const tooltipComponent = ( @@ -119,21 +119,20 @@ class TooltipComponent extends React.Component { } } +const HIDDEN_TOOLTIP_PROPS = { + isVisible: false, + info: undefined, + position: null, + headerFormatter: undefined, +}; + const mapStateToProps = (state: GlobalChartState): TooltipStateProps => { if (!isInitialized(state)) { - return { - isVisible: false, - info: { - header: null, - values: [], - }, - position: null, - headerFormatter: undefined, - }; + return HIDDEN_TOOLTIP_PROPS; } return { isVisible: getInternalIsTooltipVisibleSelector(state), - info: getTooltipValuesSelector(state), + info: getInternalTooltipInfoSelector(state), position: getTooltipPositionSelector(state), headerFormatter: getTooltipHeaderFormatterSelector(state), }; diff --git a/src/state/chart_state.ts b/src/state/chart_state.ts index 45788266d6..db376498fe 100644 --- a/src/state/chart_state.ts +++ b/src/state/chart_state.ts @@ -15,6 +15,7 @@ import { UPDATE_PARENT_DIMENSION } from './actions/chart_settings'; import { EXTERNAL_POINTER_EVENT } from './actions/events'; import { RefObject } from 'react'; import { PartitionState } from '../chart_types/partition_chart/state/chart_state'; +import { TooltipInfo } from '../components/tooltip/types'; export type BackwardRef = () => React.RefObject; @@ -23,24 +24,56 @@ export type BackwardRef = () => React.RefObject; * globally by the and */ export interface InternalChartState { - // the chart type + /** + * the chart type + */ chartType: ChartTypes; - // returns a JSX element with the chart rendered (lenged excluded) + /** + * returns a JSX element with the chart rendered (lenged excluded) + * @param containerRef + * @param forwardStageRef + */ chartRenderer(containerRef: BackwardRef, forwardStageRef: RefObject): JSX.Element | null; - // true if the brush is available for this chart type + /** + * true if the brush is available for this chart type + * @param globalState + */ isBrushAvailable(globalState: GlobalChartState): boolean; - // true if the brush is available for this chart type + /** + * true if the brush is available for this chart type + * @param globalState + */ isBrushing(globalState: GlobalChartState): boolean; - // true if the chart is empty (no data displayed) + /** + * true if the chart is empty (no data displayed) + * @param globalState + */ isChartEmpty(globalState: GlobalChartState): boolean; - // return the list of legend items + /** + * return the list of legend items + * @param globalState + */ getLegendItems(globalState: GlobalChartState): Map; - // return the list of values for each legend item + /** + * return the list of values for each legend item + * @param globalState + */ getLegendItemsValues(globalState: GlobalChartState): Map; - // return the CSS pointer cursor depending on the internal chart state + /** + * return the CSS pointer cursor depending on the internal chart state + * @param globalState + */ getPointerCursor(globalState: GlobalChartState): string; - // true if the tooltip is visible, false otherwise + /** + * true if the tooltip is visible, false otherwise + * @param globalState + */ isTooltipVisible(globalState: GlobalChartState): boolean; + /** + * get the tooltip information to display + * @param globalState the GlobalChartState + */ + getTooltipInfo(globalState: GlobalChartState): TooltipInfo | undefined; } export interface SpecList { diff --git a/src/state/selectors/get_internal_tooltip_info.ts b/src/state/selectors/get_internal_tooltip_info.ts new file mode 100644 index 0000000000..e15fcf1abf --- /dev/null +++ b/src/state/selectors/get_internal_tooltip_info.ts @@ -0,0 +1,10 @@ +import { GlobalChartState } from '../chart_state'; +import { TooltipInfo } from '../../components/tooltip/types'; + +export const getInternalTooltipInfoSelector = (state: GlobalChartState): TooltipInfo | undefined => { + if (state.internalChartState) { + return state.internalChartState.getTooltipInfo(state); + } else { + return undefined; + } +}; From 75448c86737cd722a8dbba2efb81eb85e720657b Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Tue, 18 Feb 2020 19:37:39 +0100 Subject: [PATCH 15/17] refactor: decouple tooltip anchor position --- .../partition_chart/state/chart_state.tsx | 3 + .../crosshair/crosshair_utils.test.ts | 114 +++++++----------- .../xy_chart/crosshair/crosshair_utils.ts | 107 ++++------------ .../xy_chart/state/chart_state.tsx | 4 + .../state/selectors/get_tooltip_position.ts | 15 ++- src/components/tooltip/index.tsx | 11 +- src/components/tooltip/utils.ts | 78 ++++++++++++ src/state/chart_state.ts | 9 +- .../get_internal_tooltip_anchor_position.ts | 10 ++ 9 files changed, 182 insertions(+), 169 deletions(-) create mode 100644 src/components/tooltip/utils.ts create mode 100644 src/state/selectors/get_internal_tooltip_anchor_position.ts diff --git a/src/chart_types/partition_chart/state/chart_state.tsx b/src/chart_types/partition_chart/state/chart_state.tsx index ff07027c51..53bd8f9ac2 100644 --- a/src/chart_types/partition_chart/state/chart_state.tsx +++ b/src/chart_types/partition_chart/state/chart_state.tsx @@ -33,4 +33,7 @@ export class PartitionState implements InternalChartState { getTooltipInfo() { return undefined; } + getTooltipAnchor() { + return null; + } } diff --git a/src/chart_types/xy_chart/crosshair/crosshair_utils.test.ts b/src/chart_types/xy_chart/crosshair/crosshair_utils.test.ts index dbe6e52b22..3508b06bac 100644 --- a/src/chart_types/xy_chart/crosshair/crosshair_utils.test.ts +++ b/src/chart_types/xy_chart/crosshair/crosshair_utils.test.ts @@ -1,4 +1,4 @@ -import { getFinalTooltipPosition } from './crosshair_utils'; +import { getFinalTooltipPosition } from '../../../components/tooltip/utils'; describe('Tooltip position', () => { const container = { @@ -19,15 +19,11 @@ describe('Tooltip position', () => { container, tooltip, { - isRotatedHorizontal: true, - vPosition: { - bandHeight: 0, - bandTop: 0, - }, - hPosition: { - bandWidth: 0, - bandLeft: 10, - }, + isRotated: false, + y1: 0, + y0: 0, + x0: 10, + x1: 10, }, 5, ); @@ -39,15 +35,11 @@ describe('Tooltip position', () => { container, tooltip, { - isRotatedHorizontal: true, - vPosition: { - bandHeight: 0, - bandTop: 90, - }, - hPosition: { - bandWidth: 0, - bandLeft: 10, - }, + isRotated: false, + y0: 90, + y1: 90, + x0: 10, + x1: 10, }, 5, ); @@ -59,15 +51,11 @@ describe('Tooltip position', () => { container, tooltip, { - isRotatedHorizontal: true, - vPosition: { - bandHeight: 0, - bandTop: 0, - }, - hPosition: { - bandWidth: 0, - bandLeft: 100, - }, + isRotated: false, + y0: 0, + y1: 0, + x0: 100, + x1: 100, }, 5, ); @@ -79,15 +67,11 @@ describe('Tooltip position', () => { container, tooltip, { - isRotatedHorizontal: true, - vPosition: { - bandHeight: 0, - bandTop: 90, - }, - hPosition: { - bandWidth: 0, - bandLeft: 100, - }, + isRotated: false, + y0: 90, + y1: 90, + x0: 100, + x1: 100, }, 5, ); @@ -101,15 +85,11 @@ describe('Tooltip position', () => { container, tooltip, { - isRotatedHorizontal: false, - vPosition: { - bandHeight: 0, - bandTop: 0, - }, - hPosition: { - bandWidth: 0, - bandLeft: 10, - }, + isRotated: true, + y0: 0, + y1: 0, + x1: 10, + x0: 10, }, 5, ); @@ -121,15 +101,11 @@ describe('Tooltip position', () => { container, tooltip, { - isRotatedHorizontal: false, - vPosition: { - bandHeight: 0, - bandTop: 90, - }, - hPosition: { - bandWidth: 0, - bandLeft: 10, - }, + isRotated: true, + y0: 90, + y1: 90, + x1: 10, + x0: 10, }, 5, ); @@ -141,15 +117,11 @@ describe('Tooltip position', () => { container, tooltip, { - isRotatedHorizontal: false, - vPosition: { - bandHeight: 0, - bandTop: 0, - }, - hPosition: { - bandWidth: 0, - bandLeft: 100, - }, + isRotated: true, + y0: 0, + y1: 0, + x1: 100, + x0: 100, }, 5, ); @@ -161,15 +133,11 @@ describe('Tooltip position', () => { container, tooltip, { - isRotatedHorizontal: false, - vPosition: { - bandHeight: 0, - bandTop: 90, - }, - hPosition: { - bandWidth: 0, - bandLeft: 100, - }, + isRotated: true, + y0: 90, + y1: 90, + x1: 100, + x0: 100, }, 5, ); diff --git a/src/chart_types/xy_chart/crosshair/crosshair_utils.ts b/src/chart_types/xy_chart/crosshair/crosshair_utils.ts index 3852d8416b..03d03cc18e 100644 --- a/src/chart_types/xy_chart/crosshair/crosshair_utils.ts +++ b/src/chart_types/xy_chart/crosshair/crosshair_utils.ts @@ -1,29 +1,14 @@ import { Rotation } from '../../../utils/commons'; import { Dimensions } from '../../../utils/dimensions'; import { Scale } from '../../../scales'; -import { isHorizontalRotation } from '../state/utils'; +import { isHorizontalRotation, isVerticalRotation } from '../state/utils'; import { Point } from '../../../utils/point'; +import { TooltipAnchorPosition } from '../../../components/tooltip/utils'; export interface SnappedPosition { position: number; band: number; } -export interface TooltipPosition { - /** true if the x axis is horizontal */ - isRotatedHorizontal: boolean; - vPosition: { - /** the top position of the tooltip relative to the parent */ - bandTop: number; - /** the height of the crosshair band if any */ - bandHeight: number; - }; - hPosition: { - /** the left position of the tooltip relative to the parent */ - bandLeft: number; - /** the width of the crosshair band if any */ - bandWidth: number; - }; -} export const DEFAULT_SNAP_POSITION_BAND = 1; @@ -162,32 +147,32 @@ export function getCursorBandPosition( } } -export function getTooltipPosition( +export function getTooltipAnchorPosition( chartDimensions: Dimensions, chartRotation: Rotation, cursorBandPosition: Dimensions, cursorPosition: { x: number; y: number }, isSingleValueXScale: boolean, -): TooltipPosition { - const isHorizontalRotated = isHorizontalRotation(chartRotation); +): TooltipAnchorPosition { + const isRotated = isVerticalRotation(chartRotation); const hPosition = getHorizontalTooltipPosition( cursorPosition.x, cursorBandPosition, chartDimensions, - isHorizontalRotated, + isRotated, isSingleValueXScale, ); const vPosition = getVerticalTooltipPosition( cursorPosition.y, cursorBandPosition, chartDimensions, - isHorizontalRotated, + isRotated, isSingleValueXScale, ); return { - isRotatedHorizontal: isHorizontalRotated, - vPosition, - hPosition, + isRotated, + ...vPosition, + ...hPosition, }; } @@ -197,16 +182,16 @@ function getHorizontalTooltipPosition( chartDimensions: Dimensions, isHorizontalRotated: boolean, isSingleValueXScale: boolean, -): { bandLeft: number; bandWidth: number } { +): { x0: number; x1: number } { if (isHorizontalRotated) { return { - bandLeft: cursorBandPosition.left, - bandWidth: isSingleValueXScale ? 0 : cursorBandPosition.width, + x0: cursorBandPosition.left, + x1: cursorBandPosition.left + (isSingleValueXScale ? 0 : cursorBandPosition.width), }; } else { return { - bandWidth: 0, - bandLeft: chartDimensions.left + cursorXPosition, + x0: 0, + x1: chartDimensions.left + cursorXPosition, }; } } @@ -218,68 +203,18 @@ function getVerticalTooltipPosition( isHorizontalRotated: boolean, isSingleValueXScale: boolean, ): { - bandHeight: number; - bandTop: number; + y0: number; + y1: number; } { if (isHorizontalRotated) { return { - bandHeight: 0, - bandTop: cursorYPosition + chartDimensions.top, + y0: cursorYPosition + chartDimensions.top, + y1: cursorYPosition + chartDimensions.top, }; } else { return { - bandHeight: isSingleValueXScale ? 0 : cursorBandPosition.height, - bandTop: cursorBandPosition.top, + y0: cursorBandPosition.top, + y1: (isSingleValueXScale ? 0 : cursorBandPosition.height) + cursorBandPosition.top, }; } } - -export function getFinalTooltipPosition( - /** the dimensions of the chart parent container */ - container: Dimensions, - /** the dimensions of the tooltip container */ - tooltip: Dimensions, - /** the tooltip computed position not adjusted within chart bounds */ - tooltipPosition: TooltipPosition, - /** the padding to add between the tooltip position and the final position */ - padding = 10, -): { - left: string | null; - top: string | null; -} { - const { hPosition, vPosition, isRotatedHorizontal: isHorizontalRotated } = tooltipPosition; - let left = 0; - let top = 0; - if (isHorizontalRotated) { - const leftOfBand = window.pageXOffset + container.left + hPosition.bandLeft; - if (hPosition.bandLeft + hPosition.bandWidth + tooltip.width + padding > container.width) { - left = leftOfBand - tooltip.width - padding; - } else { - left = leftOfBand + hPosition.bandWidth + padding; - } - const topOfBand = window.pageYOffset + container.top; - if (vPosition.bandTop + tooltip.height > container.height) { - top = topOfBand + container.height - tooltip.height; - } else { - top = topOfBand + vPosition.bandTop; - } - } else { - const leftOfBand = window.pageXOffset + container.left; - if (hPosition.bandLeft + hPosition.bandWidth + tooltip.width > container.width) { - left = leftOfBand + container.width - tooltip.width; - } else { - left = leftOfBand + hPosition.bandLeft + hPosition.bandWidth; - } - const topOfBand = window.pageYOffset + container.top + vPosition.bandTop; - if (vPosition.bandTop + vPosition.bandHeight + tooltip.height + padding > container.height) { - top = topOfBand - tooltip.height - padding; - } else { - top = topOfBand + vPosition.bandHeight + padding; - } - } - - return { - left: `${Math.round(left)}px`, - top: `${Math.round(top)}px`, - }; -} diff --git a/src/chart_types/xy_chart/state/chart_state.tsx b/src/chart_types/xy_chart/state/chart_state.tsx index 833131a1d4..1c0f4497f4 100644 --- a/src/chart_types/xy_chart/state/chart_state.tsx +++ b/src/chart_types/xy_chart/state/chart_state.tsx @@ -17,6 +17,7 @@ import { isTooltipVisibleSelector } from './selectors/is_tooltip_visible'; import { getTooltipInfoSelector } from './selectors/get_tooltip_values_highlighted_geoms'; import { htmlIdGenerator } from '../../../utils/commons'; import { Tooltip } from '../../../components/tooltip'; +import { getTooltipAnchorPositionSelector } from './selectors/get_tooltip_position'; export class XYAxisChartState implements InternalChartState { chartType = ChartTypes.XYAxis; @@ -57,4 +58,7 @@ export class XYAxisChartState implements InternalChartState { getTooltipInfo(globalState: GlobalChartState) { return getTooltipInfoSelector(globalState); } + getTooltipAnchor(globalState: GlobalChartState) { + return getTooltipAnchorPositionSelector(globalState); + } } diff --git a/src/chart_types/xy_chart/state/selectors/get_tooltip_position.ts b/src/chart_types/xy_chart/state/selectors/get_tooltip_position.ts index 3e6364890d..f3c76e5368 100644 --- a/src/chart_types/xy_chart/state/selectors/get_tooltip_position.ts +++ b/src/chart_types/xy_chart/state/selectors/get_tooltip_position.ts @@ -1,13 +1,14 @@ import createCachedSelector from 're-reselect'; import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs'; -import { getTooltipPosition, TooltipPosition } from '../../crosshair/crosshair_utils'; +import { getTooltipAnchorPosition } from '../../crosshair/crosshair_utils'; import { getProjectedPointerPositionSelector } from './get_projected_pointer_position'; import { getComputedScalesSelector } from './get_computed_scales'; import { getCursorBandPositionSelector } from './get_cursor_band'; import { computeChartDimensionsSelector } from './compute_chart_dimensions'; import { getChartIdSelector } from '../../../../state/selectors/get_chart_id'; +import { TooltipAnchorPosition } from '../../../../components/tooltip/utils'; -export const getTooltipPositionSelector = createCachedSelector( +export const getTooltipAnchorPositionSelector = createCachedSelector( [ computeChartDimensionsSelector, getSettingsSpecSelector, @@ -15,11 +16,17 @@ export const getTooltipPositionSelector = createCachedSelector( getProjectedPointerPositionSelector, getComputedScalesSelector, ], - ({ chartDimensions }, settings, cursorBandPosition, projectedPointerPosition, scales): TooltipPosition | null => { + ( + { chartDimensions }, + settings, + cursorBandPosition, + projectedPointerPosition, + scales, + ): TooltipAnchorPosition | null => { if (!cursorBandPosition) { return null; } - return getTooltipPosition( + return getTooltipAnchorPosition( chartDimensions, settings.rotation, cursorBandPosition, diff --git a/src/components/tooltip/index.tsx b/src/components/tooltip/index.tsx index 8bc009094b..47028c9aad 100644 --- a/src/components/tooltip/index.tsx +++ b/src/components/tooltip/index.tsx @@ -2,25 +2,26 @@ import classNames from 'classnames'; import React from 'react'; import { createPortal } from 'react-dom'; import { connect } from 'react-redux'; +import { getFinalTooltipPosition, TooltipAnchorPosition } from './utils'; +import { TooltipInfo } from './types'; import { TooltipValueFormatter, TooltipValue } from '../../specs'; import { GlobalChartState, BackwardRef } from '../../state/chart_state'; import { isInitialized } from '../../state/selectors/is_initialized'; import { getInternalIsTooltipVisibleSelector } from '../../state/selectors/get_internal_is_tooltip_visible'; import { getTooltipHeaderFormatterSelector } from '../../state/selectors/get_tooltip_header_formatter'; -import { getTooltipPositionSelector } from '../../chart_types/xy_chart/state/selectors/get_tooltip_position'; -import { getFinalTooltipPosition, TooltipPosition } from '../../chart_types/xy_chart/crosshair/crosshair_utils'; -import { TooltipInfo } from './types'; import { getInternalTooltipInfoSelector } from '../../state/selectors/get_internal_tooltip_info'; +import { getInternalTooltipAnchorPositionSelector } from '../../state/selectors/get_internal_tooltip_anchor_position'; interface TooltipStateProps { isVisible: boolean; - position: TooltipPosition | null; + position: TooltipAnchorPosition | null; info?: TooltipInfo; headerFormatter?: TooltipValueFormatter; } interface TooltipOwnProps { getChartContainerRef: BackwardRef; } + type TooltipProps = TooltipStateProps & TooltipOwnProps; class TooltipComponent extends React.Component { @@ -133,7 +134,7 @@ const mapStateToProps = (state: GlobalChartState): TooltipStateProps => { return { isVisible: getInternalIsTooltipVisibleSelector(state), info: getInternalTooltipInfoSelector(state), - position: getTooltipPositionSelector(state), + position: getInternalTooltipAnchorPositionSelector(state), headerFormatter: getTooltipHeaderFormatterSelector(state), }; }; diff --git a/src/components/tooltip/utils.ts b/src/components/tooltip/utils.ts new file mode 100644 index 0000000000..6dece3cf88 --- /dev/null +++ b/src/components/tooltip/utils.ts @@ -0,0 +1,78 @@ +import { Dimensions } from '../../utils/dimensions'; + +export interface TooltipAnchorPosition { + /** + * true if the x axis is vertical + */ + isRotated?: boolean; + /** + * the top position of the anchor + */ + y0?: number; + /** + * the bottom position of the anchor + */ + y1: number; + /** + * the right position of anchor + */ + x0?: number; + /** + * the left position of the anchor + */ + x1: number; +} + +export function getFinalTooltipPosition( + /** the dimensions of the chart parent container */ + container: Dimensions, + /** the dimensions of the tooltip container */ + tooltip: Dimensions, + /** the tooltip anchor computed position not adjusted within chart bounds */ + anchorPosition: TooltipAnchorPosition, + /** the padding to add between the tooltip position and the final position */ + padding = 10, +): { + left: string | null; + top: string | null; +} { + const { x1, y1, isRotated } = anchorPosition; + let left = 0; + let top = 0; + + const x0 = anchorPosition.x0 || 0; + const y0 = anchorPosition.y0 || 0; + + if (!isRotated) { + const leftOfBand = window.pageXOffset + container.left + x0; + if (x1 + tooltip.width + padding > container.width) { + left = leftOfBand - tooltip.width - padding; + } else { + left = leftOfBand + (x1 - x0) + padding; + } + const topOfBand = window.pageYOffset + container.top; + if (y0 + tooltip.height > container.height) { + top = topOfBand + container.height - tooltip.height; + } else { + top = topOfBand + y0; + } + } else { + const leftOfBand = window.pageXOffset + container.left; + if (x1 + tooltip.width > container.width) { + left = leftOfBand + container.width - tooltip.width; + } else { + left = leftOfBand + x1; + } + const topOfBand = window.pageYOffset + container.top + y0; + if (y1 + tooltip.height + padding > container.height) { + top = topOfBand - tooltip.height - padding; + } else { + top = topOfBand + (y1 - y0) + padding; + } + } + + return { + left: `${Math.round(left)}px`, + top: `${Math.round(top)}px`, + }; +} diff --git a/src/state/chart_state.ts b/src/state/chart_state.ts index db376498fe..1cb0c4d29e 100644 --- a/src/state/chart_state.ts +++ b/src/state/chart_state.ts @@ -16,6 +16,7 @@ import { EXTERNAL_POINTER_EVENT } from './actions/events'; import { RefObject } from 'react'; import { PartitionState } from '../chart_types/partition_chart/state/chart_state'; import { TooltipInfo } from '../components/tooltip/types'; +import { TooltipAnchorPosition } from '../components/tooltip/utils'; export type BackwardRef = () => React.RefObject; @@ -70,10 +71,16 @@ export interface InternalChartState { */ isTooltipVisible(globalState: GlobalChartState): boolean; /** - * get the tooltip information to display + * Get the tooltip information to display * @param globalState the GlobalChartState */ getTooltipInfo(globalState: GlobalChartState): TooltipInfo | undefined; + + /** + * Get the tooltip anchor position + * @param globalState + */ + getTooltipAnchor(globalState: GlobalChartState): TooltipAnchorPosition | null; } export interface SpecList { diff --git a/src/state/selectors/get_internal_tooltip_anchor_position.ts b/src/state/selectors/get_internal_tooltip_anchor_position.ts new file mode 100644 index 0000000000..b026df2be1 --- /dev/null +++ b/src/state/selectors/get_internal_tooltip_anchor_position.ts @@ -0,0 +1,10 @@ +import { GlobalChartState } from '../chart_state'; +import { TooltipAnchorPosition } from '../../components/tooltip/utils'; + +export const getInternalTooltipAnchorPositionSelector = (state: GlobalChartState): TooltipAnchorPosition | null => { + if (state.internalChartState) { + return state.internalChartState.getTooltipAnchor(state); + } else { + return null; + } +}; From e1197ae731cff045578244b452c92a56ccab2f06 Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Wed, 19 Feb 2020 10:20:54 +0100 Subject: [PATCH 16/17] fix: rotated tooltip position --- .../xy_chart/crosshair/crosshair_utils.ts | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/chart_types/xy_chart/crosshair/crosshair_utils.ts b/src/chart_types/xy_chart/crosshair/crosshair_utils.ts index 03d03cc18e..84200e19ba 100644 --- a/src/chart_types/xy_chart/crosshair/crosshair_utils.ts +++ b/src/chart_types/xy_chart/crosshair/crosshair_utils.ts @@ -180,41 +180,39 @@ function getHorizontalTooltipPosition( cursorXPosition: number, cursorBandPosition: Dimensions, chartDimensions: Dimensions, - isHorizontalRotated: boolean, + isRotated: boolean, isSingleValueXScale: boolean, ): { x0: number; x1: number } { - if (isHorizontalRotated) { + if (!isRotated) { return { x0: cursorBandPosition.left, x1: cursorBandPosition.left + (isSingleValueXScale ? 0 : cursorBandPosition.width), }; - } else { - return { - x0: 0, - x1: chartDimensions.left + cursorXPosition, - }; } + return { + x0: 0, + x1: chartDimensions.left + cursorXPosition, + }; } function getVerticalTooltipPosition( cursorYPosition: number, cursorBandPosition: Dimensions, chartDimensions: Dimensions, - isHorizontalRotated: boolean, + isRotated: boolean, isSingleValueXScale: boolean, ): { y0: number; y1: number; } { - if (isHorizontalRotated) { + if (!isRotated) { return { y0: cursorYPosition + chartDimensions.top, y1: cursorYPosition + chartDimensions.top, }; - } else { - return { - y0: cursorBandPosition.top, - y1: (isSingleValueXScale ? 0 : cursorBandPosition.height) + cursorBandPosition.top, - }; } + return { + y0: cursorBandPosition.top, + y1: (isSingleValueXScale ? 0 : cursorBandPosition.height) + cursorBandPosition.top, + }; } From ebb0637a8a544c3359abc6dc345e208b74659f61 Mon Sep 17 00:00:00 2001 From: Marco Vettorello Date: Thu, 20 Feb 2020 12:21:16 +0100 Subject: [PATCH 17/17] refactor(legend): showLegendExtra to false by default change the default value of `showLegendExtra` to `false` BREAKING CHANGE: Previously `showLegendExtra` was true by default always showing the legend extra value. Now it becomes always hidden by default close #246 --- src/components/legend/legend.test.tsx | 6 ++--- src/specs/settings.tsx | 3 ++- stories/annotations.tsx | 2 +- stories/area_chart.tsx | 14 +++++----- stories/bar_chart.tsx | 32 +++++++++++++---------- stories/interactions.tsx | 37 +++++++++++++++++++-------- stories/legend.tsx | 14 +++++----- stories/line_chart.tsx | 10 ++++---- stories/mixed.tsx | 10 ++++---- stories/rotations.tsx | 17 ++++++------ stories/styling.tsx | 37 ++++++++++++++++++--------- 11 files changed, 109 insertions(+), 73 deletions(-) diff --git a/src/components/legend/legend.test.tsx b/src/components/legend/legend.test.tsx index e2e24dfa66..593d18479d 100644 --- a/src/components/legend/legend.test.tsx +++ b/src/components/legend/legend.test.tsx @@ -11,7 +11,7 @@ describe('Legend', () => { it('shall render the all the series names', () => { const wrapper = mount( - + { const data = dg.generateGroupedSeries(10, numberOfSeries, 'split'); const wrapper = mount( - + { const data = dg.generateGroupedSeries(10, numberOfSeries, 'split'); const wrapper = mount( - + { return ( - + { return ( - + { const allMetrics = [...data3, ...data2, ...data1]; return ( - + { const stackedAsPercentage = boolean('stacked as percentage', true); return ( - + { return ( - + { return ( - + { const y1AccessorFormat = text('y1AccessorFormat', ''); return ( - + { return ( - + { const stackAccessors = isStackedSeries ? ['x'] : undefined; return ( - + { return ( - + { return ( - + { const clusterBars = boolean('cluster', true); return ( - + { }; return ( - + { return ( - + { return ( - + { return ( - + { return ( - + { return ( - + { return ( - + { return ( - + { }; return ( - + { return ( - + { return ( - + { return ( - + { return ( - + { return ( - + { return ( { return ( { return ( { return ( - + { return ( - + { button(label, handler, groupId); return ( - + { return ( - + { return ( - + { return ( - + { return ( - + { const splitSeries = boolean('split series', true) ? ['g1', 'g2'] : undefined; return ( - + { return ( - + { return ( - + { return ( - + { return ( - + { return ( - + { return ( - + { return ( - + { return ( - + { return ( - + { return ( - + { return ( - + { return ( - + { { return ( - + @@ -104,7 +105,7 @@ negative90DegreeOrdinal.story = { export const rotations0DegOrdinal = () => { return ( - + @@ -132,7 +133,7 @@ rotations0DegOrdinal.story = { export const rotations90DegOrdinal = () => { return ( - + @@ -160,7 +161,7 @@ rotations90DegOrdinal.story = { export const rotations180DegOrdinal = () => { return ( - + @@ -188,7 +189,7 @@ rotations180DegOrdinal.story = { export const negative90DegLinear = () => { return ( - + @@ -216,7 +217,7 @@ negative90DegLinear.story = { export const rotations0DegLinear = () => { return ( - + @@ -244,7 +245,7 @@ rotations0DegLinear.story = { export const rotations90DegLinear = () => { return ( - + @@ -272,7 +273,7 @@ rotations90DegLinear.story = { export const rotations180DegLinear = () => { return ( - + diff --git a/stories/styling.tsx b/stories/styling.tsx index 66f757243c..a5089830f1 100644 --- a/stories/styling.tsx +++ b/stories/styling.tsx @@ -221,7 +221,13 @@ export const marginsAndPaddings = () => { const withTopTitle = boolean('top axis with title', true); return ( - + { theme={theme} baseTheme={darkmode ? DARK_THEME : LIGHT_THEME} debug={boolean('debug', false)} - showLegend={true} + showLegend + showLegendExtra legendPosition={Position.Right} /> @@ -499,7 +506,7 @@ export const partialCustomTheme = () => { return ( - + { return ( - + { return ( - + { return ( - + { return ( - + { return ( - + { return ( - + { return ( - + { }; return ( - + { return ( - +