diff --git a/e2e/screenshots/all.test.ts-snapshots/baselines/line-chart/isolated-data-points-chrome-linux.png b/e2e/screenshots/all.test.ts-snapshots/baselines/line-chart/isolated-data-points-chrome-linux.png index 4c305c0b08..0532e50b8d 100644 Binary files a/e2e/screenshots/all.test.ts-snapshots/baselines/line-chart/isolated-data-points-chrome-linux.png and b/e2e/screenshots/all.test.ts-snapshots/baselines/line-chart/isolated-data-points-chrome-linux.png differ diff --git a/e2e/screenshots/area_stories.test.ts-snapshots/area-series-stories/area-with-isolated-data-points/render-correctly-fit-function-chrome-linux.png b/e2e/screenshots/area_stories.test.ts-snapshots/area-series-stories/area-with-isolated-data-points/render-correctly-fit-function-chrome-linux.png index 1146ba4112..3974c17491 100644 Binary files a/e2e/screenshots/area_stories.test.ts-snapshots/area-series-stories/area-with-isolated-data-points/render-correctly-fit-function-chrome-linux.png and b/e2e/screenshots/area_stories.test.ts-snapshots/area-series-stories/area-with-isolated-data-points/render-correctly-fit-function-chrome-linux.png differ diff --git a/e2e/screenshots/line_stories.test.ts-snapshots/line-series-stories/points-auto-visibility/hide-points-when-space-between-point-is-small-chrome-linux.png b/e2e/screenshots/line_stories.test.ts-snapshots/line-series-stories/points-auto-visibility/hide-points-when-space-between-point-is-small-chrome-linux.png new file mode 100644 index 0000000000..de069e48f5 Binary files /dev/null and b/e2e/screenshots/line_stories.test.ts-snapshots/line-series-stories/points-auto-visibility/hide-points-when-space-between-point-is-small-chrome-linux.png differ diff --git a/e2e/screenshots/line_stories.test.ts-snapshots/line-series-stories/points-auto-visibility/show-points-when-space-between-point-is-enough-chrome-linux.png b/e2e/screenshots/line_stories.test.ts-snapshots/line-series-stories/points-auto-visibility/show-points-when-space-between-point-is-enough-chrome-linux.png new file mode 100644 index 0000000000..04f758a896 Binary files /dev/null and b/e2e/screenshots/line_stories.test.ts-snapshots/line-series-stories/points-auto-visibility/show-points-when-space-between-point-is-enough-chrome-linux.png differ diff --git a/e2e/tests/line_stories.test.ts b/e2e/tests/line_stories.test.ts index 534b647a4e..b1724d2078 100644 --- a/e2e/tests/line_stories.test.ts +++ b/e2e/tests/line_stories.test.ts @@ -58,4 +58,16 @@ test.describe('Line series stories', () => { ); }); }); + test.describe('Points auto visibility', () => { + test('show points when space between point is enough', async ({ page }) => { + await common.expectChartAtUrlToMatchScreenshot(page)( + 'http://localhost:9001/?path=/story/line-chart--isolated-data-points&globals=toggles.showHeader:true;toggles.showChartTitle:false;toggles.showChartDescription:false;toggles.showChartBoundary:false;theme:light&knob-default point radius=3&knob-enable fit function=true&knob-max data points=25&knob-point shape=circle&knob-point visibility=auto&knob-point visibility min distance=20&knob-series type=line&knob-visible series[0]=A&knob-visible series[1]=B&knob-visible series[2]=C&knob-visible series[3]=D', + ); + }); + test('hide points when space between point is small', async ({ page }) => { + await common.expectChartAtUrlToMatchScreenshot(page)( + 'http://localhost:9001/?path=/story/line-chart--isolated-data-points&globals=toggles.showHeader:true;toggles.showChartTitle:false;toggles.showChartDescription:false;toggles.showChartBoundary:false;theme:light&knob-default point radius=3&knob-enable fit function=true&knob-max data points=60&knob-point shape=circle&knob-point visibility=auto&knob-point visibility min distance=20&knob-series type=line&knob-visible series[0]=A&knob-visible series[1]=B&knob-visible series[2]=C&knob-visible series[3]=D', + ); + }); + }); }); diff --git a/packages/charts/src/chart_types/xy_chart/renderer/canvas/points.ts b/packages/charts/src/chart_types/xy_chart/renderer/canvas/points.ts index 242f716920..bf21b19c3e 100644 --- a/packages/charts/src/chart_types/xy_chart/renderer/canvas/points.ts +++ b/packages/charts/src/chart_types/xy_chart/renderer/canvas/points.ts @@ -29,11 +29,13 @@ export function renderPoints( { opacity }: GeometryStateStyle, pointStyle: PointStyle, lineStrokeWidth: number, - minDistanceBetweenPoints: number, - minDistanceToShowPoints: number, + seriesMinPointDistance: number, + pointsDistanceVisibilityThreshold: number, hasConnectingLine: boolean, ) { - const isHiddenOnAuto = pointStyle.visible === 'auto' && minDistanceBetweenPoints < minDistanceToShowPoints; + // seriesMinPointDistance could be Infinity if we don't have points, or we just have a single point per series. + // In this case the point should be visible if the visibility style is set to `auto` + const isHiddenOnAuto = pointStyle.visible === 'auto' && seriesMinPointDistance < pointsDistanceVisibilityThreshold; const hideDataPoints = pointStyle.visible === 'never' || isHiddenOnAuto; const hideIsolatedDataPoints = hasConnectingLine && hideDataPoints; diff --git a/packages/charts/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx b/packages/charts/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx index 52452fd041..45ccf89f1b 100644 --- a/packages/charts/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx +++ b/packages/charts/src/chart_types/xy_chart/renderer/canvas/xy_chart.tsx @@ -207,7 +207,6 @@ const DEFAULT_PROPS: ReactiveChartStateProps = { areas: [], bars: [], lines: [], - points: [], bubbles: [], }, geometriesIndex: new IndexedGeometryMap(), diff --git a/packages/charts/src/chart_types/xy_chart/rendering/area.ts b/packages/charts/src/chart_types/xy_chart/rendering/area.ts index 69a6dedc85..e8ab78975d 100644 --- a/packages/charts/src/chart_types/xy_chart/rendering/area.ts +++ b/packages/charts/src/chart_types/xy_chart/rendering/area.ts @@ -19,7 +19,6 @@ import { } from './utils'; import { Color } from '../../../common/colors'; import { ScaleBand, ScaleContinuous } from '../../../scales'; -import { isBandScale } from '../../../scales/types'; import { CurveType, getCurveFactory } from '../../../utils/curves'; import { Dimensions } from '../../../utils/dimensions'; import { AreaGeometry } from '../../../utils/geometry'; @@ -71,7 +70,7 @@ export function renderArea( if (y1Line) lines.push(y1Line); if (y0Line) lines.push(y0Line); - const { pointGeometries, indexedGeometryMap } = renderPoints( + const { pointGeometries, indexedGeometryMap, minDistanceBetweenPoints } = renderPoints( shift - xScaleOffset, dataSeries, xScale, @@ -103,7 +102,7 @@ export function renderArea( clippedRanges, shouldClip: hasFit, hasFit, - minPointDistance: isBandScale(xScale) ? xScale.bandwidth : xScale.scale(xScale.domain[0] + xScale.minInterval), + minPointDistance: minDistanceBetweenPoints, }; return { areaGeometry, diff --git a/packages/charts/src/chart_types/xy_chart/rendering/line.ts b/packages/charts/src/chart_types/xy_chart/rendering/line.ts index 4e569c5e5f..70b1a608b4 100644 --- a/packages/charts/src/chart_types/xy_chart/rendering/line.ts +++ b/packages/charts/src/chart_types/xy_chart/rendering/line.ts @@ -12,7 +12,6 @@ import { renderPoints } from './points'; import { getClippedRanges, getY1ScaledValueFn, getYDatumValueFn, isYValueDefinedFn, MarkSizeOptions } from './utils'; import { Color } from '../../../common/colors'; import { ScaleBand, ScaleContinuous } from '../../../scales'; -import { isBandScale } from '../../../scales/types'; import { CurveType, getCurveFactory } from '../../../utils/curves'; import { Dimensions } from '../../../utils/dimensions'; import { LineGeometry } from '../../../utils/geometry'; @@ -50,7 +49,7 @@ export function renderLine( .defined((datum) => definedFn(datum, y1Accessor)) .curve(getCurveFactory(curve)); - const { pointGeometries, indexedGeometryMap } = renderPoints( + const { pointGeometries, indexedGeometryMap, minDistanceBetweenPoints } = renderPoints( shift - xScaleOffset, dataSeries, xScale, @@ -70,7 +69,7 @@ export function renderLine( // TODO we can probably avoid computing the clipped ranges if no fit function is applied. const clippedRanges = getClippedRanges(dataSeries.data, xScale, xScaleOffset); - const lineGeometry = { + const lineGeometry: LineGeometry = { line: pathGenerator(dataSeries.data) || '', points: pointGeometries, color, @@ -83,10 +82,7 @@ export function renderLine( clippedRanges, shouldClip: hasFit, hasFit, - minPointDistance: isBandScale(xScale) ? xScale.bandwidth : xScale.scale(xScale.domain[0] + xScale.minInterval), - }; - return { - lineGeometry, - indexedGeometryMap, + minPointDistance: minDistanceBetweenPoints, }; + return { lineGeometry, indexedGeometryMap }; } diff --git a/packages/charts/src/chart_types/xy_chart/rendering/points.ts b/packages/charts/src/chart_types/xy_chart/rendering/points.ts index e909cb0fce..848fd782fd 100644 --- a/packages/charts/src/chart_types/xy_chart/rendering/points.ts +++ b/packages/charts/src/chart_types/xy_chart/rendering/points.ts @@ -56,6 +56,7 @@ export function renderPoints( styleAccessor?: PointStyleAccessor, ): { pointGeometries: PointGeometry[]; + minDistanceBetweenPoints: number; indexedGeometryMap: IndexedGeometryMap; } { const indexedGeometryMap = new IndexedGeometryMap(); @@ -74,78 +75,91 @@ export function renderPoints( let isolatedPointStyle = buildPointGeometryStyles(color, isolatedPointThemeStyle); let styleOverrides: Partial | undefined = undefined; - const pointGeometries = dataSeries.data.reduce>((acc, datum, dataIndex) => { - const { x: xValue, mark } = datum; - const prev = dataSeries.data[dataIndex - 1]; - const next = dataSeries.data[dataIndex + 1]; - // don't create the point if not within the xScale domain - if (!xScale.isValueInDomain(xValue)) return acc; + const { pointGeometries, minDistanceBetweenPoints } = dataSeries.data.reduce<{ + pointGeometries: SortedArray; + minDistanceBetweenPoints: number; + prevX: number | undefined; + }>( + (acc, datum, dataIndex) => { + const { x: xValue, mark } = datum; + const prev = dataSeries.data[dataIndex - 1]; + const next = dataSeries.data[dataIndex + 1]; + // don't create the point if not within the xScale domain + if (!xScale.isValueInDomain(xValue)) return acc; - // don't create the point if it that point was filled - const x = xScale.scale(xValue); + // don't create the point if it that point was filled + const x = xScale.scale(xValue); - if (Number.isNaN(x)) return acc; + if (!isFiniteNumber(x)) return acc; - const yDatumKeyNames: Array> = isBandedSpec ? ['y0', 'y1'] : ['y1']; - const seriesIdentifier: XYChartSeriesIdentifier = getSeriesIdentifierFromDataSeries(dataSeries); - const isPointIsolated = allowIsolated && isIsolatedPoint(dataIndex, dataSeries.data.length, yDefined, prev, next); - if (styleAccessor) { - styleOverrides = getPointStyleOverrides(datum, seriesIdentifier, isPointIsolated, styleAccessor); - style = buildPointGeometryStyles(color, pointStyle, styleOverrides); - isolatedPointStyle = buildPointGeometryStyles(color, isolatedPointThemeStyle, styleOverrides); - } - yDatumKeyNames.forEach((yDatumKeyName, keyIndex) => { - const valueAccessor = getYDatumValueFn(yDatumKeyName); - const y = yDatumKeyName === 'y1' ? y1Fn(datum) : y0Fn(datum); - const originalY = getDatumYValue(datum, keyIndex === 0, isBandedSpec, dataSeries.stackMode); + if (isFiniteNumber(acc.prevX) && !isDatumFilled(datum)) { + acc.minDistanceBetweenPoints = Math.min(acc.minDistanceBetweenPoints, Math.abs(x - acc.prevX)); + } + acc.prevX = x; + + const yDatumKeyNames: Array> = isBandedSpec ? ['y0', 'y1'] : ['y1']; + const seriesIdentifier: XYChartSeriesIdentifier = getSeriesIdentifierFromDataSeries(dataSeries); + const isPointIsolated = allowIsolated && isIsolatedPoint(dataIndex, dataSeries.data.length, yDefined, prev, next); + if (styleAccessor) { + styleOverrides = getPointStyleOverrides(datum, seriesIdentifier, isPointIsolated, styleAccessor); + style = buildPointGeometryStyles(color, pointStyle, styleOverrides); + isolatedPointStyle = buildPointGeometryStyles(color, isolatedPointThemeStyle, styleOverrides); + } + yDatumKeyNames.forEach((yDatumKeyName, keyIndex) => { + const valueAccessor = getYDatumValueFn(yDatumKeyName); + const y = yDatumKeyName === 'y1' ? y1Fn(datum) : y0Fn(datum); + const originalY = getDatumYValue(datum, keyIndex === 0, isBandedSpec, dataSeries.stackMode); - // if radius is defined with the mark, limit the minimum radius to the theme radius value - const radius = isPointIsolated - ? isolatedPointRadius(lineStrokeWidth) - : markSizeOptions.enabled - ? Math.max(getRadius(mark), pointStyle.radius) - : styleOverrides?.radius ?? pointStyle.radius; + // if radius is defined with the mark, limit the minimum radius to the theme radius value + const radius = isPointIsolated + ? isolatedPointRadius(lineStrokeWidth) + : markSizeOptions.enabled + ? Math.max(getRadius(mark), pointStyle.radius) + : styleOverrides?.radius ?? pointStyle.radius; - const pointGeometry: PointGeometry = { - x, - y: y === null ? NaN : y, - radius, - color, - style: isPointIsolated ? isolatedPointStyle : style, - value: { - x: xValue, - y: originalY, - mark, - accessor: isBandedSpec && keyIndex === 0 ? BandedAccessorType.Y0 : BandedAccessorType.Y1, - datum: datum.datum, - }, - transform: { - x: shift, - y: 0, - }, - seriesIdentifier, - panel, - isolated: isPointIsolated, - }; - indexedGeometryMap.set(pointGeometry, geometryType); - // use the geometry only if the yDatum in contained in the current yScale domain - if ( - isFiniteNumber(y) && - yDefined(datum, valueAccessor) && - yScale.isValueInDomain(valueAccessor(datum)) && - !isDatumFilled(datum) - ) { - if (needSorting) { - inplaceInsertInSortedArray(acc, pointGeometry, (p) => p?.radius ?? NaN); - } else { - acc.push(pointGeometry); + const pointGeometry: PointGeometry = { + x, + y: y === null ? NaN : y, + radius, + color, + style: isPointIsolated ? isolatedPointStyle : style, + value: { + x: xValue, + y: originalY, + mark, + accessor: isBandedSpec && keyIndex === 0 ? BandedAccessorType.Y0 : BandedAccessorType.Y1, + datum: datum.datum, + }, + transform: { + x: shift, + y: 0, + }, + seriesIdentifier, + panel, + isolated: isPointIsolated, + }; + indexedGeometryMap.set(pointGeometry, geometryType); + // use the geometry only if the yDatum in contained in the current yScale domain + if ( + isFiniteNumber(y) && + yDefined(datum, valueAccessor) && + yScale.isValueInDomain(valueAccessor(datum)) && + !isDatumFilled(datum) + ) { + if (needSorting) { + inplaceInsertInSortedArray(acc.pointGeometries, pointGeometry, (p) => p?.radius ?? NaN); + } else { + acc.pointGeometries.push(pointGeometry); + } } - } - }); - return acc; - }, []); + }); + return acc; + }, + { pointGeometries: [], minDistanceBetweenPoints: Infinity, prevX: undefined }, + ); return { pointGeometries, + minDistanceBetweenPoints, indexedGeometryMap, }; } diff --git a/packages/charts/src/chart_types/xy_chart/state/utils/types.ts b/packages/charts/src/chart_types/xy_chart/state/utils/types.ts index 89a18eb8d5..54de2abb29 100644 --- a/packages/charts/src/chart_types/xy_chart/state/utils/types.ts +++ b/packages/charts/src/chart_types/xy_chart/state/utils/types.ts @@ -8,14 +8,7 @@ import { SmallMultiplesSeriesDomains } from '../../../../common/panel_utils'; import { ScaleBand, ScaleContinuous } from '../../../../scales'; -import { - PointGeometry, - BarGeometry, - AreaGeometry, - LineGeometry, - BubbleGeometry, - PerPanel, -} from '../../../../utils/geometry'; +import { BarGeometry, AreaGeometry, LineGeometry, BubbleGeometry, PerPanel } from '../../../../utils/geometry'; import { GroupId } from '../../../../utils/ids'; import { XDomain, YDomain } from '../../domains/types'; import { IndexedGeometryMap } from '../../utils/indexed_geometry_map'; @@ -48,7 +41,6 @@ export interface ComputedScales { /** @internal */ export interface Geometries { - points: PointGeometry[]; bars: Array>; areas: Array>; lines: Array>; diff --git a/packages/charts/src/chart_types/xy_chart/state/utils/utils.ts b/packages/charts/src/chart_types/xy_chart/state/utils/utils.ts index 63dbc7be04..37a6c5c818 100644 --- a/packages/charts/src/chart_types/xy_chart/state/utils/utils.ts +++ b/packages/charts/src/chart_types/xy_chart/state/utils/utils.ts @@ -19,14 +19,7 @@ import { TextMeasure } from '../../../../utils/bbox/canvas_text_bbox_calculator' import { isFiniteNumber, isNil, isUniqueArray, mergePartial, Rotation } from '../../../../utils/common'; import { CurveType } from '../../../../utils/curves'; import { Dimensions, Size } from '../../../../utils/dimensions'; -import { - AreaGeometry, - BarGeometry, - BubbleGeometry, - LineGeometry, - PerPanel, - PointGeometry, -} from '../../../../utils/geometry'; +import { AreaGeometry, BarGeometry, BubbleGeometry, LineGeometry, PerPanel } from '../../../../utils/geometry'; import { GroupId, SpecId } from '../../../../utils/ids'; import { SeriesCompareFn } from '../../../../utils/series_sort'; import { ColorConfig, Theme } from '../../../../utils/themes/theme'; @@ -307,7 +300,6 @@ function renderGeometries( fallBackTickFormatter: TickFormatter, measureText: TextMeasure, ): Omit { - const points: PointGeometry[] = []; const bars: Array> = []; const areas: Array> = []; const lines: Array> = []; @@ -325,7 +317,9 @@ function renderGeometries( bubblePoints: 0, }; const barsPadding = enableHistogramMode ? chartTheme.scales.histogramPadding : chartTheme.scales.barsPadding; - + // This var remains Infinity if we don't have points, or we just have a single point per series. + // In this case the point should be visible if the visibility style is set to `auto` + let globalMinPointsDistance = Infinity; dataSeries.forEach((ds) => { const spec = getSpecsById(seriesSpecs, ds.specId); if (spec === undefined) { @@ -463,11 +457,12 @@ function renderGeometries( }); geometriesCounts.linePoints += renderedLines.lineGeometry.points.length; geometriesCounts.lines += 1; + globalMinPointsDistance = Math.min(globalMinPointsDistance, renderedLines.lineGeometry.minPointDistance); } else if (isAreaSeriesSpec(spec)) { const areaShift = barIndexOrder && barIndexOrder.length > 0 ? barIndexOrder.length : 1; const areaSeriesStyle = getAreaSeriesStyles(chartTheme.areaSeriesStyle, spec.areaSeriesStyle); const xScaleOffset = computeXScaleOffset(xScale, enableHistogramMode, spec.histogramModeAlignment); - const renderedAreas = renderArea( + const renderedArea = renderArea( // move the point on half of the bandwidth if we have mixed bars/lines (xScale.bandwidth * areaShift) / 2, ds, @@ -487,22 +482,28 @@ function renderGeometries( hasFitFnConfigured(spec.fit), spec.pointStyleAccessor, ); - geometriesIndex.merge(renderedAreas.indexedGeometryMap); + geometriesIndex.merge(renderedArea.indexedGeometryMap); areas.push({ panel, - value: renderedAreas.areaGeometry, + value: renderedArea.areaGeometry, }); - geometriesCounts.areasPoints += renderedAreas.areaGeometry.points.length; + geometriesCounts.areasPoints += renderedArea.areaGeometry.points.length; geometriesCounts.areas += 1; + globalMinPointsDistance = Math.min(globalMinPointsDistance, renderedArea.areaGeometry.minPointDistance); } }); return { geometries: { - points, bars, - areas, - lines, + areas: areas.map((a) => { + a.value.minPointDistance = globalMinPointsDistance; + return a; + }), + lines: lines.map((l) => { + l.value.minPointDistance = globalMinPointsDistance; + return l; + }), bubbles, }, geometriesIndex, diff --git a/packages/charts/src/utils/geometry.ts b/packages/charts/src/utils/geometry.ts index 59f5953944..cddf78785e 100644 --- a/packages/charts/src/utils/geometry.ts +++ b/packages/charts/src/utils/geometry.ts @@ -13,6 +13,7 @@ import { BarSeriesStyle, PointStyle, PointShape, LineSeriesStyle, AreaSeriesStyl import { XYChartSeriesIdentifier } from '../chart_types/xy_chart/utils/series'; import { LabelOverflowConstraint } from '../chart_types/xy_chart/utils/specs'; import { Color } from '../common/colors'; +import { Pixels } from '../common/geometry'; import { Fill, Stroke } from '../geoms/types'; /** @@ -122,7 +123,7 @@ export interface LineGeometry { clippedRanges: ClippedRanges; shouldClip: boolean; hasFit: boolean; - minPointDistance: number; + minPointDistance: Pixels; } /** @internal */ @@ -144,7 +145,7 @@ export interface AreaGeometry { clippedRanges: ClippedRanges; shouldClip: boolean; hasFit: boolean; - minPointDistance: number; + minPointDistance: Pixels; } /** @internal */ diff --git a/storybook/stories/line/12_isolated_data_points.story.tsx b/storybook/stories/line/12_isolated_data_points.story.tsx index a115942991..44b00c9ddc 100644 --- a/storybook/stories/line/12_isolated_data_points.story.tsx +++ b/storybook/stories/line/12_isolated_data_points.story.tsx @@ -15,6 +15,7 @@ import { KIBANA_METRICS } from '@elastic/charts/src/utils/data_samples/test_data import { ChartsStory } from '../../types'; import { useBaseTheme } from '../../use_base_theme'; import { customKnobs } from '../utils/knobs'; +import { getMultiSelectKnob } from '../utils/knobs/custom'; export const Example: ChartsStory = (_, { title, description }) => { const fitEnabled = boolean('enable fit function', false); @@ -45,6 +46,8 @@ export const Example: ChartsStory = (_, { title, description }) => { }, ); + const visibleSeries = getMultiSelectKnob('visible series', { A: 'A', B: 'B', C: 'C', D: 'D' }, ['A', 'B', 'C', 'D']); + return ( { } return [...d, 'C']; }), - ]} + ...KIBANA_METRICS.metrics.kibana_os_load.v3.data.slice(0, maxDataPoints).map((d, i) => { + if (i % 5 !== 0) { + return [d[0], null, 'D']; + } + return [d[0], i % 2 === 0 ? 2 : 4, 'D']; + }), + ].filter((d) => visibleSeries.includes(d[2] as unknown as 'A' | 'B' | 'C' | 'D'))} fit={fitEnabled ? Fit.Linear : undefined} curve={CurveType.CURVE_MONOTONE_X} />