fix(plugin-chart-echarts): hide value labels on zero-height stacked segments - #42854
fix(plugin-chart-echarts): hide value labels on zero-height stacked segments#42854krishn1301 wants to merge 1 commit into
Conversation
…egments thresholdValues[dataIndex] is 0 when percentage_threshold is 0, which is falsy, so the guard fell back to Number.MIN_SAFE_INTEGER and every value passed. A zero-value series occupies no space in a stacked chart, so its label rendered on top of the adjacent segment's label. Skips the label when the value is exactly 0 while leaving negative values and non-zero thresholds untouched. Fixes apache#42702 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dosubot
Bot
added
change:frontend
Code Review Agent Run #2b2655Actionable Suggestions - 0Additional Suggestions - 1
Filtered by Review RulesBito filtered these suggestions based on rules created automatically for your feedback. Manage rules.
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
✅ Deploy Preview for superset-docs-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![codeant-ai-for-open-source[bot]](https://avatars.githubusercontent.com/u/151810221?s=60&v=4){kind=small}
| if ( | ||
| numericValue !== 0 && | ||
| numericValue >= | ||
| (thresholdValues[dataIndex] || Number.MIN_SAFE_INTEGER) | ||
| (thresholdValues[dataIndex] || Number.MIN_SAFE_INTEGER) | ||
| ) { |
There was a problem hiding this comment.
Suggestion: The zero-value guard is reached for non-stacked series when legendState marks the series as unselected, because those series bypass the earlier !stack && isSelectedLegend branch. This changes the existing behavior and suppresses zero labels even though the overlap problem only applies to stacked segments. Restrict the zero suppression to stacked series or preserve the non-stacked label path. [incorrect condition logic]
Severity Level: Minor 🧹
- ⚠️ Isolated unstacked series can lose zero-value labels.
- ⚠️ Legend double-click filtering affects label rendering.
- ⚠️ Stacked-chart overlap fix changes unstacked chart behavior.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts
**Line:** 473:477
**Comment:**
*Incorrect Condition Logic: The zero-value guard is reached for non-stacked series when `legendState` marks the series as unselected, because those series bypass the earlier `!stack && isSelectedLegend` branch. This changes the existing behavior and suppresses zero labels even though the overlap problem only applies to stacked segments. Restrict the zero suppression to stacked series or preserve the non-stacked label path.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
The flagged issue is correct. The current implementation unconditionally suppresses zero-value labels for all series, which incorrectly affects non-stacked charts where zero values should remain visible. To resolve this, the zero-value check should be conditional on whether the chart is stacked. Proposed FixUpdate the condition in if (
(stack && numericValue !== 0 || !stack) &&
numericValue >=
(thresholdValues[dataIndex] || Number.MIN_SAFE_INTEGER)
) {
return formatter(numericValue);
}This change ensures that zero-value suppression only applies to stacked series, preserving existing behavior for non-stacked charts. Please let me know if you would like me to check the remaining comments on this PR. superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts |
1 participant
SUMMARY
Fixes #42702.
On a stacked timeseries bar chart, a series whose value is
0occupies no vertical space, but its value label was still rendered — landing on top of the label of the segment next to it.The root cause is a falsy check, not the label logic itself. In
transformSeries:When
percentage_thresholdis0(the default),thresholdValues[dataIndex]is0, which is falsy, so the||falls through toNumber.MIN_SAFE_INTEGERand every value passes the guard, including0.This skips the label when the value is exactly
0, before the threshold comparison.I used
numericValue !== 0rather than thenumericValue > 0suggested in the issue:> 0would also hide labels on negative segments, which do occupy space and legitimately need a label. There's a regression test covering that case.TESTING INSTRUCTIONS
Four tests were added under
transformSeries › stacked value labels:0(fails without this change)0> 0regression)Manually: create a Stacked Timeseries Bar chart with two metrics where one returns
0for some x-values, turn on Show Value, and confirm the0labels no longer overlap the adjacent labels.ADDITIONAL INFORMATION