fix(deckgl): hide legend when Legend Position is None (#42402)

superset-frontend/plugins/preset-chart-deckgl/src/components/Legend.test.tsx

@@ -22,7 +22,7 @@ import { createEvent, fireEvent, render, screen } from '@testing-library/react';
 import '@testing-library/jest-dom';
 import { supersetTheme, ThemeProvider } from '@apache-superset/core/theme';
 import type { ReactElement } from 'react';
-import Legend from './Legend';
+import Legend, { type LegendProps } from './Legend';
 
 const renderWithTheme = (component: ReactElement) =>
   render(<ThemeProvider theme={supersetTheme}>{component}</ThemeProvider>);
@@ -31,6 +31,7 @@ test('formats interval-notation labels while preserving brackets', () => {
   renderWithTheme(
     <Legend
       format=",.2f"
+      position="tr"
       categories={{
         '[1, 81)': { enabled: true, color: [0, 0, 0] },
         '[81, 212)': { enabled: true, color: [0, 0, 0] },
@@ -48,6 +49,7 @@ test('still formats legacy "a - b" delimiter labels', () => {
   renderWithTheme(
     <Legend
       format=",.1f"
+      position="tr"
       categories={{
         '0 - 100000': { enabled: true, color: [0, 0, 0] },
         '100001 - 200000': { enabled: true, color: [0, 0, 0] },
@@ -63,6 +65,7 @@ test('leaves labels untouched when no format is provided', () => {
   renderWithTheme(
     <Legend
       format={null}
+      position="tr"
       categories={{ '[1, 81)': { enabled: true, color: [0, 0, 0] } }}
     />,
   );
@@ -78,6 +81,7 @@ test('clicking a legend item toggles the category without triggering anchor navi
   renderWithTheme(
     <Legend
       format={null}
+      position="tr"
       categories={{
         Positive: { enabled: true, color: [0, 255, 0] },
         Negative: { enabled: true, color: [255, 0, 0] },
@@ -103,6 +107,7 @@ test('ctrl+clicking a legend item toggles the category without opening a new tab
   renderWithTheme(
     <Legend
       format={null}
+      position="tr"
       categories={{
         cat1: { enabled: true, color: [255, 0, 0] },
         cat2: { enabled: false, color: [0, 0, 255] },
@@ -122,3 +127,47 @@ test('ctrl+clicking a legend item toggles the category without opening a new tab
   expect(toggleCategory).toHaveBeenCalledTimes(1);
   expect(toggleCategory).toHaveBeenCalledWith('cat1');
 });
+
+// Regression proof for the "Legend Position: None" control not hiding the
+// legend. "None" is the 'none' sentinel; null and '' also hide so charts saved
+// under the older null-valued choice keep working.
+test.each(['none', null, ''])(
+  'renders nothing when Legend Position is None (position=%p)',
+  position => {
+    renderWithTheme(
+      <Legend
+        format={null}
+        position={position as LegendProps['position']}
+        categories={{ Alpha: { enabled: true, color: [0, 0, 0] } }}
+      />,
+    );
+
+    expect(screen.queryByText('Alpha')).not.toBeInTheDocument();
+  },
+);
+
+test('renders the legend for a valid corner position', () => {
+  renderWithTheme(
+    <Legend
+      format={null}
+      position="tr"
+      categories={{ Alpha: { enabled: true, color: [0, 0, 0] } }}
+    />,
+  );
+
+  expect(screen.getByText('Alpha')).toBeInTheDocument();
+});
+
+test('falls back to the top-right default when position is unset', () => {
+  // Layers without a Legend Position control (e.g. Hex, Path) pass an
+  // undefined position; the legend must still render at the default corner.
+  renderWithTheme(
+    <Legend
+      format={null}
+      position={undefined}
+      categories={{ Alpha: { enabled: true, color: [0, 0, 0] } }}
+    />,
+  );
+
+  expect(screen.getByText('Alpha')).toBeInTheDocument();
+});

superset-frontend/plugins/preset-chart-deckgl/src/components/Legend.tsx

@@ -95,7 +95,7 @@ const parseInterval = (label: string) => {
 export type LegendProps = {
   format: string | null;
   forceCategorical?: boolean;
-  position?: null | 'tl' | 'tr' | 'bl' | 'br';
+  position?: null | 'none' | 'tl' | 'tr' | 'bl' | 'br';
   categories: Record<string, { enabled: boolean; color: Color | undefined }>;
   toggleCategory?: (key: string) => void;
   showSingleCategory?: (key: string) => void;
@@ -142,7 +142,16 @@ const Legend = ({
     return format(k);
   };
 
-  if (Object.keys(categoriesObject).length === 0 || position === null) {
+  // Hide the legend when there are no categories, or when Legend Position is
+  // "None". "None" is the 'none' sentinel from the control; null/'' are also
+  // treated as hidden so charts saved under the older null-valued choice keep
+  // working. An unset position (undefined) keeps the 'tr' default so layers
+  // without a Legend Position control (e.g. Hex, Path) still show their legend.
+  if (
+    Object.keys(categoriesObject).length === 0 ||
+    !position ||
+    position === 'none'
+  ) {
     return null;
   }
 

superset-frontend/plugins/preset-chart-deckgl/src/layers/Polygon/Polygon.test.tsx

@@ -23,6 +23,7 @@ import { render, screen } from '@testing-library/react';
 import '@testing-library/jest-dom';
 import { supersetTheme, ThemeProvider } from '@apache-superset/core/theme';
 import DeckGLPolygon, { getPoints } from './Polygon';
+import type { LegendProps } from '../../components/Legend';
 import { COLOR_SCHEME_TYPES } from '../../utilities/utils';
 import * as utils from '../../utils';
 
@@ -53,15 +54,26 @@ jest.mock('../../utils/mapbox', () => ({
   hasMapboxApiKey: () => true,
 }));
 
-jest.mock('../../components/Legend', () => ({ categories, position }: any) => (
-  <div
-    data-testid="legend"
-    data-categories={JSON.stringify(categories)}
-    data-position={position}
-  >
-    Legend Mock
-  </div>
-));
+// Stand in for the real Legend, exposing the props it received so the layer's
+// wiring can be asserted. The hide-on-"none" behavior belongs to the real
+// Legend (covered in Legend.test.tsx), so the mock does not reimplement it.
+// Emits data-test (the configured testIdAttribute) so screen queries resolve.
+jest.mock(
+  '../../components/Legend',
+  () =>
+    ({
+      categories,
+      position,
+    }: Pick<LegendProps, 'categories' | 'position'>) => (
+      <div
+        data-test="legend"
+        data-categories={JSON.stringify(categories)}
+        data-position={String(position)}
+      >
+        Legend Mock
+      </div>
+    ),
+);
 
 const mockProps = {
   formData: {
@@ -369,20 +381,26 @@ describe('DeckGLPolygon Error Handling and Edge Cases', () => {
     expect(mockGetBuckets).not.toHaveBeenCalled();
   });
 
-  test('handles null legend_position correctly', () => {
-    const propsWithNullLegendPosition = {
-      ...mockProps,
-      formData: {
-        ...mockProps.formData,
-        legend_position: null,
-      },
-    };
+  // The layer forwards formData.legend_position to the Legend's position prop
+  // unchanged; hiding for the "none" sentinel is the Legend's responsibility
+  // (covered in Legend.test.tsx). Asserting the wiring here avoids
+  // re-implementing the hide gate in the mock.
+  test.each(['none', 'tr'])(
+    'forwards legend_position "%s" to the Legend',
+    legendPosition => {
+      const props = {
+        ...mockProps,
+        formData: { ...mockProps.formData, legend_position: legendPosition },
+      };
 
-    renderWithTheme(<DeckGLPolygon {...propsWithNullLegendPosition} />);
+      renderWithTheme(<DeckGLPolygon {...props} />);
 
-    // Legend should not be rendered when position is null
-    expect(screen.queryByTestId('legend')).not.toBeInTheDocument();
-  });
+      expect(screen.getByTestId('legend')).toHaveAttribute(
+        'data-position',
+        legendPosition,
+      );
+    },
+  );
 });
 
 describe('DeckGLPolygon Legend Integration', () => {
@@ -398,16 +416,16 @@ describe('DeckGLPolygon Legend Integration', () => {
     render(<ThemeProvider theme={supersetTheme}>{component}</ThemeProvider>);
 
   test('renders legend with non-empty categories when metric and linear_palette are defined', () => {
-    const { container } = renderWithTheme(<DeckGLPolygon {...mockProps} />);
+    renderWithTheme(<DeckGLPolygon {...mockProps} />);
 
     // Verify the component renders and calls the correct bucket function
     expect(mockGetBuckets).toHaveBeenCalled();
     expect(mockGetColorBreakpointsBuckets).not.toHaveBeenCalled();
 
     // Verify the legend mock was rendered with non-empty categories
-    const legendElement = container.querySelector('[data-testid="legend"]');
-    expect(legendElement).toBeTruthy();
-    const categoriesAttr = legendElement?.getAttribute('data-categories');
+    const legendElement = screen.getByTestId('legend');
+    expect(legendElement).toBeInTheDocument();
+    const categoriesAttr = legendElement.getAttribute('data-categories');
     const categoriesData = JSON.parse(categoriesAttr || '{}');
     expect(Object.keys(categoriesData)).toHaveLength(2);
   });

superset-frontend/plugins/preset-chart-deckgl/src/utilities/Shared_DeckGL.tsx

@@ -221,7 +221,10 @@ export const legendPosition = {
     clearable: false,
     default: 'tr',
     choices: [
-      [null, t('None')],
+      // A stable string sentinel rather than null: the Select control does not
+      // reliably round-trip a null-valued option back as null (it can surface
+      // as undefined/the default), which left "None" unable to hide the legend.
+      ['none', t('None')],
       ['tl', t('Top left')],
       ['tr', t('Top right')],
       ['bl', t('Bottom left')],

superset-frontend/src/explore/components/controls/SelectControl.test.tsx

@@ -472,3 +472,39 @@ describe('SelectControl', () => {
     });
   });
 });
+
+// Control-path regression proof for the deck.gl "Legend Position: None" bug:
+// a string sentinel ('none') survives selection through the real Select and
+// reaches onChange unchanged, so it can hide the legend. A null-valued option
+// did not round-trip reliably, which is why the choice value is a sentinel.
+test('selecting a string "none" option round-trips through onChange', async () => {
+  const onChange = jest.fn();
+  render(
+    <SelectControl
+      name="legend_position"
+      label="Legend Position"
+      clearable={false}
+      default="tr"
+      value="tr"
+      choices={[
+        ['none', 'None'],
+        ['tl', 'Top left'],
+        ['tr', 'Top right'],
+        ['bl', 'Bottom left'],
+        ['br', 'Bottom right'],
+      ]}
+      onChange={onChange}
+    />,
+  );
+
+  const selectorInput = screen.getByLabelText('Legend Position', {
+    selector: 'input',
+  });
+  userEvent.click(selectorInput);
+  act(() => jest.runAllTimers());
+
+  userEvent.click(screen.getByRole('option', { name: 'None' }));
+  act(() => jest.runAllTimers());
+
+  expect(onChange).toHaveBeenCalledWith('none', expect.anything());
+});