Skip to content

fix(treemap): remove gaps between chart nodes#40181

Open
ya-nsh wants to merge 2 commits into
apache:masterfrom
ya-nsh:fix/36807-treemap-color-gaps
Open

fix(treemap): remove gaps between chart nodes#40181
ya-nsh wants to merge 2 commits into
apache:masterfrom
ya-nsh:fix/36807-treemap-color-gaps

Conversation

@ya-nsh
Copy link
Copy Markdown

@ya-nsh ya-nsh commented May 16, 2026

Summary

  • Remove Treemap node border/gap spacing so tiles render flush again
  • Add a transformProps regression test covering the root and child node item styles

Before and after screenshots

Before: treemap nodes showed visible seams from border and gap spacing.

Before: treemap nodes with visible gaps

After: tiles render flush with zero border and gap spacing, including the filtered-node path covered by the regression test.

After: treemap nodes rendered flush

Testing

  • npm run test -- plugins/plugin-chart-echarts/test/Treemap/transformProps.test.ts
  • npm run lint -- plugins/plugin-chart-echarts/src/Treemap/constants.ts plugins/plugin-chart-echarts/test/Treemap/transformProps.test.ts
  • uvx pre-commit run --files superset-frontend/plugins/plugin-chart-echarts/src/Treemap/constants.ts superset-frontend/plugins/plugin-chart-echarts/test/Treemap/transformProps.test.ts

Partial fix for #36807: this addresses the visible gap between Treemap nodes. The sequential color behavior described in the issue appears to be separate and is left unchanged here to keep this PR focused.

@dosubot dosubot Bot added the viz:charts:treemap Related to the Treemap chart label May 16, 2026
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented May 16, 2026

Code Review Agent Run #60a1dc

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 5ba60d6..5ba60d6
    • superset-frontend/plugins/plugin-chart-echarts/src/Treemap/constants.ts
    • superset-frontend/plugins/plugin-chart-echarts/test/Treemap/transformProps.test.ts
  • Files skipped - 0
  • Tools
    • Eslint (Linter) - ✔︎ Successful
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@bito-code-review
Copy link
Copy Markdown
Contributor

The PR comment file contains only 1 row of actual content (header row plus 1 data row). The suggestion about treemap border/gap constants appears to be the only comment in this PR. I'll analyze it and provide a code fix.

transformProps.ts

207:       // Filtered leaf node style override
208:       itemStyle: {
209:         colorAlpha: OpacityEnum.SemiTransparent,
210:         color: theme.colorText,
211:         borderColor: theme.colorBgBase,
212:         borderWidth: 2, // Hardcoded value causing gaps
213:       },

constants.ts

22: export const BORDER_WIDTH = 0;
23: export const GAP_WIDTH = 0;

@ya-nsh
Copy link
Copy Markdown
Author

ya-nsh commented May 16, 2026

Addressed the review note in c0d03ab: filtered, non-selected treemap leaves now use the same zero border/gap constants as default nodes, and the regression test covers the filtered state.\n\nValidation:\n- npm run test -- plugins/plugin-chart-echarts/test/Treemap/transformProps.test.ts\n- npm run lint -- plugins/plugin-chart-echarts/src/Treemap/transformProps.ts plugins/plugin-chart-echarts/test/Treemap/transformProps.test.ts\n- uvx pre-commit run --files superset-frontend/plugins/plugin-chart-echarts/src/Treemap/transformProps.ts superset-frontend/plugins/plugin-chart-echarts/test/Treemap/transformProps.test.ts

Copy link
Copy Markdown
Contributor

@bito-code-review bito-code-review Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Agent Run #60bd9f

Actionable Suggestions - 1
  • superset-frontend/plugins/plugin-chart-echarts/test/Treemap/transformProps.test.ts - 1
Review Details
  • Files reviewed - 2 · Commit Range: 5ba60d6..c0d03ab
    • superset-frontend/plugins/plugin-chart-echarts/src/Treemap/transformProps.ts
    • superset-frontend/plugins/plugin-chart-echarts/test/Treemap/transformProps.test.ts
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • Eslint (Linter) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@ya-nsh ya-nsh force-pushed the fix/36807-treemap-color-gaps branch from c0d03ab to 1ce1060 Compare May 16, 2026 18:23
@ya-nsh
Copy link
Copy Markdown
Author

ya-nsh commented May 16, 2026

Addressed the latest Bito review note in 1ce1060: the filtered-state regression test now asserts the dimmed-state properties (colorAlpha: OpacityEnum.SemiTransparent and label override) in addition to the zero border/gap values. I also resolved both bot review threads.

Validation:

  • npm run test -- plugins/plugin-chart-echarts/test/Treemap/transformProps.test.ts
  • npm run lint -- plugins/plugin-chart-echarts/test/Treemap/transformProps.test.ts
  • uvx pre-commit run --files superset-frontend/plugins/plugin-chart-echarts/src/Treemap/transformProps.ts superset-frontend/plugins/plugin-chart-echarts/test/Treemap/transformProps.test.ts

@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented May 16, 2026

Code Review Agent Run #1e4a27

Actionable Suggestions - 0
Review Details
  • Files reviewed - 3 · Commit Range: 5ba60d6..1ce1060
    • superset-frontend/plugins/plugin-chart-echarts/src/Treemap/constants.ts
    • superset-frontend/plugins/plugin-chart-echarts/src/Treemap/transformProps.ts
    • superset-frontend/plugins/plugin-chart-echarts/test/Treemap/transformProps.test.ts
  • Files skipped - 0
  • Tools
    • Eslint (Linter) - ✔︎ Successful
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.16%. Comparing base (8d2b655) to head (1ce1060).
⚠️ Report is 172 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #40181   +/-   ##
=======================================
  Coverage   64.16%   64.16%           
=======================================
  Files        2591     2591           
  Lines      138162   138162           
  Branches    32048    32048           
=======================================
+ Hits        88647    88650    +3     
+ Misses      47986    47983    -3     
  Partials     1529     1529           
Flag Coverage Δ
javascript 67.01% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@ya-nsh
Copy link
Copy Markdown
Author

ya-nsh commented May 20, 2026

Friendly review ping: this PR is ready for maintainer review when you get a chance. Known status: 4 check(s) currently failing; I can follow up if review points to needed changes.

Happy to adjust quickly if you want a different shape or narrower scope.

@SBIN2010
Copy link
Copy Markdown
Contributor

Thanks for your contribution to the project. Could you please add before and after screenshots to the pull request description?

@ya-nsh
Copy link
Copy Markdown
Author

ya-nsh commented May 24, 2026

Thanks, added a before and after visual section to the PR description with the gap/seam case and the flush rendering after this change.

Copy link
Copy Markdown
Contributor

@SBIN2010 SBIN2010 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks LGTM!

@SBIN2010 SBIN2010 added the merge-if-green If approved and tests are green, please go ahead and merge it for me label May 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-if-green If approved and tests are green, please go ahead and merge it for me plugins size/M viz:charts:treemap Related to the Treemap chart

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants