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 fe6b07bc782..73c52b28937 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 @@ -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({component}); @@ -31,6 +31,7 @@ test('formats interval-notation labels while preserving brackets', () => { renderWithTheme( { renderWithTheme( { renderWithTheme( , ); @@ -78,6 +81,7 @@ test('clicking a legend item toggles the category without triggering anchor navi renderWithTheme( { + renderWithTheme( + , + ); + + expect(screen.queryByText('Alpha')).not.toBeInTheDocument(); + }, +); + +test('renders the legend for a valid corner position', () => { + renderWithTheme( + , + ); + + 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 c531cad982b..02001481d7d 100644 --- a/superset-frontend/plugins/preset-chart-deckgl/src/components/Legend.tsx +++ b/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; 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; } 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 6c392c0486b..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 @@ -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) => ( -
- Legend Mock -
-)); +// 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) => ( +
+ Legend Mock +
+ ), +); 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(); + renderWithTheme(); - // 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({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); }); 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 e6fd1d0e9d9..c9d4a73f8a7 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()); +});