You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "MannXo (via GitHub)" <gi...@apache.org> on 2026/08/04 15:05:02 UTC
[PR] fix(plugin-chart-echarts): omit stacked value labels on zero-height segments [superset]
MannXo opened a new pull request, #42756:
URL: https://github.com/apache/superset/pull/42756
### SUMMARY
Fixes #42702.
On a stacked `echarts_timeseries_bar` with "Show Value" on and "Only Total" off, a series whose value is `0` for a category still gets a value label. A zero-height stacked segment begins and ends at the same coordinate as the top of the segment beneath it, so echarts draws that label on top of the label belonging to the segment below, and the two numbers render over each other as unreadable text.
This adds one guard: a stacked segment with no height carries no label.
**One correction to the root cause in the issue.** The issue reports that with the default `percentage_threshold: 0` the condition becomes `numericValue >= 0`. It does not. `0` is falsy, so `thresholdValues[dataIndex] || Number.MIN_SAFE_INTEGER` evaluates to `Number.MIN_SAFE_INTEGER` and the condition is `numericValue >= Number.MIN_SAFE_INTEGER`, which is true for everything. Observed against the label formatter:
```
thresholdValues=[0] 0 -> "0" -5 -> "-5" 32 -> "32"
thresholdValues=[10] 0 -> "" 5 -> "" 50 -> "50"
```
If the mechanism were `>= 0`, that `-5` would have been dropped. So the `||` is doing its job: a `0` threshold means "no threshold filtering", and a real threshold already suppresses zeros. That makes this a label-collision bug rather than a threshold bug, and it is why the fix does not touch the threshold expression. Rewriting that comparison would change threshold semantics for every user and would suppress legitimate negative labels.
**A second defect on the same code path.** A `null` value renders the literal string `"null"` as a chart label, with the production number formatter:
```
0 -> "0" null -> "null" 32 -> "32" -5 -> "-5"
```
A null segment has no height either, so it collides identically. The guard covers both under one rule rather than special-casing zero.
**Prior art in this plugin.** `Timeseries/transformProps.ts` already omits zero observations from the rich tooltip of a stacked series (`if (value.observation === 0 && stack) return;`). This applies the same judgement to the per-series value label.
Scope: stacked series only. Unstacked labels sit on the bar itself with nothing to collide with, and they return earlier in the formatter. Only-total labels render the stack total and are unaffected. The `stack &&` clause is deliberate for legibility even though the earlier `!stack` return makes it defensive rather than load-bearing, matching how the tooltip guard above spells out the same condition.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Stacked bar, two metrics, "Show Value" on, "Only Total" off, default percentage threshold, with the upper series at `0` for two categories.
<!-- BEFORE -->
<!-- AFTER -->
The label coordinates from the rendered SVG, so the images and the numbers agree:
```
before after
x=146.3 y=143.0 '0' x=146.3 y=143.0 '32'
x=146.3 y=143.0 '32' x=775.2 y=179.2 '27'
x=775.2 y=179.2 '0'
x=775.2 y=179.2 '27'
overlapping pairs: ('0','32'), ('0','27') overlapping pairs: none
```
### TESTING INSTRUCTIONS
Manual:
1. Create an `echarts_timeseries_bar` chart with two metrics, for example `A` and `B`.
2. Set Stacked, turn on "Show Value", leave "Only Total" off and "Percentage threshold" at its default.
3. Include a category where the metric stacked on top is `0` and the one below it is not, for example `{"category": "1-3g", "B": 32, "A": 0}`.
4. Run the chart. Before this change the `0` label is drawn over the `32` label as doubled text. After it, only `32` is labelled.
5. Confirm the unchanged cases: a category where both metrics are non-zero still labels both segments; switching off Stacked still labels a `0`; turning on "Only Total" still shows the stack total.
Automated, in `plugins/plugin-chart-echarts/test/Timeseries/transformers.test.ts`:
```
npx jest plugins/plugin-chart-echarts/test/Timeseries/transformers.test.ts
```
Five cases cover zero and null suppressed when stacked, non-zero and negative kept, zero kept when not stacked, `percentage_threshold` still filtering below-threshold values, and only-total labels unchanged.
Gates run locally on Node 24.16.0, using the commands from `.github/workflows/superset-frontend.yml`:
```
npx jest plugins/plugin-chart-echarts 69 suites, 772 tests, all passing
npm run lint clean
npm run plugins:build && npm run type clean
npm ls --all --package-lock-only --depth=0 clean
pre-commit run --files <the two changed files> all hooks passing
```
Each new assertion was checked by reverting the behaviour it covers and confirming the intended test fails: removing the guard fails the zero-height case, and guarding zero without null fails it too. One further sabotage, removing the `stack &&` clause, is *not* caught, because the `!stack` branch returns before the guard is reached, so no test can distinguish it. Noting that rather than presenting the set as complete.
### ADDITIONAL INFORMATION
- [x] Has associated issue: #42702
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): omit stacked value labels on zero-height segments [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe merged PR #42756:
URL: https://github.com/apache/superset/pull/42756
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): omit stacked value labels on zero-height segments [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #42756:
URL: https://github.com/apache/superset/pull/42756#issuecomment-5181336189
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/42756?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 65.59%. Comparing base ([`e66c7fc`](https://app.codecov.io/gh/apache/superset/commit/e66c7fcb638eccc54248980c68c506b216a26f5e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`20b8ee2`](https://app.codecov.io/gh/apache/superset/commit/20b8ee2a0f92a5c9b51fcc6c7bf8a22373b0269f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #42756 +/- ##
==========================================
- Coverage 65.59% 65.59% -0.01%
==========================================
Files 2819 2819
Lines 160166 160168 +2
Branches 36569 36570 +1
==========================================
- Hits 105059 105055 -4
- Misses 53059 53065 +6
Partials 2048 2048
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/42756/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/42756/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `71.74% <100.00%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/42756?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(plugin-chart-echarts): omit stacked value labels on zero-height segments [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42756:
URL: https://github.com/apache/superset/pull/42756#issuecomment-5180900476
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org