fix(Tooltip): tooltip display exception when setting tooltip.valueFor…#428
fix(Tooltip): tooltip display exception when setting tooltip.valueFor…#428ader-h wants to merge 1 commit intoopentiny:devfrom
Conversation
WalkthroughThe changes introduce support for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/BarChart/handleSeries.js`:
- Around line 680-687: The current branch directly assigns
iChartOption.tooltip.valueFormatter into baseOption.tooltip.valueFormatter,
bypassing setLimitFormatter's normalization (which filters 'Placeholder' series
and restores true values for % barMinHeight). Instead, keep using
setLimitFormatter(baseOption.tooltip.formatter, ...) to produce the sanitized
tooltip formatter and then wrap that result so the user-provided valueFormatter
(iChartOption.tooltip.valueFormatter) is applied to the final displayed value;
e.g., call setLimitFormatter with the existing baseOption.tooltip.formatter,
preserve exclude/barMinHeight/colors context, and compose the returned formatter
with valueFormatter so placeholders are removed and min-height-corrected values
are passed into valueFormatter before showing.
In `@src/components/LineChart/handleOptipn.js`:
- Around line 169-180: The current change replaces the existing tooltip wrapper
when valueFormatter is present, skipping the normalization/filtering logic;
instead, keep the wrapper in place (the function assigned to
baseOpt.tooltip.formatter that uses isFilter, legendData, coverObjDataToInit,
formatter, defaultFormatter) and apply tooltip?.valueFormatter within that
wrapper before returning/rendering, i.e., call the configured valueFormatter on
the normalized initParams (and pass through ticket/callback) rather than
replacing the whole formatter—ensure baseOpt.tooltip.valueFormatter is used
inside the existing formatter function so trimming helper series and converting
threshold objects still occur.
In `@src/components/PieChart/handleOptipn.js`:
- Around line 64-71: Don't short-circuit the tooltip rendering by setting
baseOpt.tooltip.valueFormatter; instead always assign baseOpt.tooltip.formatter
and inside that function build initParams (use coverObjDataToInit when params is
array), then if formatter exists call it, otherwise call defaultFormatter but
pass-through the original valueFormatter so defaultFormatter can apply custom
HTML, hideEmpty, tooltip.order and adaptive behavior; update the call site to
defaultFormatter(initParams, color, iChartOpt, baseOpt.tooltip?.hideEmpty,
baseOpt.tooltip, valueFormatter) (or update defaultFormatter's signature to
accept a valueFormatter parameter) and remove the separate
baseOpt.tooltip.valueFormatter branch so valueFormatter only affects value
formatting, not the entire renderer.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0a2b355b-b643-42f5-a15d-d69d45a4690a
📒 Files selected for processing (3)
src/components/BarChart/handleSeries.jssrc/components/LineChart/handleOptipn.jssrc/components/PieChart/handleOptipn.js
| const valueFormatter = iChartOption.tooltip?.valueFormatter; | ||
| const toolTipFormatter = baseOption.tooltip.formatter; | ||
| const exclude = ['Placeholder']; | ||
| const colors = baseOption.color; | ||
| const barMinHeight = iChartOption.itemStyle && iChartOption.itemStyle.barMinHeight; | ||
| baseOption.tooltip.formatter = (params, ticket, callback) => { | ||
| const newParams = params.filter(item => { | ||
| return exclude.indexOf(item.seriesName) === -1; | ||
| }); | ||
| // 如果设置了最小高度高度，并按%计算，将newParams值重新校正 | ||
| if (barMinHeight && barMinHeight.toString().indexOf('%') !== -1) { | ||
| newParams.forEach((item) => { | ||
| if (iChartOption.data && iChartOption.data[item.dataIndex] && isNumber(iChartOption.data[item.dataIndex][item.seriesName])) { | ||
| item.data = item.value = iChartOption.data[item.dataIndex][item.seriesName] | ||
| } | ||
| }) | ||
| } | ||
| if (toolTipFormatter) { | ||
| return toolTipFormatter(newParams, ticket, callback); | ||
| } | ||
| const config = { | ||
| title: '', | ||
| children: [], | ||
| hideEmpty: baseOption.tooltip?.hideEmpty | ||
| } | ||
| newParams.forEach((item, index) => { | ||
| let value = item.value || seriesData[item.seriesName][item.dataIndex]; | ||
| if (isObject(item.data) && iChartOption.series) { | ||
| if (iChartOption.series[0]?.encode) { | ||
| value = item.data[item.encode.x[0]]; | ||
| } else { | ||
| value = seriesData[item.seriesName] && seriesData[item.seriesName][item.dataIndex]; | ||
| } | ||
| if (valueFormatter){ | ||
| baseOption.tooltip.valueFormatter = valueFormatter | ||
| } else { |
There was a problem hiding this comment.
Preserve BarChart tooltip sanitization when valueFormatter is configured.
setLimitFormatter() is not just a formatter hook: it strips synthetic Placeholder series and restores real values when %-based barMinHeight is used. This branch bypasses that wrapper completely, so charts using tooltip.valueFormatter will surface placeholder rows and inflated min-height values in the tooltip. Please keep the filter/normalization path and apply valueFormatter to the final displayed value instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/BarChart/handleSeries.js` around lines 680 - 687, The current
branch directly assigns iChartOption.tooltip.valueFormatter into
baseOption.tooltip.valueFormatter, bypassing setLimitFormatter's normalization
(which filters 'Placeholder' series and restores true values for %
barMinHeight). Instead, keep using
setLimitFormatter(baseOption.tooltip.formatter, ...) to produce the sanitized
tooltip formatter and then wrap that result so the user-provided valueFormatter
(iChartOption.tooltip.valueFormatter) is applied to the final displayed value;
e.g., call setLimitFormatter with the existing baseOption.tooltip.formatter,
preserve exclude/barMinHeight/colors context, and compose the returned formatter
with valueFormatter so placeholders are removed and min-height-corrected values
are passed into valueFormatter before showing.
| const valueFormatter = tooltip?.valueFormatter | ||
| if (valueFormatter) { | ||
| baseOpt.tooltip.valueFormatter = valueFormatter | ||
| } else { | ||
| baseOpt.tooltip.formatter = (echartsParams, ticket, callback) => { | ||
| let params = echartsParams | ||
| if (isFilter) { | ||
| const lineNumber = legendData.length | ||
| params = echartsParams.slice(0, lineNumber) | ||
| } | ||
| const initParams = coverObjDataToInit(params) | ||
| return formatter && typeof formatter === 'function' ? formatter(initParams, ticket, callback) : defaultFormatter(initParams, color, iChartOpt, baseOpt.tooltip?.hideEmpty, baseOpt.tooltip) |
There was a problem hiding this comment.
Keep the LineChart filtering/normalization wrapper even when valueFormatter is set.
This wrapper is doing more than formatting: it trims helper series for discrete/predict/filtered-area cases and converts threshold object data back to primitive values before rendering. The new early branch skips all of that, so enabling tooltip.valueFormatter will reintroduce extra series/raw object values in the tooltip and bypass the custom tooltip HTML. Please apply valueFormatter inside the existing wrapper instead of replacing it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/LineChart/handleOptipn.js` around lines 169 - 180, The current
change replaces the existing tooltip wrapper when valueFormatter is present,
skipping the normalization/filtering logic; instead, keep the wrapper in place
(the function assigned to baseOpt.tooltip.formatter that uses isFilter,
legendData, coverObjDataToInit, formatter, defaultFormatter) and apply
tooltip?.valueFormatter within that wrapper before returning/rendering, i.e.,
call the configured valueFormatter on the normalized initParams (and pass
through ticket/callback) rather than replacing the whole formatter—ensure
baseOpt.tooltip.valueFormatter is used inside the existing formatter function so
trimming helper series and converting threshold objects still occur.
| const valueFormatter = tooltip?.valueFormatter | ||
| if (valueFormatter) { | ||
| baseOpt.tooltip.valueFormatter = valueFormatter | ||
| } else { | ||
| baseOpt.tooltip.formatter = (params, ticket, callback) => { | ||
| const initParams = isArray(params) ? coverObjDataToInit(params) : params; | ||
| return formatter && typeof formatter === 'function' ? formatter(initParams, ticket, callback) : defaultFormatter(initParams, color, iChartOpt, baseOpt.tooltip?.hideEmpty, baseOpt.tooltip) | ||
| } |
There was a problem hiding this comment.
Don't bypass PieChart's custom tooltip renderer for valueFormatter.
defaultFormatter() is also where this component applies the custom tooltip HTML, hideEmpty, tooltip.order, and adaptive/mobile behavior. This branch skips that path entirely, so setting tooltip.valueFormatter changes the whole tooltip rendering instead of only formatting the value. Please thread valueFormatter into the existing formatter/defaultFormatter flow rather than short-circuiting it here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/PieChart/handleOptipn.js` around lines 64 - 71, Don't
short-circuit the tooltip rendering by setting baseOpt.tooltip.valueFormatter;
instead always assign baseOpt.tooltip.formatter and inside that function build
initParams (use coverObjDataToInit when params is array), then if formatter
exists call it, otherwise call defaultFormatter but pass-through the original
valueFormatter so defaultFormatter can apply custom HTML, hideEmpty,
tooltip.order and adaptive behavior; update the call site to
defaultFormatter(initParams, color, iChartOpt, baseOpt.tooltip?.hideEmpty,
baseOpt.tooltip, valueFormatter) (or update defaultFormatter's signature to
accept a valueFormatter parameter) and remove the separate
baseOpt.tooltip.valueFormatter branch so valueFormatter only affects value
formatting, not the entire renderer.
1 participant
…matter
Summary by CodeRabbit
New Features
valueFormatteroption for chart tooltips, enabling direct value formatting in BarChart, LineChart, and PieChart components.Refactor