[codex] Fix empty heatmap cells leaking splitArea background#21574
[codex] Fix empty heatmap cells leaking splitArea background#21574
Conversation
|
Thanks for your contribution! The pull request is marked to be To reviewers: If this PR is going to be described in the changelog in the future release, please make sure this PR has one of the following labels: This message is shown because the PR description doesn't contain the document related template. |
There was a problem hiding this comment.
Pull request overview
This PR fixes inconsistent rendering of empty heatmap cells when splitArea is enabled by ensuring “empty” data points still produce placeholder rects (so axis split-area bands don’t show through), and adds a regression test to lock in the behavior.
Changes:
- Render placeholder heatmap rects for empty (NaN) values in cartesian and matrix coordinate systems, using the chart background color (fallback
tokens.color.neutral00) as the fill. - Update progressive rendering path to apply the same empty-cell fill behavior.
- Add a Jest regression test verifying empty cells are rendered and filled to prevent
splitAreabackground leakage.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/chart/heatmap/HeatmapView.ts |
Stops skipping empty values on cartesian/matrix heatmaps; renders placeholder rects with a background fill to prevent splitArea bleed-through (including in incremental rendering). |
test/ut/spec/series/heatmap.test.ts |
Adds regression coverage to ensure empty heatmap cells render as rects and use the expected fill, preventing split-area background leakage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
The changes brought by this PR can be previewed at: https://echarts.apache.org/examples/editor?version=PR-21574@adb71c1 |
Ovilia
left a comment
There was a problem hiding this comment.
Can you please provide a screenshot of the colors before and after the change?
src/chart/heatmap/HeatmapView.ts
Outdated
| this.group.removeAll(); | ||
|
|
||
| const coordSys = seriesModel.coordinateSystem; | ||
| const emptyCellFill: ZRColor = ecModel.get('backgroundColor') || tokens.color.neutral00; |
There was a problem hiding this comment.
It should probably use tokens.color.background instead.
There was a problem hiding this comment.
Updated in adb71c1 to use tokens.color.background as the fallback instead of tokens.color.neutral00.
|
Updated the fallback color in . I also re-ran the case locally with a high-contrast \ palette and a yellow chart background to make the visual difference obvious: before the fix, the empty cells showed the split-area colors directly; after the fix, the empty cells use the chart background instead. |
|
Updated the fallback color in I also re-ran the case locally with a high-contrast
|
Summary
Root Cause
Empty heatmap cells on cartesian-style heatmaps were skipped entirely. With
splitAreaenabled, the alternating axis background showed through those skipped cells, so adjacent empty cells could appear inconsistent.Validation
./node_modules/.bin/jest --config test/ut/jest.config.cjs --runInBand test/ut/spec/series/heatmap.test.ts./node_modules/.bin/eslint src/chart/heatmap/HeatmapView.ts test/ut/spec/series/heatmap.test.tsFixes #21290