diff --git a/e2e/screenshots/interactions.test.ts-snapshots/interactions/highlighter-z-index-should-respect-geometry-z-index-chrome-linux.png b/e2e/screenshots/interactions.test.ts-snapshots/interactions/highlighter-z-index-should-respect-geometry-z-index-chrome-linux.png deleted file mode 100644 index 770e3d1818..0000000000 Binary files a/e2e/screenshots/interactions.test.ts-snapshots/interactions/highlighter-z-index-should-respect-geometry-z-index-chrome-linux.png and /dev/null differ diff --git a/e2e/screenshots/interactions.test.ts-snapshots/interactions/highlighting/highlighter-z-index-should-respect-geometry-z-index-chrome-linux.png b/e2e/screenshots/interactions.test.ts-snapshots/interactions/highlighting/highlighter-z-index-should-respect-geometry-z-index-chrome-linux.png new file mode 100644 index 0000000000..cded80dc6a Binary files /dev/null and b/e2e/screenshots/interactions.test.ts-snapshots/interactions/highlighting/highlighter-z-index-should-respect-geometry-z-index-chrome-linux.png differ diff --git a/e2e/screenshots/interactions.test.ts-snapshots/interactions/highlighting/should-correctly-highlight-the-points-and-the-tooltip-when-hovering-over-overlapping-point-geometries-chrome-linux.png b/e2e/screenshots/interactions.test.ts-snapshots/interactions/highlighting/should-correctly-highlight-the-points-and-the-tooltip-when-hovering-over-overlapping-point-geometries-chrome-linux.png new file mode 100644 index 0000000000..47b269b3c3 Binary files /dev/null and b/e2e/screenshots/interactions.test.ts-snapshots/interactions/highlighting/should-correctly-highlight-the-points-and-the-tooltip-when-hovering-over-overlapping-point-geometries-chrome-linux.png differ diff --git a/e2e/screenshots/interactions.test.ts-snapshots/interactions/tooltips/should-render-current-tooltip-in-dark-theme-chrome-linux.png b/e2e/screenshots/interactions.test.ts-snapshots/interactions/tooltips/should-render-current-tooltip-in-dark-theme-chrome-linux.png index 92ca71d928..fcba3d6263 100644 Binary files a/e2e/screenshots/interactions.test.ts-snapshots/interactions/tooltips/should-render-current-tooltip-in-dark-theme-chrome-linux.png and b/e2e/screenshots/interactions.test.ts-snapshots/interactions/tooltips/should-render-current-tooltip-in-dark-theme-chrome-linux.png differ diff --git a/e2e/tests/interactions.test.ts b/e2e/tests/interactions.test.ts index 565c93817b..4038980927 100644 --- a/e2e/tests/interactions.test.ts +++ b/e2e/tests/interactions.test.ts @@ -420,12 +420,23 @@ test.describe('Interactions', () => { (t) => `should show cursor band when background is set with ${t} theme`, ); }); - // currently wrong due to https://github.com/elastic/elastic-charts/issues/1921 - test('highlighter zIndex should respect geometry zIndex', async ({ page }) => { - await common.expectChartWithMouseAtUrlToMatchScreenshot(page)( - 'http://localhost:9001/?path=/story/test-cases--highlighter-z-index', - { left: 247, top: 76 }, // mouse over the second point - ); + + test.describe('Highlighting', () => { + test('highlighter zIndex should respect geometry zIndex', async ({ page }) => { + await common.expectChartWithMouseAtUrlToMatchScreenshot(page)( + 'http://localhost:9001/?path=/story/test-cases--highlighter-z-index', + { left: 247, top: 76 }, // mouse over the second point + ); + }); + + test('should correctly highlight the points and the tooltip, when hovering over overlapping point geometries', async ({ + page, + }) => { + await common.expectChartWithMouseAtUrlToMatchScreenshot(page)( + 'http://localhost:9001/?path=/story/interactions--series-highlight-style&globals=toggles.showHeader:true;toggles.showChartTitle:false;toggles.showChartDescription:false;toggles.showChartBoundary:false;theme:light&knob-Banded=true&knob-Scale%20Type=log&knob-Series%20Type=bar&knob-Show%20positive%20data=true&knob-Split=true&knob-Stacked=true&knob-chart%20type=area&knob-data%20points=50&knob-logMinLimit=1&knob-series=10', + { left: 370, top: 180 }, + ); + }); }); // should not disable dragging when returning to origin diff --git a/packages/charts/src/chart_types/xy_chart/renderer/canvas/lines.ts b/packages/charts/src/chart_types/xy_chart/renderer/canvas/lines.ts index 120c5a08d3..e346e4ed8c 100644 --- a/packages/charts/src/chart_types/xy_chart/renderer/canvas/lines.ts +++ b/packages/charts/src/chart_types/xy_chart/renderer/canvas/lines.ts @@ -43,7 +43,7 @@ export function renderLines( }; }) // sort by dimmed first once are rendered ontop of the non-highlighted ones - .sort(({ highlightState }) => (highlightState === 'dimmed' ? -1 : 1)) + .sort((a, b) => (a.highlightState === b.highlightState ? 0 : a.highlightState === 'dimmed' ? -1 : 1)) .forEach(({ panel, line, highlightState }) => { const clippings = getPanelClipping(panel, rotation); if (line.style.line.visible) { diff --git a/packages/charts/src/chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms.ts b/packages/charts/src/chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms.ts index 01c246f322..0263e184cf 100644 --- a/packages/charts/src/chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms.ts +++ b/packages/charts/src/chart_types/xy_chart/state/selectors/get_tooltip_values_highlighted_geoms.ts @@ -27,7 +27,7 @@ import type { PointerValue } from '../../../../state/types'; import type { Rotation } from '../../../../utils/common'; import { isNil } from '../../../../utils/common'; import { isValidPointerOverEvent } from '../../../../utils/events'; -import type { IndexedGeometry, PointGeometry } from '../../../../utils/geometry'; +import { isPointGeometry, type IndexedGeometry, type PointGeometry } from '../../../../utils/geometry'; import type { Point } from '../../../../utils/point'; import type { SeriesCompareFn } from '../../../../utils/series_sort'; import { isLineAreaPointWithinPanel, isPointOnGeometry } from '../../rendering/utils'; @@ -121,17 +121,27 @@ function getTooltipAndHighlightFromValue( return EMPTY_VALUES; } + const legendSeriesSortFn: SeriesCompareFn = (a, b) => { + const aDs = seriesIdentifierDataSeriesMap[a.key]; + const bDs = seriesIdentifierDataSeriesMap[b.key]; + return defaultXYLegendSeriesSort(aDs, bDs); + }; + // build the tooltip value list let header: PointerValue | null = null; const highlightedGeometries: IndexedGeometry[] = []; const highlightedPoints: PointGeometry[] = []; + const hoveredPointsMap = new Map(); const xValues = new Set(); const hideNullValues = !tooltip.showNullValues; const values = matchingGeoms .slice() .sort((a, b) => { - // presort matchingGeoms to group by series then y value to prevent flipping - return b.seriesIdentifier.key.localeCompare(a.seriesIdentifier.key) || b.value.y - a.value.y; + // pre-sort matchingGeoms to group by series and sortingOrder then y value to prevent flipping + const seriesSort = legendSeriesSortFn(a.seriesIdentifier, b.seriesIdentifier); + if (seriesSort !== 0) return seriesSort; + // Within same series, sort by y value (for stability) + return b.value.y - a.value.y; }) .reduce((acc, indexedGeometry) => { if (hideNullValues && indexedGeometry.value.y === null) { @@ -163,9 +173,32 @@ function getTooltipAndHighlightFromValue( const isLineAreaPoint = isLineAreaPointWithinPanel(spec, indexedGeometry); if (isGeometryHovered) { - // Pointer is on geometry and geometry is area/line point -> hover highlight isTooltipHighlighted = true; - highlightedGeometries.push(indexedGeometry); + + if (isPointGeometry(indexedGeometry)) { + // If Point Geometries overlap, then highlight the one with the highest sortingOrder + const hoveredPointKey = `${indexedGeometry.x}_${indexedGeometry.y}_${indexedGeometry.radius}`; + const existingGeom = hoveredPointsMap.get(hoveredPointKey); + + if (!existingGeom) { + // No existing geometry -> add the current one + hoveredPointsMap.set(hoveredPointKey, { geom: indexedGeometry, index: highlightedGeometries.length }); + highlightedGeometries.push(indexedGeometry); + } else { + // Already an existing geometry -> check if the current one has a higher sortingOrder + const currentDs = seriesIdentifierDataSeriesMap[indexedGeometry.seriesIdentifier.key]; + const existingDs = seriesIdentifierDataSeriesMap[existingGeom.geom.seriesIdentifier.key]; + const shouldReplaceExistingGeometry = + currentDs && existingDs && currentDs.sortOrder > existingDs.sortOrder; + + if (shouldReplaceExistingGeometry) { + hoveredPointsMap.set(hoveredPointKey, { geom: indexedGeometry, index: existingGeom.index }); + highlightedGeometries[existingGeom.index] = indexedGeometry; + } + } + } else { + highlightedGeometries.push(indexedGeometry); + } } if (isLineAreaPoint) { // Geometry is area/line point -> bucket highlight @@ -200,12 +233,7 @@ function getTooltipAndHighlightFromValue( header = null; } - const baseTooltipSortFn: SeriesCompareFn = (a, b) => { - const aDs = seriesIdentifierDataSeriesMap[a.key]; - const bDs = seriesIdentifierDataSeriesMap[b.key]; - return defaultXYLegendSeriesSort(aDs, bDs); - }; - const tooltipSortFn = tooltip.sort ?? settings.legendSort ?? baseTooltipSortFn; + const tooltipSortFn = tooltip.sort ?? settings.legendSort ?? legendSeriesSortFn; const sortedTooltipValues = values.sort((a, b) => { return tooltipSortFn(a.seriesIdentifier, b.seriesIdentifier); }); diff --git a/storybook/stories/test_cases/10_highlighter_z_index.story.tsx b/storybook/stories/test_cases/10_highlighter_z_index.story.tsx index 92bf134324..3492e319c5 100644 --- a/storybook/stories/test_cases/10_highlighter_z_index.story.tsx +++ b/storybook/stories/test_cases/10_highlighter_z_index.story.tsx @@ -49,7 +49,3 @@ export const Example: ChartsStory = (_, { title, description }) => { ); }; - -Example.parameters = { - markdown: 'Currently not correctly rendered due to [#1921](https://github.com/elastic/elastic-charts/issues/1921)', -};