feat(chart): dim non-hovered series rows in tooltip by lkipke · Pull Request #447 · cloudflare/kumo

lkipke · 2026-04-24T12:54:04Z

When hovering a series on a bar or line chart, the tooltip now dims rows for other series to 50% opacity, matching the emphasis dimming that ECharts already applies to the chart elements themselves.

Reviews
- bonk has reviewed the change
- automated review not possible because:
Tests
- Tests included/updated
- Automated tests not possible - manual testing has been completed as follows: Verified dimming works for both line and bar charts (see screenshots)
- Additional testing not necessary because:

When hovering a series on a bar or line chart, the tooltip now dims rows for other series to 50% opacity, matching the emphasis dimming that ECharts already applies to the chart elements themselves.

pkg-pr-new · 2026-04-24T12:59:53Z

npm i https://pkg.pr.new/@cloudflare/kumo@447

commit: d49b1cd

github-actions · 2026-04-24T13:00:40Z

Docs Preview

View docs preview

Commit: d49b1cd

mattrothenberg

Oops, didn't mean to approve. Still reviewing

mattrothenberg · 2026-04-24T13:25:14Z

/bonk My other AI Agent wants to know: Why drive tooltip-row dimming from element-level mouseover/mouseout and a seriesName ref, when axis-trigger tooltips aren't element-activated. Does this silently no-op on line charts with showSymbol: false, flicker between adjacent bars of the same series, and read a ref from inside the formatter that ECharts has no way to keep in sync with tooltip re-renders? Would updateAxisPointer with series-index matching not be the correct signal here?

ask-bonk · 2026-04-24T14:10:35Z

@mattrothenberg Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

mattrothenberg · 2026-04-24T14:12:02Z

/bonk pls review

ask-bonk

Summary

This PR adds tooltip row dimming when hovering chart series. The visual goal is good, but the implementation has a critical synchronization issue that will cause the dimming to not work reliably or at all.

Critical Issues

1. Stale Closure in Tooltip Formatter (Primary Issue)

The dangerousHtmlFormatter is created inside useMemo and captures hoveredSeriesRef.current in its closure. When mouseover/mouseout update the ref, the formatter function does not get re-created because refs don't trigger re-renders and the useMemo dependencies don't include the hovered state.

Result: The formatter always sees the initial null value, so dimming never actually works.

2. Event Mismatch with Axis-Triggered Tooltips

The tooltip uses trigger: 'axis', meaning tooltips appear on axis hover, not element hover. However, mouseover/mouseout events fire when hovering specific graphical elements.

Problems:

Line charts with showSymbol: false: No symbols to hover, so mouseover events may not fire for the tooltip to dim rows
Bar charts: Rapidly moving between adjacent bars of the same series can cause flicker (mouseout clears, then mouseover sets)
The tooltip renders before/without element mouseover, leaving rows undimmed during axis hover

3. Incomplete Incomplete-Series Config

The incomplete-series config adds triggerLineEvent: true but this doesn't work for bar series (bar charts don't have lines). The hover tracking won't work consistently across chart types.

Suggested Approach

Consider using updateAxisPointer event instead, which fires consistently for axis-triggered tooltips. Match the axis pointer's series index to dim the correct tooltip row:

handlers.updateAxisPointer = (params: any) => {
  // params.dataIndex gives the hovered data index
  // Compare with series data to determine which series is 'active'
  hoveredSeriesRef.current = findSeriesAtDataIndex(params.dataIndex);
};

Or use React state with a throttled update to ensure the formatter re-renders when hover changes.

Questions

Have you verified this works on line charts with showSymbol: false?
Does the dimming update when hovering between bars of different series?
Would the emphasis.focus: 'series' state from ECharts be more reliable than tracking mouse events?

ask-bonk · 2026-04-24T14:14:09Z

              ? `<div style="font-weight:600;margin-bottom:4px;">${echarts.format.encodeHTML(formatTimestamp(ts))}</div>`
              : "";

+          const hovered = hoveredSeriesRef.current;


Critical issue: This ref read happens inside a useMemo closure. The formatter captures the initial null value and never updates because:

Ref updates don't trigger re-renders

The useMemo deps don't include hover state

The formatter function is the same closure from initial render

Result: hovered is always null, so dimming never actually occurs.

To fix, you'd need to either:

Use React state + add it to the useMemo deps (with throttling to avoid perf issues)

Or trigger chart.setOption() manually when hover changes to force formatter re-evaluation

ask-bonk · 2026-04-24T14:14:09Z

+      };
+    }
+
+    // Track which series is hovered so the tooltip formatter can dim


Issue: With trigger: 'axis' tooltips, these element-level events may not fire consistently:

Line charts: With showSymbol: false, no symbols exist to receive mouseover

Bar charts: Rapid movement between adjacent bars causes flicker (clear → set → clear)

Axis hover: Tooltip shows on axis hover without any element mouseover, leaving rows undimmed

Consider using updateAxisPointer event which fires consistently for axis-triggered tooltips:

ask-bonk · 2026-04-24T14:14:09Z

        showSymbol: false,
-        emphasis: { focus: "series" as const },
+        symbolSize: 14,
+        itemStyle: { opacity: 0 },


triggerLineEvent: true only works for line series. For bar charts (type === 'bar'), this has no effect. The hover tracking won't work consistently across chart types.

For bar charts, you'd need a different approach - perhaps updateAxisPointer + matching the data index to the hovered bar's series.

ask-bonk · 2026-04-24T14:14:09Z

+            type: "line",
+            // Render invisible symbols at each data point with a larger
+            // radius so they act as wider hover targets around the line.
+            showSymbol: false,


With showSymbol: false, there are no symbols to receive mouseover events. Combined with triggerLineEvent: true, the line itself can receive events, but this requires the user to hover exactly on the line (1-2px wide).

Consider whether this provides enough hover area for a good user experience, or if symbolSize: 14 with invisible symbols would create better hover targets.

ask-bonk · 2026-04-24T14:14:09Z

+                hovered != null && param.seriesName !== hovered;
+              const style = isDimmed ? ' style="opacity:0.5"' : "";
+
+              return `<div${style}>${param.marker} ${echarts.format.encodeHTML(param.seriesName)}: <strong>${formattedValue}</strong></div>`;


Changing from <br/> to <div> wrapping changes the vertical spacing of tooltip rows. Is this intentional? It adds block-level layout which may affect spacing differently than the inline <br/> approach.

If this is just for styling, consider keeping <br/> and using CSS on the tooltip container instead, or verify the visual result matches the intended design.

ask-bonk · 2026-04-24T14:14:26Z

I've submitted a comprehensive review of PR #447. Here's a summary of the key findings:

Key Issues Identified

1. Stale Closure Bug (Critical)

The dangerousHtmlFormatter captures hoveredSeriesRef.current in its closure inside useMemo. Since ref updates don't trigger re-renders, the formatter always sees the initial null value, meaning the dimming never actually works.

2. Event Mismatch with Axis-Triggered Tooltips

The tooltip uses trigger: 'axis' but the implementation relies on mouseover/mouseout events which fire on graphical elements. This creates problems:

Line charts with showSymbol: false: No symbols to hover, so events don't fire
Bar charts: Flickering when moving between adjacent bars
Axis hover: Tooltip can show without any element mouseover

3. `triggerLineEvent` Doesn't Work for Bar Charts

This configuration only applies to line series, so hover tracking won't work consistently across chart types.

Conversation

lkipke commented Apr 24, 2026

Uh oh!

pkg-pr-new Bot commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Docs Preview

Uh oh!

mattrothenberg left a comment

Choose a reason for hiding this comment

Uh oh!

mattrothenberg commented Apr 24, 2026

Uh oh!

ask-bonk Bot commented Apr 24, 2026

Uh oh!

mattrothenberg commented Apr 24, 2026

Uh oh!

ask-bonk Bot left a comment

Choose a reason for hiding this comment

Summary

Critical Issues

1. Stale Closure in Tooltip Formatter (Primary Issue)

2. Event Mismatch with Axis-Triggered Tooltips

3. Incomplete Incomplete-Series Config

Suggested Approach

Questions

Uh oh!

ask-bonk Bot Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot commented Apr 24, 2026

Key Issues Identified

1. Stale Closure Bug (Critical)

2. Event Mismatch with Axis-Triggered Tooltips

3. triggerLineEvent Doesn't Work for Bar Charts

Suggested Solution

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pkg-pr-new Bot commented Apr 24, 2026 •

edited

Loading

github-actions Bot commented Apr 24, 2026 •

edited

Loading

3. `triggerLineEvent` Doesn't Work for Bar Charts