Skip to content

fix(deckgl): scatterplot fix categorical color #35537

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sadpandajoe merged 4 commits into from
Oct 14, 2025
Merged

fix(deckgl): scatterplot fix categorical color #35537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
796e566
Fix categorical color
DamianPendrak Oct 6, 2025
fc52739
Fall back to fixed color when no dimension selected
DamianPendrak Oct 6, 2025
45e3af3
Change the default color palette from fixed to categorical
DamianPendrak Oct 6, 2025
2f529d7
Code improvements
DamianPendrak Oct 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 29 additions & 15 deletions superset-frontend/plugins/legacy-preset-chart-deckgl/src/CategoricalDeckGLContainer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,27 @@ const CategoricalDeckGLContainer = (props: CategoricalDeckGLContainerProps) => {
switch (selectedColorScheme) {
case COLOR_SCHEME_TYPES.fixed_color: {
color = fd.color_picker || { r: 0, g: 0, b: 0, a: 100 };
const colorArray = [color.r, color.g, color.b, color.a * 255];

return data.map(d => ({
...d,
color: [color.r, color.g, color.b, color.a * 255],
}));
return data.map(d => ({ ...d, color: colorArray }));
}
case COLOR_SCHEME_TYPES.categorical_palette: {
if (!fd.dimension) {
const fallbackColor = fd.color_picker || {
r: 0,
g: 0,
b: 0,
a: 100,
};
const colorArray = [
fallbackColor.r,
Copy link
Copy Markdown
Contributor

@bito-code-review bito-code-review Bot Oct 6, 2025

Choose a reason for hiding this comment

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

Invalid alpha value range

The alpha value in the fallback color is set to 100, which when multiplied by 255 results in 25500, exceeding the valid alpha range (0-255). This will cause color rendering issues in DeckGLContainerStyledWrapper and downstream components. Change the alpha to 1 (0-1 range) to match the pattern used in getCategories function.

Code suggestion
Check the AI-generated fix before applying
Suggested change
fallbackColor.r,
a: 1,

Code Review Run #a4c27a

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

fallbackColor.g,
fallbackColor.b,
fallbackColor.a * 255,
];
return data.map(d => ({ ...d, color: colorArray }));
}

return data.map(d => ({
...d,
color: hexToRGB(colorFn(d.cat_color, fd.slice_id)),
Expand Down Expand Up @@ -190,17 +204,17 @@ const CategoricalDeckGLContainer = (props: CategoricalDeckGLContainerProps) => {
d.metric <= breakpoint.maxValue,
);

return {
...d,
color: breakpointForPoint
? [
breakpointForPoint?.color.r,
breakpointForPoint?.color.g,
breakpointForPoint?.color.b,
breakpointForPoint?.color.a * 255,
]
: defaultBreakpointColor,
};
if (breakpointForPoint) {
const pointColor = [
breakpointForPoint.color.r,
breakpointForPoint.color.g,
breakpointForPoint.color.b,
breakpointForPoint.color.a * 255,
];
return { ...d, color: pointColor };
}

return { ...d, color: defaultBreakpointColor };
});
}
default: {
Expand Down
8 changes: 4 additions & 4 deletions superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/buildQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,14 @@ export interface DeckScatterFormData
min_radius?: number;
max_radius?: number;
color_picker?: { r: number; g: number; b: number; a: number };
category_name?: string;
dimension?: string;
}

export default function buildQuery(formData: DeckScatterFormData) {
const {
spatial,
point_radius_fixed,
category_name,
dimension,
js_columns,
tooltip_contents,
} = formData;
Expand All @@ -67,8 +67,8 @@ export default function buildQuery(formData: DeckScatterFormData) {
const spatialColumns = getSpatialColumns(spatial);
let columns = [...(baseQueryObject.columns || []), ...spatialColumns];

if (category_name) {
columns.push(category_name);
if (dimension) {
columns.push(dimension);
}

const columnStrings = columns.map(col =>
Expand Down
5 changes: 1 addition & 4 deletions superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/controlPanel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import {
tooltipContents,
tooltipTemplate,
} from '../../utilities/Shared_DeckGL';
import { COLOR_SCHEME_TYPES } from '../../utilities/utils';

const config: ControlPanelConfig = {
onInit: controlState => ({
Expand Down Expand Up @@ -134,9 +133,7 @@ const config: ControlPanelConfig = {
controlSetRows: [
[legendPosition],
[legendFormat],
...generateDeckGLColorSchemeControls({
defaultSchemeType: COLOR_SCHEME_TYPES.fixed_color,
}),
...generateDeckGLColorSchemeControls({}),
],
Copy link
Copy Markdown
Contributor

@bito-code-review bito-code-review Bot Oct 6, 2025

Choose a reason for hiding this comment

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

Missing color scheme parameter

The PR incorrectly removes the defaultSchemeType: COLOR_SCHEME_TYPES.fixed_color parameter from the generateDeckGLColorSchemeControls call. This changes the default color scheme behavior from fixed_color to categorical_palette, which breaks the intended Scatter plot color configuration. Based on the function signature in Shared_DeckGL.tsx, the parameter is optional but has specific behavior implications. Restore the original parameter to maintain consistent color scheme behavior across Scatter plots.

Code suggestion
Check the AI-generated fix before applying
Suggested change
],
...generateDeckGLColorSchemeControls({
defaultSchemeType: COLOR_SCHEME_TYPES.fixed_color,
}),

Code Review Run #a4c27a

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

},
{
Expand Down
4 changes: 2 additions & 2 deletions superset-frontend/plugins/legacy-preset-chart-deckgl/src/layers/Scatter/transformProps.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ function processScatterData(

export default function transformProps(chartProps: ChartProps) {
const { rawFormData: formData } = chartProps;
const { spatial, point_radius_fixed, category_name, js_columns } =
const { spatial, point_radius_fixed, dimension, js_columns } =
formData as DeckScatterFormData;

const radiusMetricLabel = getMetricLabelFromFormData(point_radius_fixed);
Expand All @@ -104,7 +104,7 @@ export default function transformProps(chartProps: ChartProps) {
records,
spatial,
radiusMetricLabel,
category_name,
dimension,
js_columns,
);

Expand Down
Loading