fix(big-number): guard against null colorPicker in transformProps#39110
fix(big-number): guard against null colorPicker in transformProps#39110
Conversation
Code Review Agent Run #699483Actionable Suggestions - 0Review 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. |
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
force-pushed
the
fix/bignumber-colorpicker-null-crash
branch
from
0a646e7 to
536d30b
Compare
Code Review Agent Run #94ada3Actionable Suggestions - 0Review 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 |
| const mainColor = `rgb(${r}, ${g}, ${b})`; | ||
| const mainColor = colorPicker | ||
| ? `rgb(${colorPicker.r}, ${colorPicker.g}, ${colorPicker.b})` | ||
| : undefined; |
There was a problem hiding this comment.
Can we add a regression test where mainColor is undefined? I don't think our tests currently cover this, so this would go back to L295 where we should be getting brandColor.
|
Yes, adding a regression test for |
3 participants
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
ADDITIONAL INFORMATION