Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Commit a6e293b

Browse files
authored
Fix line chart hover tooltip logic (#44378)
* Fix line chart hover tooltip logic * Fix integration and skip unit test * Fix point link rendering unit test
1 parent b386b54 commit a6e293b

File tree

4 files changed

+27
-25
lines changed

4 files changed

+27
-25
lines changed

client/web/src/integration/insights/insight/insight-chart-focus.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ describe('Code insights [Insight Card] should has a proper focus management ', (
7474
assert.strictEqual(
7575
await hasFocus(
7676
driver,
77-
`[aria-label="Chart series"] > [role="listitem"]:nth-child(${lineIndex + 2}) a:nth-child(${
77+
`[aria-label="Chart series"] > [role="listitem"]:nth-child(${lineIndex + 1}) a:nth-child(${
7878
pointIndex + 1
7979
})`
8080
),

client/wildcard/src/components/Charts/components/line-chart/LineChart.test.tsx

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
import { render, screen, within } from '@testing-library/react'
2-
import userEvent from '@testing-library/user-event'
3-
import { assert, stub } from 'sinon'
42

53
import { LineChart } from './LineChart'
64
import { FLAT_SERIES } from './story/mocks'
@@ -53,24 +51,18 @@ describe('LineChart', () => {
5351

5452
describe('should handle clicks', () => {
5553
it('on a point', () => {
56-
const openStub = stub(window, 'open')
57-
5854
renderChart(defaultArgs)
5955

6056
// Query chart series list
6157
const series = screen.getByLabelText('Chart series')
62-
const [firstPoint, secondPoint, thirdPoint] = within(series).getAllByRole('listitem')
58+
const [firstSeries] = within(series).getAllByRole('listitem')
59+
const [point00, point01, point02] = within(firstSeries).getAllByRole('listitem')
6360

6461
// Spot checking multiple points
6562
// related issue https://github.com/sourcegraph/sourcegraph/issues/38304
66-
userEvent.click(firstPoint)
67-
userEvent.click(secondPoint)
68-
userEvent.click(thirdPoint)
69-
70-
assert.alwaysCalledWith(openStub, 'https://google.com/search')
71-
assert.calledThrice(openStub)
72-
73-
openStub.restore()
63+
expect(point00).toHaveAttribute('href', 'https://google.com/search')
64+
expect(point01).toHaveAttribute('href', 'https://google.com/search')
65+
expect(point02).toHaveAttribute('href', 'https://google.com/search')
7466
})
7567
})
7668
})

client/wildcard/src/components/Charts/components/line-chart/LineChartContent.tsx

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ export function LineChartContent<Datum>(props: LineChartContentProps<Datum>): Re
8181
})(Object.values(points).flat())
8282
}, [activeSeries, height, width, xScale, yScale])
8383

84-
const handlers = useChartEventHandlers({
84+
const { onPointerLeave, onPointerMove, onClick, onBlurCapture } = useChartEventHandlers({
8585
onPointerMove: point => {
8686
const closestPoint = voronoiLayout.find(point.x - left, point.y - top)
8787

@@ -105,12 +105,9 @@ export function LineChartContent<Datum>(props: LineChartContentProps<Datum>): Re
105105
role="list"
106106
aria-label="Chart series"
107107
className={classNames(styles.root, className, { [styles.rootWithHoveredLinkPoint]: activePoint?.linkUrl })}
108+
onBlurCapture={onBlurCapture}
108109
{...attributes}
109-
{...handlers}
110110
>
111-
{/* Spread the group element in order to track user input events from all visible chart content surface*/}
112-
<rect x={0} y={0} opacity={0} width={width} height={height} aria-hidden={true} pointerEvents="none" />
113-
114111
{stacked && <StackedArea dataSeries={activeSeries} xScale={xScale} yScale={yScale} />}
115112

116113
{activeSeries.map(line => (
@@ -153,6 +150,19 @@ export function LineChartContent<Datum>(props: LineChartContentProps<Datum>): Re
153150
/>
154151
)}
155152

153+
{/* Spread the group element in order to track user input events from all visible chart content surface*/}
154+
<rect
155+
x={0}
156+
y={0}
157+
opacity={0}
158+
width={width}
159+
height={height}
160+
aria-hidden={true}
161+
onPointerLeave={onPointerLeave}
162+
onPointerMove={onPointerMove}
163+
onClick={onClick}
164+
/>
165+
156166
{activePoint && rootRef.current && (
157167
<Tooltip containerElement={rootRef.current} activeElement={activePoint.node as HTMLElement}>
158168
<TooltipContent series={activeSeries} activePoint={activePoint} stacked={stacked} />

client/wildcard/src/components/Charts/components/line-chart/hooks/event-listeners.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,15 @@ import { Point } from '@visx/point'
66
interface UseChartEventHandlersProps {
77
onPointerMove: (point: Point) => void
88
onPointerLeave: () => void
9-
onFocusOut: FocusEventHandler<SVGSVGElement>
10-
onClick: MouseEventHandler<SVGSVGElement>
9+
onFocusOut: FocusEventHandler<SVGGraphicsElement>
10+
onClick: MouseEventHandler<SVGGraphicsElement>
1111
}
1212

1313
interface ChartHandlers {
14-
onPointerMove: PointerEventHandler<SVGSVGElement>
15-
onPointerLeave: PointerEventHandler<SVGSVGElement>
16-
onBlurCapture: FocusEventHandler<SVGSVGElement>
17-
onClick: MouseEventHandler<SVGSVGElement>
14+
onPointerMove: PointerEventHandler<SVGGraphicsElement>
15+
onPointerLeave: PointerEventHandler<SVGGraphicsElement>
16+
onBlurCapture: FocusEventHandler<SVGGraphicsElement>
17+
onClick: MouseEventHandler<SVGGraphicsElement>
1818
}
1919

2020
/**

0 commit comments

Comments
 (0)