fix(bugs): fixing bugs for world map chart (#38030)

This commit is contained in:
Alexandru Soare
2026-02-27 11:33:35 +02:00
committed by GitHub
parent 11dfda11d3
commit 7743183401
6 changed files with 157 additions and 20 deletions

View File

@@ -244,18 +244,20 @@ function WorldMap(element: HTMLElement, props: WorldMapProps): void {
}, },
]; ];
} }
onContextMenu(pointerEvent.clientX, pointerEvent.clientY, { if (onContextMenu) {
drillToDetail: drillToDetailFilters, onContextMenu(pointerEvent.clientX, pointerEvent.clientY, {
crossFilter: getCrossFilterDataMask(source), drillToDetail: drillToDetailFilters,
drillBy: { filters: drillByFilters, groupbyFieldName: 'entity' }, crossFilter: getCrossFilterDataMask(source),
}); drillBy: { filters: drillByFilters, groupbyFieldName: 'entity' },
});
}
}; };
const map = new Datamap({ const map = new Datamap({
element, element,
width, width,
height, height,
data: processedData, data: mapData,
fills: { fills: {
defaultFill: theme.colorBorder, defaultFill: theme.colorBorder,
}, },
@@ -268,6 +270,7 @@ function WorldMap(element: HTMLElement, props: WorldMapProps): void {
highlightFillColor: color, highlightFillColor: color,
highlightBorderWidth: 1, highlightBorderWidth: 1,
popupTemplate: (geo, d) => popupTemplate: (geo, d) =>
d &&
`<div class="hoverinfo"><strong>${d.name}</strong><br>${formatter( `<div class="hoverinfo"><strong>${d.name}</strong><br>${formatter(
d.m1, d.m1,
)}</div>`, )}</div>`,
@@ -298,7 +301,8 @@ function WorldMap(element: HTMLElement, props: WorldMapProps): void {
.selectAll('.datamaps-subunit') .selectAll('.datamaps-subunit')
.on('contextmenu', handleContextMenu) .on('contextmenu', handleContextMenu)
.on('click', handleClick) .on('click', handleClick)
.on('mouseover', function onMouseOver() { // Use namespaced events to avoid overriding Datamaps' default tooltip handlers
.on('mouseover.fillPreserve', function onMouseOver() {
if (inContextMenu) { if (inContextMenu) {
return; return;
} }
@@ -311,7 +315,7 @@ function WorldMap(element: HTMLElement, props: WorldMapProps): void {
// Store original fill color for restoration // Store original fill color for restoration
element.attr('data-original-fill', originalFill); element.attr('data-original-fill', originalFill);
}) })
.on('mouseout', function onMouseOut() { .on('mouseout.fillPreserve', function onMouseOut() {
if (inContextMenu) { if (inContextMenu) {
return; return;
} }

View File

@@ -77,8 +77,13 @@ const mockSvg = {
style: jest.fn().mockReturnThis(), style: jest.fn().mockReturnThis(),
}; };
// Store the last Datamap config for assertions
let lastDatamapConfig: Record<string, unknown> | null = null;
jest.mock('datamaps/dist/datamaps.all.min', () => jest.mock('datamaps/dist/datamaps.all.min', () =>
jest.fn().mockImplementation(config => { jest.fn().mockImplementation(config => {
// Store config for test assertions
lastDatamapConfig = config;
// Call the done callback immediately to simulate Datamap initialization // Call the done callback immediately to simulate Datamap initialization
if (config.done) { if (config.done) {
config.done({ config.done({
@@ -158,9 +163,11 @@ test('sets up mouseover and mouseout handlers on countries', () => {
expect(mockSvg.selectAll).toHaveBeenCalledWith('.datamaps-subunit'); expect(mockSvg.selectAll).toHaveBeenCalledWith('.datamaps-subunit');
const onCalls = mockSvg.on.mock.calls; const onCalls = mockSvg.on.mock.calls;
// Find mouseover and mouseout handler registrations // Find mouseover and mouseout handler registrations (namespaced events)
const hasMouseover = onCalls.some(call => call[0] === 'mouseover'); const hasMouseover = onCalls.some(
const hasMouseout = onCalls.some(call => call[0] === 'mouseout'); call => call[0] === 'mouseover.fillPreserve',
);
const hasMouseout = onCalls.some(call => call[0] === 'mouseout.fillPreserve');
expect(hasMouseover).toBe(true); expect(hasMouseover).toBe(true);
expect(hasMouseout).toBe(true); expect(hasMouseout).toBe(true);
@@ -199,9 +206,9 @@ test('stores original fill color on mouseover', () => {
jest.spyOn(d3 as any, 'select').mockReturnValue(mockD3Selection as any); jest.spyOn(d3 as any, 'select').mockReturnValue(mockD3Selection as any);
// Capture the mouseover handler // Capture the mouseover handler (namespaced event)
mockSvg.on.mockImplementation((event: string, handler: MouseEventHandler) => { mockSvg.on.mockImplementation((event: string, handler: MouseEventHandler) => {
if (event === 'mouseover') { if (event === 'mouseover.fillPreserve') {
mouseoverHandler = handler; mouseoverHandler = handler;
} }
return mockSvg; return mockSvg;
@@ -254,9 +261,9 @@ test('restores original fill color on mouseout for country with data', () => {
jest.spyOn(d3 as any, 'select').mockReturnValue(mockD3Selection as any); jest.spyOn(d3 as any, 'select').mockReturnValue(mockD3Selection as any);
// Capture the mouseout handler // Capture the mouseout handler (namespaced event)
mockSvg.on.mockImplementation((event: string, handler: MouseEventHandler) => { mockSvg.on.mockImplementation((event: string, handler: MouseEventHandler) => {
if (event === 'mouseout') { if (event === 'mouseout.fillPreserve') {
mouseoutHandler = handler; mouseoutHandler = handler;
} }
return mockSvg; return mockSvg;
@@ -310,8 +317,9 @@ test('restores default fill color on mouseout for country with no data', () => {
jest.spyOn(d3 as any, 'select').mockReturnValue(mockD3Selection as any); jest.spyOn(d3 as any, 'select').mockReturnValue(mockD3Selection as any);
// Capture the mouseout handler (namespaced event)
mockSvg.on.mockImplementation((event: string, handler: MouseEventHandler) => { mockSvg.on.mockImplementation((event: string, handler: MouseEventHandler) => {
if (event === 'mouseout') { if (event === 'mouseout.fillPreserve') {
mouseoutHandler = handler; mouseoutHandler = handler;
} }
return mockSvg; return mockSvg;
@@ -352,11 +360,12 @@ test('does not handle mouse events when inContextMenu is true', () => {
jest.spyOn(d3 as any, 'select').mockReturnValue(mockD3Selection as any); jest.spyOn(d3 as any, 'select').mockReturnValue(mockD3Selection as any);
// Capture namespaced event handlers
mockSvg.on.mockImplementation((event: string, handler: MouseEventHandler) => { mockSvg.on.mockImplementation((event: string, handler: MouseEventHandler) => {
if (event === 'mouseover') { if (event === 'mouseover.fillPreserve') {
mouseoverHandler = handler; mouseoverHandler = handler;
} }
if (event === 'mouseout') { if (event === 'mouseout.fillPreserve') {
mouseoutHandler = handler; mouseoutHandler = handler;
} }
return mockSvg; return mockSvg;
@@ -387,3 +396,115 @@ test('does not handle mouse events when inContextMenu is true', () => {
expect(fillChangeCalls.length).toBe(0); expect(fillChangeCalls.length).toBe(0);
expect(fillStyleChangeCalls.length).toBe(0); expect(fillStyleChangeCalls.length).toBe(0);
}); });
test('does not throw error when onContextMenu is undefined', () => {
const propsWithoutContextMenu = {
...baseProps,
onContextMenu: undefined,
};
// Should not throw
expect(() => {
WorldMap(container, propsWithoutContextMenu as any);
}).not.toThrow();
});
test('calls onContextMenu when provided and right-click occurs', () => {
const mockOnContextMenu = jest.fn();
const propsWithContextMenu = {
...baseProps,
onContextMenu: mockOnContextMenu,
};
let contextMenuHandler: ((source: any) => void) | undefined;
mockSvg.on.mockImplementation((event: string, handler: any) => {
if (event === 'contextmenu') {
contextMenuHandler = handler;
}
return mockSvg;
});
// Mock d3.event
(d3 as any).event = {
preventDefault: jest.fn(),
clientX: 100,
clientY: 200,
};
WorldMap(container, propsWithContextMenu);
expect(contextMenuHandler).toBeDefined();
contextMenuHandler!({ country: 'USA' });
expect(mockOnContextMenu).toHaveBeenCalledWith(100, 200, expect.any(Object));
});
test('initializes Datamap with keyed object data for tooltip support', () => {
WorldMap(container, baseProps);
// Verify data is an object (not an array) keyed by country codes
expect(Array.isArray(lastDatamapConfig?.data)).toBe(false);
expect(typeof lastDatamapConfig?.data).toBe('object');
const data = lastDatamapConfig?.data as Record<string, unknown>;
// Verify the data is keyed by country code
expect(data).toHaveProperty('USA');
expect(data).toHaveProperty('CAN');
// Verify the keyed data contains the expected properties for tooltips
expect(data.USA).toMatchObject({
country: 'USA',
name: 'United States',
m1: 100,
m2: 200,
});
expect(data.CAN).toMatchObject({
country: 'CAN',
name: 'Canada',
m1: 50,
m2: 100,
});
});
test('popupTemplate returns tooltip HTML when country data exists', () => {
WorldMap(container, baseProps);
const geographyConfig = lastDatamapConfig?.geographyConfig as Record<
string,
unknown
>;
const popupTemplate = geographyConfig?.popupTemplate as (
geo: unknown,
d: unknown,
) => string;
const mockGeo = { properties: { name: 'United States' } };
const mockCountryData = { name: 'United States', m1: 100 };
const tooltipHtml = popupTemplate(mockGeo, mockCountryData);
expect(tooltipHtml).toContain('United States');
expect(tooltipHtml).toContain('hoverinfo');
});
test('popupTemplate handles null/undefined country data gracefully', () => {
WorldMap(container, baseProps);
const geographyConfig = lastDatamapConfig?.geographyConfig as Record<
string,
unknown
>;
const popupTemplate = geographyConfig?.popupTemplate as (
geo: unknown,
d: unknown,
) => string | undefined;
const mockGeo = { properties: { name: 'Antarctica' } };
// When hovering over a country with no data, 'd' will be undefined
const tooltipHtml = popupTemplate(mockGeo, undefined);
expect(tooltipHtml).toBeFalsy();
});

View File

@@ -205,7 +205,11 @@ const SliceHeader = forwardRef<HTMLDivElement, SliceHeaderProps>(
const sqlRowCount = const sqlRowCount =
countFromSecondQuery != null countFromSecondQuery != null
? countFromSecondQuery ? countFromSecondQuery
: Number(firstQueryResponse?.sql_rowcount ?? 0); : Number(
firstQueryResponse?.sql_rowcount ??
firstQueryResponse?.rowcount ??
0,
);
const canExplore = !editMode && supersetCanExplore; const canExplore = !editMode && supersetCanExplore;
const showRowLimitWarning = const showRowLimitWarning =

View File

@@ -499,6 +499,8 @@ const Chart = (props: ChartProps) => {
} else if ((queriesResponse?.[0] as JsonObject)?.sql_rowcount != null) { } else if ((queriesResponse?.[0] as JsonObject)?.sql_rowcount != null) {
actualRowCount = (queriesResponse![0] as JsonObject) actualRowCount = (queriesResponse![0] as JsonObject)
.sql_rowcount as number; .sql_rowcount as number;
} else if ((queriesResponse?.[0] as JsonObject)?.rowcount != null) {
actualRowCount = (queriesResponse![0] as JsonObject).rowcount as number;
} else { } else {
actualRowCount = (exportFormData as JsonObject)?.row_limit as actualRowCount = (exportFormData as JsonObject)?.row_limit as
| number | number

View File

@@ -77,7 +77,11 @@ export const ChartPills = forwardRef(
const actualRowCount = const actualRowCount =
isTableChart && countFromSecondQuery != null isTableChart && countFromSecondQuery != null
? countFromSecondQuery ? countFromSecondQuery
: Number(firstQueryResponse?.sql_rowcount ?? 0); : Number(
firstQueryResponse?.sql_rowcount ??
firstQueryResponse?.rowcount ??
0,
);
return ( return (
<div ref={ref}> <div ref={ref}>

View File

@@ -311,6 +311,8 @@ export const useExploreAdditionalActionsMenu = (
actualRowCount = queriesResponse[1].data[0].rowcount; actualRowCount = queriesResponse[1].data[0].rowcount;
} else if (queriesResponse && queriesResponse[0]?.sql_rowcount != null) { } else if (queriesResponse && queriesResponse[0]?.sql_rowcount != null) {
actualRowCount = queriesResponse[0].sql_rowcount; actualRowCount = queriesResponse[0].sql_rowcount;
} else if (queriesResponse && queriesResponse[0]?.rowcount != null) {
actualRowCount = queriesResponse[0].rowcount;
} else { } else {
actualRowCount = latestQueryFormData?.row_limit; actualRowCount = latestQueryFormData?.row_limit;
} }