Skip to content

fix(big-number): guard against null colorPicker in transformProps#39110

Open
rusackas wants to merge 2 commits intomasterfrom
fix/bignumber-colorpicker-null-crash
Open

fix(big-number): guard against null colorPicker in transformProps#39110
rusackas wants to merge 2 commits intomasterfrom
fix/bignumber-colorpicker-null-crash

Conversation

@rusackas
Copy link
Copy Markdown
Member

@rusackas rusackas commented Apr 4, 2026

SUMMARY

`colorPicker` defaults to `null` when a Big Number chart is first created and the colour-picker control has never been set. Destructuring it directly crashes:

```
TypeError: Cannot destructure property 'r' of 'colorPicker' as it is null
```

Falls back to the default teal (`rgb(0, 122, 135)`) when `colorPicker` is null or undefined.

```ts
// before
const { r, g, b } = colorPicker;
// after
const { r = 0, g = 122, b = 135 } = colorPicker ?? {};
```

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before: Big Number chart crashes on render when `colorPicker` is null (newly created chart or form data created without the colour picker value).

After: Chart renders with the default teal colour; user-configured colours continue to work as before.

TESTING INSTRUCTIONS

  1. Create a new Big Number chart against any dataset
  2. Do not set a custom colour in the Customize tab
  3. Confirm the chart renders without a TypeError crash
  4. Set a custom colour → confirm it is applied correctly

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration
  • Introduces new feature or API
  • Removes existing feature or API

@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented Apr 4, 2026 •
edited
Loading

Code Review Agent Run #699483

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: 536d30b..536d30b
    • superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/transformProps.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

@dosubot dosubot Bot added the viz:charts:bignumber Related to BigNumber charts label Apr 4, 2026
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 4, 2026

✅ Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit 0a646e7
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/69d147594b49f500089932d8
😎 Deploy Preview https://deploy-preview-39110--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@aminghadersohi @claude
fix(big-number): guard against null colorPicker in transformProps
536d30b 
colorPicker defaults to null when the colour-picker control has never been
set (e.g. a freshly-created Big Number chart). Destructuring it directly
crashes:

  TypeError: Cannot destructure property 'r' of 'colorPicker' as it is null

When colorPicker is null/undefined, mainColor is now left undefined so that
BigNumberViz falls back to its existing BRAND_COLOR default. The ECharts
trendline options fall back to BRAND_COLOR explicitly since they require a
string value.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@rusackas rusackas force-pushed the fix/bignumber-colorpicker-null-crash branch from 0a646e7 to 536d30b Compare April 4, 2026 17:21
@pull-request-size pull-request-size Bot added size/S and removed size/XS labels Apr 4, 2026
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented Apr 4, 2026 •
edited
Loading

Code Review Agent Run #94ada3

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: 536d30b..536d30b
    • superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/transformProps.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

@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented Apr 7, 2026

Yes, adding a regression test for mainColor being undefined is a good idea. The code now falls back to BRAND_COLOR when colorPicker is falsy, and testing this ensures the fallback works as expected. This would cover the scenario at the updated lines around 290-295.

@claude
test(big-number): add regression test for null colorPicker in transfo…
35679e1 
…rmProps

Adds BRAND_COLOR to the @superset-ui/core mock and a test verifying
that transformProps does not crash when colorPicker is null, and that
mainColor is undefined (so the ?? BRAND_COLOR fallback takes effect).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@pull-request-size pull-request-size Bot added size/M and removed size/S labels Apr 16, 2026
@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented Apr 16, 2026 •
edited
Loading

Code Review Agent Run #9b503b

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: 536d30b..35679e1
    • superset-frontend/plugins/plugin-chart-echarts/src/BigNumber/BigNumberWithTrendline/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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sadpandajoe sadpandajoe sadpandajoe left review comments

@hainenber hainenber Awaiting requested review from hainenber

@justinpark justinpark Awaiting requested review from justinpark

@mistercrunch mistercrunch Awaiting requested review from mistercrunch

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

plugins size/M viz:charts:bignumber Related to BigNumber charts

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rusackas @sadpandajoe @aminghadersohi @claude