diff --git a/superset-frontend/plugins/preset-chart-deckgl/src/components/Legend.test.tsx b/superset-frontend/plugins/preset-chart-deckgl/src/components/Legend.test.tsx
index b0cd43dcb93..73819b638d2 100644
--- a/superset-frontend/plugins/preset-chart-deckgl/src/components/Legend.test.tsx
+++ b/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(
+ ,
+ );
+
+ expect(screen.getByText('Alpha')).toBeInTheDocument();
+});
diff --git a/superset-frontend/plugins/preset-chart-deckgl/src/components/Legend.tsx b/superset-frontend/plugins/preset-chart-deckgl/src/components/Legend.tsx
index 5c160bd5226..5663cc5830f 100644
--- a/superset-frontend/plugins/preset-chart-deckgl/src/components/Legend.tsx
+++ b/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;
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;
}
diff --git a/superset-frontend/plugins/preset-chart-deckgl/src/layers/Polygon/Polygon.test.tsx b/superset-frontend/plugins/preset-chart-deckgl/src/layers/Polygon/Polygon.test.tsx
index 7ef01a3c88f..1776f2bd98d 100644
--- a/superset-frontend/plugins/preset-chart-deckgl/src/layers/Polygon/Polygon.test.tsx
+++ b/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 ? (
-
- Legend Mock
-
- ) : null,
+ ({
+ categories,
+ position,
+ }: Pick) => (
+
+ Legend Mock
+
+ ),
);
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(
- ,
- );
+ renderWithTheme();
- 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({component});
test('renders legend with non-empty categories when metric and linear_palette are defined', () => {
- const { container } = renderWithTheme();
+ renderWithTheme();
// 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(
- ,
- );
+ renderWithTheme();
// Legend should not be rendered when no metric is defined
- expect(
- container.querySelector('[data-testid="legend"]'),
- ).not.toBeInTheDocument();
+ expect(screen.queryByTestId('legend')).not.toBeInTheDocument();
});
});
diff --git a/superset-frontend/plugins/preset-chart-deckgl/src/utilities/Shared_DeckGL.tsx b/superset-frontend/plugins/preset-chart-deckgl/src/utilities/Shared_DeckGL.tsx
index feff6b17f57..32a3ad19ce4 100644
--- a/superset-frontend/plugins/preset-chart-deckgl/src/utilities/Shared_DeckGL.tsx
+++ b/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')],
diff --git a/superset-frontend/src/explore/components/controls/SelectControl.test.tsx b/superset-frontend/src/explore/components/controls/SelectControl.test.tsx
index c599309c117..ee53ab5a831 100644
--- a/superset-frontend/src/explore/components/controls/SelectControl.test.tsx
+++ b/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(
+ ,
+ );
+
+ 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());
+});