You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/06/27 09:24:18 UTC

(superset) branch fix/issue-20459-suggested-open-pr-test-first created (now 7255509f6be)

This is an automated email from the ASF dual-hosted git repository.

rusackas pushed a change to branch fix/issue-20459-suggested-open-pr-test-first
in repository https://gitbox.apache.org/repos/asf/superset.git


      at 7255509f6be fix(deck.gl): apply categorical scatterplot colors in Multiple Layers chart

This branch includes the following new commits:

     new 7255509f6be fix(deck.gl): apply categorical scatterplot colors in Multiple Layers chart

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(superset) 01/01: fix(deck.gl): apply categorical scatterplot colors in Multiple Layers chart

Posted by ru...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

rusackas pushed a commit to branch fix/issue-20459-suggested-open-pr-test-first
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 7255509f6be2625fd697612ff34a2dceea1c697d
Author: Evan <ev...@preset.io>
AuthorDate: Sat Jun 27 02:23:53 2026 -0700

    fix(deck.gl): apply categorical scatterplot colors in Multiple Layers chart
    
    Scatterplot (and path/arc) layers resolve their per-feature categorical,
    fixed, and breakpoint colors inside CategoricalDeckGLContainer when rendered
    on their own. The deck.gl Multiple Layers chart builds sublayers by calling
    the raw getLayer functions directly, bypassing that container, so categorical
    colors were dropped and every point fell back to the default color while the
    legend still showed the configured palette.
    
    Extract the color-resolution logic into a shared addColorToFeatures util,
    reuse it in CategoricalDeckGLContainer, and apply it to color-aware sublayers
    in the Multiple Layers chart so the configured colors are preserved.
    
    Fixes #20459
    
    Co-Authored-By: Claude Opus 4.8 <no...@anthropic.com>
---
 .../src/CategoricalDeckGLContainer.tsx             |  79 +----------
 .../src/Multi/Multi.color.test.tsx                 | 148 +++++++++++++++++++++
 .../preset-chart-deckgl/src/Multi/Multi.tsx        |  55 ++++++--
 .../preset-chart-deckgl/src/utils/addColor.test.ts |  83 ++++++++++++
 .../preset-chart-deckgl/src/utils/addColor.ts      | 113 ++++++++++++++++
 5 files changed, 393 insertions(+), 85 deletions(-)

diff --git a/superset-frontend/plugins/preset-chart-deckgl/src/CategoricalDeckGLContainer.tsx b/superset-frontend/plugins/preset-chart-deckgl/src/CategoricalDeckGLContainer.tsx
index bbcde5c0acd..80a092f272c 100644
--- a/superset-frontend/plugins/preset-chart-deckgl/src/CategoricalDeckGLContainer.tsx
+++ b/superset-frontend/plugins/preset-chart-deckgl/src/CategoricalDeckGLContainer.tsx
@@ -47,11 +47,11 @@ import {
   DeckGLContainerStyledWrapper,
 } from './DeckGLContainer';
 import { GetLayerType } from './factory';
-import { ColorBreakpointType, ColorType, Point } from './types';
+import { Point } from './types';
 import { TooltipProps } from './components/Tooltip';
 import { COLOR_SCHEME_TYPES, ColorSchemeType } from './utilities/utils';
 import { getColorBreakpointsBuckets } from './utils';
-import { DEFAULT_DECKGL_COLOR } from './utilities/Shared_DeckGL';
+import { addColorToFeatures } from './utils/addColor';
 
 const { getScale } = CategoricalColorNamespace;
 
@@ -150,80 +150,7 @@ const CategoricalDeckGLContainer = (props: CategoricalDeckGLContainerProps) => {
       data: JsonObject[],
       fd: QueryFormData,
       selectedColorScheme: ColorSchemeType,
-    ) => {
-      const appliedScheme = fd.color_scheme;
-      const colorFn = getScale(appliedScheme);
-      let color: ColorType;
-
-      switch (selectedColorScheme) {
-        case COLOR_SCHEME_TYPES.fixed_color: {
-          color = fd.color_picker || { r: 0, g: 0, b: 0, a: 1 };
-          const colorArray = [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: 1,
-            };
-            const colorArray = [
-              fallbackColor.r,
-              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)),
-          }));
-        }
-        case COLOR_SCHEME_TYPES.color_breakpoints: {
-          const defaultBreakpointColor = fd.default_breakpoint_color
-            ? [
-                fd.default_breakpoint_color.r,
-                fd.default_breakpoint_color.g,
-                fd.default_breakpoint_color.b,
-                fd.default_breakpoint_color.a * 255,
-              ]
-            : [
-                DEFAULT_DECKGL_COLOR.r,
-                DEFAULT_DECKGL_COLOR.g,
-                DEFAULT_DECKGL_COLOR.b,
-                DEFAULT_DECKGL_COLOR.a * 255,
-              ];
-          return data.map(d => {
-            const breakpointForPoint: ColorBreakpointType =
-              fd.color_breakpoints?.find(
-                (breakpoint: ColorBreakpointType) =>
-                  d.metric >= breakpoint.minValue &&
-                  d.metric <= breakpoint.maxValue,
-              );
-
-            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: {
-          return [];
-        }
-      }
-    },
+    ) => addColorToFeatures(data, fd, selectedColorScheme),
     [],
   );
 
diff --git a/superset-frontend/plugins/preset-chart-deckgl/src/Multi/Multi.color.test.tsx b/superset-frontend/plugins/preset-chart-deckgl/src/Multi/Multi.color.test.tsx
new file mode 100644
index 00000000000..c70f1168682
--- /dev/null
+++ b/superset-frontend/plugins/preset-chart-deckgl/src/Multi/Multi.color.test.tsx
@@ -0,0 +1,148 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { render, waitFor } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import { supersetTheme, ThemeProvider } from '@apache-superset/core/theme';
+import { Provider } from 'react-redux';
+import { configureStore } from '@reduxjs/toolkit';
+import { DatasourceType, SupersetClient } from '@superset-ui/core';
+import DeckMulti from './Multi';
+
+// Capture the layers handed to the DeckGL container so we can inspect the
+// per-feature colors that were resolved for each sublayer.
+const mockLayerCapture: { layers: any[] } = { layers: [] };
+jest.mock('../DeckGLContainer', () => ({
+  DeckGLContainerStyledWrapper: ({ layers }: any) => {
+    mockLayerCapture.layers = layers || [];
+    return <div data-test="deckgl-container">DeckGL Container Mock</div>;
+  },
+}));
+
+jest.mock('@superset-ui/core', () => ({
+  ...jest.requireActual('@superset-ui/core'),
+  SupersetClient: {
+    get: jest.fn(),
+  },
+}));
+
+const mockStore = configureStore({
+  reducer: {
+    dataMask: () => ({}),
+  },
+});
+
+const renderWithProviders = (component: React.ReactElement) =>
+  render(
+    <Provider store={mockStore}>
+      <ThemeProvider theme={supersetTheme}>{component}</ThemeProvider>
+    </Provider>,
+  );
+
+const SCATTER_SLICE_ID = 1;
+
+const props = {
+  formData: {
+    datasource: '1__table',
+    viz_type: 'deck_multi',
+    deck_slices: [SCATTER_SLICE_ID],
+    autozoom: false,
+    map_style: 'mapbox://styles/mapbox/light-v9',
+  },
+  payload: {
+    data: {
+      slices: [
+        {
+          slice_id: SCATTER_SLICE_ID,
+          form_data: {
+            viz_type: 'deck_scatter',
+            datasource: '1__table',
+            slice_id: SCATTER_SLICE_ID,
+            // categorical color configuration coming from the saved scatter chart
+            color_scheme_type: 'categorical_palette',
+            color_scheme: 'supersetColors',
+            dimension: 'category',
+          },
+        },
+      ],
+      features: {
+        deck_scatter: [],
+      },
+      mapboxApiKey: 'test-key',
+    },
+  },
+  setControlValue: jest.fn(),
+  viewport: { longitude: 0, latitude: 0, zoom: 1 },
+  onAddFilter: jest.fn(),
+  height: 600,
+  width: 800,
+  datasource: {
+    id: 1,
+    type: DatasourceType.Table,
+    name: 'test_datasource',
+    columns: [],
+    metrics: [],
+    columnFormats: {},
+    currencyFormats: {},
+    verboseMap: {},
+  },
+  onSelect: jest.fn(),
+};
+
+beforeEach(() => {
+  jest.clearAllMocks();
+  mockLayerCapture.layers = [];
+  // The scatter sublayer query returns features tagged with a category column.
+  (SupersetClient.get as jest.Mock).mockResolvedValue({
+    json: {
+      data: {
+        features: [
+          { position: [0, 0], radius: 1, cat_color: 'A' },
+          { position: [1, 1], radius: 1, cat_color: 'B' },
+        ],
+      },
+    },
+  });
+});
+
+test('applies categorical scatterplot colors to sublayers in the multi chart', async () => {
+  renderWithProviders(<DeckMulti {...props} />);
+
+  await waitFor(() => {
+    expect(mockLayerCapture.layers.length).toBeGreaterThan(0);
+  });
+
+  const scatterLayer = mockLayerCapture.layers.find((layer: any) =>
+    layer?.id?.startsWith('scatter-layer-'),
+  );
+  expect(scatterLayer).toBeDefined();
+
+  const { data } = scatterLayer.props;
+  expect(data).toHaveLength(2);
+
+  // Both points must carry a resolved RGBA color...
+  data.forEach((d: any) => {
+    expect(Array.isArray(d.color)).toBe(true);
+    expect(d.color).toHaveLength(4);
+  });
+
+  // ...and the two distinct categories must NOT share the same color. Before
+  // the fix, categorical colors were dropped in the Multiple Layers chart and
+  // every point fell back to the same default color.
+  expect(data[0].color).not.toEqual(data[1].color);
+});
diff --git a/superset-frontend/plugins/preset-chart-deckgl/src/Multi/Multi.tsx b/superset-frontend/plugins/preset-chart-deckgl/src/Multi/Multi.tsx
index 690e4e57db6..10a94123ba4 100644
--- a/superset-frontend/plugins/preset-chart-deckgl/src/Multi/Multi.tsx
+++ b/superset-frontend/plugins/preset-chart-deckgl/src/Multi/Multi.tsx
@@ -49,6 +49,7 @@ import {
   DeckGLContainerStyledWrapper,
 } from '../DeckGLContainer';
 import { getExploreLongUrl } from '../utils/explore';
+import { addColorToFeatures } from '../utils/addColor';
 import layerGenerators from '../layers';
 import fitViewport, { Viewport } from '../utils/fitViewport';
 import { getMapboxApiKey } from '../utils/mapbox';
@@ -98,6 +99,18 @@ const MultiWrapper = styled.div<{ height: number; width: number }>`
   width: ${({ width }) => width}px;
 `;
 
+// Layer types that resolve a per-feature color from the form data's color
+// scheme (categorical palette, fixed color or breakpoints). When rendered on
+// their own these layers get their colors from CategoricalDeckGLContainer; in
+// the Multiple Layers chart we need to apply the same resolution here so the
+// configured categorical colors are preserved instead of falling back to the
+// default point color.
+const COLOR_AWARE_LAYER_TYPES = new Set([
+  'deck_scatter',
+  'deck_path',
+  'deck_arc',
+]);
+
 const selectDataMask = createSelector(
   (state: { dataMask?: DataMaskState }) => state.dataMask,
   dataMask => dataMask || {},
@@ -225,15 +238,39 @@ const DeckMulti = (props: DeckMultiProps) => {
   );
 
   const createLayerFromData = useCallback(
-    (subslice: JsonObject, json: JsonObject): Layer =>
-      // @ts-expect-error TODO(hainenber): define proper type for `form_data.viz_type` and call signature for functions in layerGenerators.
-      layerGenerators[subslice.form_data.viz_type]({
-        formData: subslice.form_data,
-        payload: json,
-        setTooltip,
-        datasource: props.datasource,
-        onSelect: props.onSelect,
-      }),
+    (subslice: JsonObject, json: JsonObject): Layer => {
+      const { form_data: subsliceFormData } = subslice;
+      let payload = json;
+
+      // Resolve per-feature categorical/fixed/breakpoint colors for layers that
+      // would otherwise get them from CategoricalDeckGLContainer when rendered
+      // standalone. Without this, scatterplot categorical colors are dropped in
+      // the Multiple Layers chart and fall back to the default point color.
+      if (
+        COLOR_AWARE_LAYER_TYPES.has(subsliceFormData.viz_type) &&
+        subsliceFormData.color_scheme_type &&
+        Array.isArray(json?.data?.features)
+      ) {
+        payload = {
+          ...json,
+          data: {
+            ...json.data,
+            features: addColorToFeatures(json.data.features, subsliceFormData),
+          },
+        };
+      }
+
+      return (
+        // @ts-expect-error TODO(hainenber): define proper type for `form_data.viz_type` and call signature for functions in layerGenerators.
+        layerGenerators[subsliceFormData.viz_type]({
+          formData: subsliceFormData,
+          payload,
+          setTooltip,
+          datasource: props.datasource,
+          onSelect: props.onSelect,
+        })
+      );
+    },
     [props.onSelect, props.datasource, setTooltip],
   );
 
diff --git a/superset-frontend/plugins/preset-chart-deckgl/src/utils/addColor.test.ts b/superset-frontend/plugins/preset-chart-deckgl/src/utils/addColor.test.ts
new file mode 100644
index 00000000000..20bf57f5779
--- /dev/null
+++ b/superset-frontend/plugins/preset-chart-deckgl/src/utils/addColor.test.ts
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { QueryFormData } from '@superset-ui/core';
+import { addColorToFeatures } from './addColor';
+import { COLOR_SCHEME_TYPES } from '../utilities/utils';
+
+const baseFormData = {
+  datasource: '1__table',
+  viz_type: 'deck_scatter',
+} as unknown as QueryFormData;
+
+test('assigns distinct colors per category for a categorical palette', () => {
+  const features = [{ cat_color: 'A' }, { cat_color: 'B' }, { cat_color: 'A' }];
+  const result = addColorToFeatures(features, {
+    ...baseFormData,
+    color_scheme_type: COLOR_SCHEME_TYPES.categorical_palette,
+    color_scheme: 'supersetColors',
+    dimension: 'category',
+    slice_id: 1,
+  } as unknown as QueryFormData);
+
+  // Each feature gets a resolved RGBA color
+  result.forEach(d => {
+    expect(Array.isArray(d.color)).toBe(true);
+    expect(d.color).toHaveLength(4);
+  });
+  // Same category resolves to the same color, different categories differ
+  expect(result[0].color).toEqual(result[2].color);
+  expect(result[0].color).not.toEqual(result[1].color);
+});
+
+test('falls back to the fixed color picker when no dimension is set', () => {
+  const features = [{ cat_color: 'A' }, { cat_color: 'B' }];
+  const result = addColorToFeatures(features, {
+    ...baseFormData,
+    color_scheme_type: COLOR_SCHEME_TYPES.categorical_palette,
+    color_picker: { r: 10, g: 20, b: 30, a: 1 },
+  } as unknown as QueryFormData);
+
+  result.forEach(d => {
+    expect(d.color).toEqual([10, 20, 30, 255]);
+  });
+});
+
+test('applies the fixed color scheme to every feature', () => {
+  const features = [{ cat_color: 'A' }, { cat_color: 'B' }];
+  const result = addColorToFeatures(features, {
+    ...baseFormData,
+    color_scheme_type: COLOR_SCHEME_TYPES.fixed_color,
+    color_picker: { r: 1, g: 2, b: 3, a: 0.5 },
+  } as unknown as QueryFormData);
+
+  result.forEach(d => {
+    expect(d.color).toEqual([1, 2, 3, 127.5]);
+  });
+});
+
+test('returns features unchanged for an unrecognized color scheme', () => {
+  const features = [{ cat_color: 'A' }];
+  const result = addColorToFeatures(features, {
+    ...baseFormData,
+    color_scheme_type: 'something_else',
+  } as unknown as QueryFormData);
+
+  expect(result).toEqual(features);
+  expect(result[0].color).toBeUndefined();
+});
diff --git a/superset-frontend/plugins/preset-chart-deckgl/src/utils/addColor.ts b/superset-frontend/plugins/preset-chart-deckgl/src/utils/addColor.ts
new file mode 100644
index 00000000000..639e7ec049a
--- /dev/null
+++ b/superset-frontend/plugins/preset-chart-deckgl/src/utils/addColor.ts
@@ -0,0 +1,113 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import {
+  CategoricalColorNamespace,
+  JsonObject,
+  QueryFormData,
+} from '@superset-ui/core';
+import { hexToRGB } from './colors';
+import { ColorBreakpointType } from '../types';
+import { COLOR_SCHEME_TYPES, ColorSchemeType } from '../utilities/utils';
+import { DEFAULT_DECKGL_COLOR } from '../utilities/Shared_DeckGL';
+
+const { getScale } = CategoricalColorNamespace;
+
+/**
+ * Resolve the per-feature color for a deck.gl layer based on the form data's
+ * color scheme configuration. This mirrors the categorical/fixed/breakpoint
+ * color logic that `CategoricalDeckGLContainer` applies when a layer is
+ * rendered on its own, so that it can be reused when layers are composed
+ * inside the deck.gl Multiple Layers chart.
+ *
+ * Features whose color scheme is not recognized are returned unchanged so the
+ * layer's own fallback color logic can take over.
+ */
+export function addColorToFeatures(
+  data: JsonObject[],
+  fd: QueryFormData,
+  selectedColorScheme: ColorSchemeType = fd.color_scheme_type,
+): JsonObject[] {
+  const appliedScheme = fd.color_scheme;
+  const colorFn = getScale(appliedScheme);
+
+  switch (selectedColorScheme) {
+    case COLOR_SCHEME_TYPES.fixed_color: {
+      const color = fd.color_picker || { r: 0, g: 0, b: 0, a: 1 };
+      const colorArray = [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: 1 };
+        const colorArray = [
+          fallbackColor.r,
+          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)),
+      }));
+    }
+    case COLOR_SCHEME_TYPES.color_breakpoints: {
+      const defaultBreakpointColor = fd.default_breakpoint_color
+        ? [
+            fd.default_breakpoint_color.r,
+            fd.default_breakpoint_color.g,
+            fd.default_breakpoint_color.b,
+            fd.default_breakpoint_color.a * 255,
+          ]
+        : [
+            DEFAULT_DECKGL_COLOR.r,
+            DEFAULT_DECKGL_COLOR.g,
+            DEFAULT_DECKGL_COLOR.b,
+            DEFAULT_DECKGL_COLOR.a * 255,
+          ];
+      return data.map(d => {
+        const breakpointForPoint: ColorBreakpointType =
+          fd.color_breakpoints?.find(
+            (breakpoint: ColorBreakpointType) =>
+              d.metric >= breakpoint.minValue &&
+              d.metric <= breakpoint.maxValue,
+          );
+
+        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:
+      return data;
+  }
+}
+
+export default addColorToFeatures;