From cbd27802a86e9197d1571ddb3338957e9d133dce Mon Sep 17 00:00:00 2001 From: Risheit Munshi Date: Mon, 8 Dec 2025 18:27:40 -0500 Subject: [PATCH] fix(chart): Display better hover text for country map charts (#36323) Co-authored-by: askinner1 <144823527+askinner1@users.noreply.github.com> Co-authored-by: codeant-ai-for-open-source[bot] <244253245+codeant-ai-for-open-source[bot]@users.noreply.github.com> (cherry picked from commit 67cf287c033cd88c59695bb4337478a36dcb1a27) --- .../src/CountryMap.css | 61 -------- .../src/CountryMap.js | 68 ++++----- .../src/ReactCountryMap.jsx | 34 ++--- .../test/CountryMap.test.tsx | 132 ++++++++++++++++++ 4 files changed, 171 insertions(+), 124 deletions(-) delete mode 100644 superset-frontend/plugins/legacy-plugin-chart-country-map/src/CountryMap.css create mode 100644 superset-frontend/plugins/legacy-plugin-chart-country-map/test/CountryMap.test.tsx diff --git a/superset-frontend/plugins/legacy-plugin-chart-country-map/src/CountryMap.css b/superset-frontend/plugins/legacy-plugin-chart-country-map/src/CountryMap.css deleted file mode 100644 index f8234e0ec41..00000000000 --- a/superset-frontend/plugins/legacy-plugin-chart-country-map/src/CountryMap.css +++ /dev/null @@ -1,61 +0,0 @@ -/** - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -.superset-legacy-chart-country-map svg { - background-color: #feffff; -} - -.superset-legacy-chart-country-map { - position: relative; -} - -.superset-legacy-chart-country-map .background { - fill: rgba(255, 255, 255, 0); - pointer-events: all; -} - -.superset-legacy-chart-country-map .map-layer { - fill: #fff; - stroke: #aaa; -} - -.superset-legacy-chart-country-map .effect-layer { - pointer-events: none; -} - -.superset-legacy-chart-country-map .text-layer { - color: #333333; - text-anchor: middle; - pointer-events: none; -} - -.superset-legacy-chart-country-map text.result-text { - font-weight: 300; - font-size: 24px; -} - -.superset-legacy-chart-country-map text.big-text { - font-weight: 700; - font-size: 16px; -} - -.superset-legacy-chart-country-map path.region { - cursor: pointer; - stroke: #eee; -} diff --git a/superset-frontend/plugins/legacy-plugin-chart-country-map/src/CountryMap.js b/superset-frontend/plugins/legacy-plugin-chart-country-map/src/CountryMap.js index b1cf5016dcf..e8e14d6a44c 100644 --- a/superset-frontend/plugins/legacy-plugin-chart-country-map/src/CountryMap.js +++ b/superset-frontend/plugins/legacy-plugin-chart-country-map/src/CountryMap.js @@ -90,15 +90,7 @@ function CountryMap(element, props) { .attr('height', height); const g = svg.append('g'); const mapLayer = g.append('g').classed('map-layer', true); - const textLayer = g - .append('g') - .classed('text-layer', true) - .attr('transform', `translate(${width / 2}, 45)`); - const bigText = textLayer.append('text').classed('big-text', true); - const resultText = textLayer - .append('text') - .classed('result-text', true) - .attr('dy', '1em'); + const hoverPopup = div.append('div').attr('class', 'hover-popup'); let centered; @@ -128,45 +120,18 @@ function CountryMap(element, props) { 'transform', `translate(${halfWidth},${halfHeight})scale(${k})translate(${-x},${-y})`, ); - textLayer - .style('opacity', 0) - .attr( - 'transform', - `translate(0,0)translate(${x},${hasCenter ? y - 5 : 45})`, - ) - .transition() - .duration(750) - .style('opacity', 1); - bigText - .transition() - .duration(750) - .style('font-size', hasCenter ? 6 : 16); - resultText - .transition() - .duration(750) - .style('font-size', hasCenter ? 16 : 24); }; backgroundRect.on('click', clicked); - const selectAndDisplayNameOfRegion = function selectAndDisplayNameOfRegion( - feature, - ) { - let name = ''; + const getNameOfRegion = function getNameOfRegion(feature) { if (feature && feature.properties) { if (feature.properties.ID_2) { - name = feature.properties.NAME_2; - } else { - name = feature.properties.NAME_1; + return feature.properties.NAME_2; } + return feature.properties.NAME_1; } - bigText.text(name); - }; - - const updateMetrics = function updateMetrics(region) { - if (region.length > 0) { - resultText.text(format(region[0].metric)); - } + return ''; }; const mouseenter = function mouseenter(d) { @@ -176,17 +141,31 @@ function CountryMap(element, props) { c = d3.rgb(c).darker().toString(); } d3.select(this).style('fill', c); - selectAndDisplayNameOfRegion(d); + // Display information popup const result = data.filter( region => region.country_id === d.properties.ISO, ); - updateMetrics(result); + + const position = d3.mouse(svg.node()); + hoverPopup + .style('display', 'block') + .style('top', `${position[1] + 30}px`) + .style('left', `${position[0]}px`) + .html( + `
${getNameOfRegion(d)}
${result.length > 0 ? format(result[0].metric) : ''}
`, + ); + }; + + const mousemove = function mousemove() { + const position = d3.mouse(svg.node()); + hoverPopup + .style('top', `${position[1] + 30}px`) + .style('left', `${position[0]}px`); }; const mouseout = function mouseout() { d3.select(this).style('fill', colorFn); - bigText.text(''); - resultText.text(''); + hoverPopup.style('display', 'none'); }; function drawMap(mapData) { @@ -225,6 +204,7 @@ function CountryMap(element, props) { .attr('vector-effect', 'non-scaling-stroke') .style('fill', colorFn) .on('mouseenter', mouseenter) + .on('mousemove', mousemove) .on('mouseout', mouseout) .on('click', clicked); } diff --git a/superset-frontend/plugins/legacy-plugin-chart-country-map/src/ReactCountryMap.jsx b/superset-frontend/plugins/legacy-plugin-chart-country-map/src/ReactCountryMap.jsx index 9d8413b4275..d72eeb5210e 100644 --- a/superset-frontend/plugins/legacy-plugin-chart-country-map/src/ReactCountryMap.jsx +++ b/superset-frontend/plugins/legacy-plugin-chart-country-map/src/ReactCountryMap.jsx @@ -21,7 +21,7 @@ import Component from './CountryMap'; const ReactComponent = reactify(Component); -const CountryMap = ({ className, ...otherProps }) => ( +const CountryMap = ({ className = '', ...otherProps }) => (
@@ -42,33 +42,29 @@ export default styled(CountryMap)` pointer-events: all; } + .superset-legacy-chart-country-map .hover-popup { + position: absolute; + color: ${theme.colorTextSecondary}; + display: none; + padding: 4px; + border-radius: 1px; + background-color: ${theme.colorBgElevated}; + box-shadow: ${theme.boxShadow}; + font-size: 12px; + border: 1px solid ${theme.colorBorder}; + z-index: 10001; + } + .superset-legacy-chart-country-map .map-layer { fill: ${theme.colorBgContainer}; stroke: ${theme.colorBorderSecondary}; + pointer-events: all; } .superset-legacy-chart-country-map .effect-layer { pointer-events: none; } - .superset-legacy-chart-country-map .text-layer { - color: ${theme.colorText}; - text-anchor: middle; - pointer-events: none; - } - - .superset-legacy-chart-country-map text.result-text { - fill: ${theme.colorText}; - font-weight: ${theme.fontWeightLight}; - font-size: ${theme.fontSizeXL}px; - } - - .superset-legacy-chart-country-map text.big-text { - fill: ${theme.colorText}; - font-weight: ${theme.fontWeightStrong}; - font-size: ${theme.fontSizeLG}px; - } - .superset-legacy-chart-country-map path.region { cursor: pointer; stroke: ${theme.colorSplit}; diff --git a/superset-frontend/plugins/legacy-plugin-chart-country-map/test/CountryMap.test.tsx b/superset-frontend/plugins/legacy-plugin-chart-country-map/test/CountryMap.test.tsx new file mode 100644 index 00000000000..2f47c1d7d3c --- /dev/null +++ b/superset-frontend/plugins/legacy-plugin-chart-country-map/test/CountryMap.test.tsx @@ -0,0 +1,132 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// eslint-disable-next-line import/no-extraneous-dependencies +import '@testing-library/jest-dom'; +// eslint-disable-next-line import/no-extraneous-dependencies +import { render, fireEvent } from '@testing-library/react'; +import d3 from 'd3'; +import ReactCountryMap from '../src/ReactCountryMap'; + +jest.spyOn(d3, 'json'); + +type Projection = ((...args: unknown[]) => void) & { + scale: () => Projection; + center: () => Projection; + translate: () => Projection; +}; + +type PathFn = (() => string) & { + projection: jest.Mock; + bounds: jest.Mock<[[number, number], [number, number]]>; + centroid: jest.Mock<[number, number]>; +}; + +const mockPath: PathFn = jest.fn(() => 'M10 10 L20 20') as unknown as PathFn; +mockPath.projection = jest.fn(); +mockPath.bounds = jest.fn(() => [ + [0, 0], + [100, 100], +]); +mockPath.centroid = jest.fn(() => [50, 50]); + +jest.spyOn(d3.geo, 'path').mockImplementation(() => mockPath); + +// Mock d3.geo.mercator +jest.spyOn(d3.geo, 'mercator').mockImplementation(() => { + const proj = (() => {}) as Projection; + proj.scale = () => proj; + proj.center = () => proj; + proj.translate = () => proj; + return proj; +}); + +// Mock d3.mouse +jest.spyOn(d3, 'mouse').mockReturnValue([100, 50]); + +const mockMapData = { + type: 'FeatureCollection', + features: [ + { + type: 'Feature', + properties: { ISO: 'CAN', NAME_1: 'Canada' }, + geometry: {}, + }, + ], +}; + +type D3JsonCallback = (error: Error | null, data: unknown) => void; + +describe('CountryMap (legacy d3)', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('renders a map after d3.json loads data', async () => { + d3.json.mockImplementation((_url: string, cb: D3JsonCallback) => + cb(null, mockMapData), + ); + + render( + , + ); + + expect(d3.json).toHaveBeenCalledTimes(1); + + const region = document.querySelector('path.region'); + expect(region).not.toBeNull(); + }); + + it('shows tooltip on mouseenter/mousemove/mouseout', async () => { + d3.json.mockImplementation((_url: string, cb: D3JsonCallback) => + cb(null, mockMapData), + ); + + render( + , + ); + + const region = document.querySelector('path.region'); + expect(region).not.toBeNull(); + + const popup = document.querySelector('.hover-popup'); + expect(popup).not.toBeNull(); + + fireEvent.mouseEnter(region!); + expect(popup!).toHaveStyle({ display: 'block' }); + + fireEvent.mouseOut(region!); + expect(popup!).toHaveStyle({ display: 'none' }); + }); +});