Skip to content

Fix empty layers spinning forever in Multi Map legend #352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ethanbienstock wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix empty layers spinning forever in Multi Map legend #352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ee3926f
fix: handle empty layers in Multi Map legend instead of spinning forever
Mar 23, 2026
8037c94
style: format MultiLegend.tsx
Mar 23, 2026
a93a7cb
feat: add "Visible but Empty" label for empty layers in Multi Map legend
Mar 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions superset-frontend/plugins/geoset-map-chart/src/GeoSetMultiMap/Multi.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,10 +342,6 @@ const DeckMulti = (props: DeckMultiProps) => {
handleFeatureClick(info, sliceFeatureInfoColumnNames),
);

if (!newLayer) {
return null;
}

const payloadData = payload?.data || [];
const geometryType = getGeometryType(payloadData[0]?.geojson);
let transformPropsGeojsonLayer =
Expand Down Expand Up @@ -470,13 +466,15 @@ const DeckMulti = (props: DeckMultiProps) => {
maxZoom: zoomSlider[1],
};

const newLayerStates = layerStatesGenerator(
newLayer,
newLayerStateOptions,
);
// When the layer has no renderable data, return an entry with
// empty layerStates so the legend can show it as "loaded but
// empty" instead of spinning forever.
const newLayerStates = newLayer
? layerStatesGenerator(newLayer, newLayerStateOptions)
: [];

if (!newLayerStates.length) {
return null;
legendEntry.empty = true;
}

const layerFeatures: JsonObject[] =
Expand Down
24 changes: 22 additions & 2 deletions superset-frontend/plugins/geoset-map-chart/src/components/MultiLegend.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,13 @@ const CategoryRow = styled.div`
gap: 8px;
`;

const NoDataLabel = styled.div`
font-size: 11px;
color: gray;
margin-bottom: 6px;
margin-left: -20px;
`;

const MetricBlock = styled.div`
margin: 6px 0;
`;
Expand All @@ -176,20 +183,22 @@ const Bounds = styled.div(
`,
);

const VisibilityCheckbox = styled.input`
const VisibilityCheckbox = styled.input<{ $empty?: boolean }>`
width: 14px;
height: 14px;
cursor: pointer;
margin: 0 !important;
flex-shrink: 0;
${({ $empty }) => $empty && 'accent-color: gray;'}
`;

// Checkbox that supports indeterminate state (shows minus sign when some but not all are selected)
const IndeterminateCheckbox: React.FC<{
checked: boolean;
indeterminate: boolean;
onChange: (e: React.ChangeEvent<HTMLInputElement>) => void;
}> = ({ checked, indeterminate, onChange }) => {
$empty?: boolean;
}> = ({ checked, indeterminate, onChange, $empty }) => {
const ref = useRef<HTMLInputElement>(null);

useEffect(() => {
Expand All @@ -204,6 +213,7 @@ const IndeterminateCheckbox: React.FC<{
type="checkbox"
checked={checked}
onChange={onChange}
$empty={$empty}
/>
);
};
Expand Down Expand Up @@ -236,6 +246,9 @@ const LegendEntryContent: React.FC<{

return (
<div>
{legendEntry.empty && (
<NoDataLabel>Visible but Empty</NoDataLabel>
)}
{/* SIMPLE - show icon and slice name (skip when sizeEntry handles the display) */}
{legendEntry.type === 'simple' &&
legendEntry.simpleStyle &&
Expand All @@ -246,6 +259,7 @@ const LegendEntryContent: React.FC<{
type="checkbox"
checked={isVisible}
onChange={onToggleVisibility}
$empty={legendEntry.empty}
/>
)}
<Swatch
Expand Down Expand Up @@ -299,6 +313,7 @@ const LegendEntryContent: React.FC<{
type="checkbox"
checked={item.enabled}
onChange={() => onToggleCategory(sliceId, item.label)}
$empty={legendEntry.empty}
/>
)}
<span>{item.label}</span>
Expand All @@ -324,6 +339,7 @@ const LegendEntryContent: React.FC<{
type="checkbox"
checked={isEnabled}
onChange={() => onToggleCategory(sliceId, cat.label)}
$empty={legendEntry.empty}
/>
)}
<Swatch
Expand Down Expand Up @@ -364,6 +380,7 @@ const LegendEntryContent: React.FC<{
type="checkbox"
checked={isVisible}
onChange={onToggleVisibility}
$empty={legendEntry.empty}
/>
<Swatch
fill={fill}
Expand Down Expand Up @@ -493,6 +510,8 @@ export const MultiLegend: React.FC<MultiLegendProps> = ({
someVisibleSomeNot || (isVisible && hasPartialCategories);

const allLoading = entries.every(e => e.legendEntry.loading);
const allEmpty =
!allLoading && entries.every(e => e.legendEntry.empty);

return (
<Group key={displayTitle}>
Expand All @@ -505,6 +524,7 @@ export const MultiLegend: React.FC<MultiLegendProps> = ({
<IndeterminateCheckbox
checked={isVisible}
indeterminate={isIndeterminate}
$empty={allEmpty}
onChange={e => {
e.stopPropagation();
setOptimisticVisibility(prev => ({
Expand Down
1 change: 1 addition & 0 deletions superset-frontend/plugins/geoset-map-chart/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ export type LegendEntry = {
isCombinedMetricSize?: boolean;
initialCollapsed?: boolean; // Whether this legend entry starts collapsed
loading?: boolean; // True for stub entries whose layer data is still loading
empty?: boolean; // True when layer loaded successfully but returned no data
};

export type LegendGroup = {
Expand Down
211 changes: 211 additions & 0 deletions superset-frontend/plugins/geoset-map-chart/test/components/MultiLegend.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -564,4 +564,215 @@ describe('MultiLegend', () => {
expect(screen.getByText('500+')).toBeInTheDocument();
});
});

describe('empty layer state', () => {
it('shows checkbox and "no data" label instead of spinner for empty entry', () => {
renderWithTheme(
<MultiLegend
legendGroups={[
createLegendGroup({
displayTitle: 'Empty Group',
entries: [
{
sliceId: '1',
legendEntry: createSimpleLegendEntry({
legendName: 'Empty Layer',
empty: true,
}),
},
],
}),
createLegendGroup({ displayTitle: 'Other' }),
]}
layerVisibility={EMPTY_VISIBILITY}
onToggleLayerVisibility={jest.fn()}
/>,
);
userEvent.click(screen.getByText('Legend'));
expect(screen.getByText('Empty Layer')).toBeInTheDocument();
expect(
screen.getByText('Visible but Empty'),
).toBeInTheDocument();
expect(screen.queryByRole('status')).not.toBeInTheDocument();
});

it('does not show "no data" label for non-empty entry', () => {
renderWithTheme(
<MultiLegend
legendGroups={[
createLegendGroup({
entries: [
{
sliceId: '1',
legendEntry: createSimpleLegendEntry({
legendName: 'Normal Layer',
}),
},
],
}),
]}
layerVisibility={EMPTY_VISIBILITY}
/>,
);
userEvent.click(screen.getByText('Legend'));
expect(screen.getByText('Normal Layer')).toBeInTheDocument();
expect(
screen.queryByText('Visible but Empty'),
).not.toBeInTheDocument();
});

it('group header shows checkbox (not spinner) when all entries are empty', () => {
renderWithTheme(
<MultiLegend
legendGroups={[
createLegendGroup({
displayTitle: 'All Empty',
entries: [
{
sliceId: '1',
legendEntry: createSimpleLegendEntry({ empty: true }),
},
{
sliceId: '2',
legendEntry: createSimpleLegendEntry({ empty: true }),
},
],
}),
createLegendGroup({ displayTitle: 'Other' }),
]}
layerVisibility={EMPTY_VISIBILITY}
onToggleLayerVisibility={jest.fn()}
/>,
);
userEvent.click(screen.getByText('Legend'));
// Group header should render a checkbox, not a loading spinner
const checkboxes = screen.getAllByRole('checkbox');
expect(checkboxes.length).toBeGreaterThanOrEqual(2);
expect(screen.queryByRole('status')).not.toBeInTheDocument();
});

it('group header checkbox is present when some entries have data', () => {
renderWithTheme(
<MultiLegend
legendGroups={[
createLegendGroup({
displayTitle: 'Mixed',
entries: [
{
sliceId: '1',
legendEntry: createSimpleLegendEntry({ empty: true }),
},
{
sliceId: '2',
legendEntry: createSimpleLegendEntry({ empty: false }),
},
],
}),
createLegendGroup({ displayTitle: 'Other' }),
]}
layerVisibility={EMPTY_VISIBILITY}
onToggleLayerVisibility={jest.fn()}
/>,
);
userEvent.click(screen.getByText('Legend'));
const checkboxes = screen.getAllByRole('checkbox');
expect(checkboxes.length).toBeGreaterThanOrEqual(2);
});

it('shows spinner when loading, even if empty is also true', () => {
renderWithTheme(
<MultiLegend
legendGroups={[
createLegendGroup({
displayTitle: 'Loading',
entries: [
{
sliceId: '1',
legendEntry: createSimpleLegendEntry({
loading: true,
empty: true,
}),
},
],
}),
createLegendGroup({
displayTitle: 'Other',
entries: [
{
sliceId: '2',
legendEntry: createSimpleLegendEntry({
loading: true,
}),
},
],
}),
]}
layerVisibility={EMPTY_VISIBILITY}
onToggleLayerVisibility={jest.fn()}
/>,
);
userEvent.click(screen.getByText('Legend'));
// Both group headers should show spinners, not checkboxes
expect(screen.queryByRole('checkbox')).not.toBeInTheDocument();
expect(
screen.queryByText('Visible but Empty'),
).not.toBeInTheDocument();
});

it('transitions from loading to empty without spinner', () => {
const { rerender } = renderWithTheme(
<MultiLegend
legendGroups={[
createLegendGroup({
displayTitle: 'Transitioning',
entries: [
{
sliceId: '1',
legendEntry: createSimpleLegendEntry({
legendName: 'Trans Layer',
loading: true,
}),
},
],
}),
createLegendGroup({ displayTitle: 'Other' }),
]}
layerVisibility={EMPTY_VISIBILITY}
onToggleLayerVisibility={jest.fn()}
/>,
);
userEvent.click(screen.getByText('Legend'));
expect(
screen.queryByText('Visible but Empty'),
).not.toBeInTheDocument();

// Re-render with loading finished and empty result
rerender(
<MultiLegend
legendGroups={[
createLegendGroup({
displayTitle: 'Transitioning',
entries: [
{
sliceId: '1',
legendEntry: createSimpleLegendEntry({
legendName: 'Trans Layer',
loading: false,
empty: true,
}),
},
],
}),
createLegendGroup({ displayTitle: 'Other' }),
]}
layerVisibility={EMPTY_VISIBILITY}
onToggleLayerVisibility={jest.fn()}
/>,
);
expect(screen.getByText('Trans Layer')).toBeInTheDocument();
expect(
screen.getByText('Visible but Empty'),
).toBeInTheDocument();
});
});
});
Loading