fix(deckgl): use 'none' sentinel so legend hides and Hex/Path stay visible

Addresses review of the prior commit. Two problems remained: dropping the
Legend's 'tr' default hid legends for deck_hex/deck_path (which have no
Legend Position control, so position is always undefined); and the "None"
choice value was null, which SelectControl does not reliably round-trip
(control-state hydration applies `value ?? default`, restoring 'tr'), so
the falsy gate never saw it in the real app.

Give "None" a stable 'none' string sentinel, restore the 'tr' default,
and hide when position is 'none' (also null/'' so charts saved under the
old null choice keep hiding). Unset (undefined) keeps the 'tr' default so
Hex/Path legends stay visible.

Adds a SelectControl test proving 'none' round-trips through onChange, and
reworks the Polygon test to assert legend_position is forwarded to the
Legend instead of re-implementing the hide gate in the mock.

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

@@ -131,10 +131,9 @@ test('ctrl+clicking a legend item toggles the category without opening a new tab
 });
 
 // Regression proof for the "Legend Position: None" control not hiding the
-// legend. The control's "None" choice has a null value, but the modernized
-// Select can hand it back as undefined or an empty string, so the legend must
-// hide for any falsy position rather than strictly matching null.
-test.each([null, undefined, ''])(
+// 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(
@@ -160,3 +159,17 @@ test('renders the legend for a valid corner position', () => {
 
   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

@@ -89,7 +89,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;
@@ -98,7 +98,7 @@ export type LegendProps = {
 const Legend = ({
   format: d3Format = null,
   forceCategorical = false,
-  position,
+  position = 'tr',
   categories: categoriesObject = {},
   toggleCategory = () => {},
   showSingleCategory = () => {},
@@ -137,11 +137,15 @@ const Legend = ({
   };
 
   // Hide the legend when there are no categories, or when Legend Position is
-  // "None". The control's "None" choice has a null value, but the Select can
-  // round-trip it as undefined (or an empty string), so treat every falsy
-  // position as "hidden" rather than strictly matching null. Valid corner
-  // values ('tl'/'tr'/'bl'/'br') are truthy and fall through to render.
-  if (Object.keys(categoriesObject).length === 0 || !position) {
+  // "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

@@ -18,11 +18,12 @@
  */
 import type { ReactElement } from 'react';
 // eslint-disable-next-line import/no-extraneous-dependencies
-import { render } from '@testing-library/react';
+import { render, screen } from '@testing-library/react';
 // eslint-disable-next-line import/no-extraneous-dependencies
 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,23 +54,25 @@ jest.mock('../../utils/mapbox', () => ({
   hasMapboxApiKey: () => true,
 }));
 
-// Stand in for the real Legend while preserving its hide contract: a falsy
-// position (Legend Position "None") renders nothing. Without this the mock
-// would render unconditionally and the "None hides the legend" assertions
-// below would pass vacuously.
+// 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 }: any) =>
-      position ? (
-        <div
-          data-testid="legend"
-          data-categories={JSON.stringify(categories)}
-          data-position={position}
-        >
-          Legend Mock
-        </div>
-      ) : null,
+    ({
+      categories,
+      position,
+    }: Pick<LegendProps, 'categories' | 'position'>) => (
+      <div
+        data-test="legend"
+        data-categories={JSON.stringify(categories)}
+        data-position={String(position)}
+      >
+        Legend Mock
+      </div>
+    ),
 );
 
 const mockProps = {
@@ -378,28 +381,24 @@ describe('DeckGLPolygon Error Handling and Edge Cases', () => {
     expect(mockGetBuckets).not.toHaveBeenCalled();
   });
 
-  // The "None" choice can reach the layer as null or (after the Select
-  // round-trip) undefined; either must hide the legend. queryByTestId is
-  // configured to match data-test, not the mock's data-testid, so assert
-  // against the container directly.
-  test.each([null, undefined])(
-    'hides the legend when legend_position is None (%p)',
+  // 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 propsWithNoLegend = {
+      const props = {
         ...mockProps,
-        formData: {
-          ...mockProps.formData,
-          legend_position: legendPosition,
-        },
+        formData: { ...mockProps.formData, legend_position: legendPosition },
       };
 
-      const { container } = renderWithTheme(
-        <DeckGLPolygon {...propsWithNoLegend} />,
-      );
+      renderWithTheme(<DeckGLPolygon {...props} />);
 
-      expect(
-        container.querySelector('[data-testid="legend"]'),
-      ).not.toBeInTheDocument();
+      expect(screen.getByTestId('legend')).toHaveAttribute(
+        'data-position',
+        legendPosition,
+      );
     },
   );
 });
@@ -417,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);
   });
@@ -440,14 +439,10 @@ describe('DeckGLPolygon Legend Integration', () => {
       },
     };
 
-    const { container } = renderWithTheme(
-      <DeckGLPolygon {...propsWithoutMetric} />,
-    );
+    renderWithTheme(<DeckGLPolygon {...propsWithoutMetric} />);
 
     // Legend should not be rendered when no metric is defined
-    expect(
-      container.querySelector('[data-testid="legend"]'),
-    ).not.toBeInTheDocument();
+    expect(screen.queryByTestId('legend')).not.toBeInTheDocument();
   });
 });
 

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());
+});