From 8f7d1d8281cec41872256f5dd18f9f1e51738d07 Mon Sep 17 00:00:00 2001 From: Levko Kravets Date: Wed, 29 Jan 2020 11:14:08 +0200 Subject: [PATCH 01/10] Choropleth: allow to use custom maps --- .../choropleth/Editor/FormatSettings.jsx | 72 ++++----- .../choropleth/Editor/GeneralSettings.jsx | 143 +++++++++--------- .../visualizations/choropleth/Editor/utils.js | 45 ++---- .../choropleth/Renderer/index.jsx | 34 +---- .../choropleth/Renderer/initChoropleth.js | 30 +++- .../choropleth/Renderer/utils.js | 22 +-- .../visualizations/choropleth/getOptions.js | 27 +++- .../choropleth/hooks/useLoadGeoJson.js | 39 +++++ .../visualizations/choropleth/maps/index.js | 13 ++ .../visualizations/choropleth_spec.js | 8 +- 10 files changed, 232 insertions(+), 201 deletions(-) create mode 100644 client/app/visualizations/choropleth/hooks/useLoadGeoJson.js create mode 100644 client/app/visualizations/choropleth/maps/index.js diff --git a/client/app/visualizations/choropleth/Editor/FormatSettings.jsx b/client/app/visualizations/choropleth/Editor/FormatSettings.jsx index 53f6d79b54..8647e634d7 100644 --- a/client/app/visualizations/choropleth/Editor/FormatSettings.jsx +++ b/client/app/visualizations/choropleth/Editor/FormatSettings.jsx @@ -1,4 +1,6 @@ -import React from "react"; +import { map } from "lodash"; +import React, { useMemo } from "react"; +import PropTypes from "prop-types"; import { useDebouncedCallback } from "use-debounce"; import * as Grid from "antd/lib/grid"; import { @@ -12,49 +14,29 @@ import { } from "@/components/visualizations/editor"; import { EditorPropTypes } from "@/visualizations/prop-types"; -function TemplateFormatHint({ mapType }) { - // eslint-disable-line react/prop-types +import useLoadGeoJson from "../hooks/useLoadGeoJson"; +import { getGeoJsonFields } from "./utils"; + +function TemplateFormatHint({ geoJsonProperties }) { return (
- All query result columns can be referenced using {"{{ column_name }}"} syntax. -
-
Use special names to access additional properties:
-
- {"{{ @@value }}"} formatted value; +
+ All query result columns can be referenced using {"{{ column_name }}"} syntax. +
+
+ Use {"{{ @@value }}"} to access formatted value. +
- {mapType === "countries" && ( + {geoJsonProperties.length > 0 && ( -
- {"{{ @@name }}"} short country name; -
-
- {"{{ @@name_long }}"} full country name; -
-
- {"{{ @@abbrev }}"} abbreviated country name; -
-
- {"{{ @@iso_a2 }}"} two-letter ISO country code; -
-
- {"{{ @@iso_a3 }}"} three-letter ISO country code; -
-
- {"{{ @@iso_n3 }}"} three-digit ISO country code. -
- - )} - {mapType === "subdiv_japan" && ( - -
- {"{{ @@name }}"} Prefecture name in English; -
-
- {"{{ @@name_local }}"} Prefecture name in Kanji; -
-
- {"{{ @@iso_3166_2 }}"} five-letter ISO subdivision code (JP-xx); +
GeoJSON properties could be accessed by these names:
+
+ {map(geoJsonProperties, property => ( +
+ {`{{ @@${property}}}`} +
+ ))}
)} @@ -62,10 +44,20 @@ function TemplateFormatHint({ mapType }) { ); } +TemplateFormatHint.propTypes = { + geoJsonProperties: PropTypes.arrayOf(PropTypes.string), +}; + +TemplateFormatHint.defaultProps = { + geoJsonProperties: [], +}; + export default function GeneralSettings({ options, onOptionsChange }) { const [onOptionsChangeDebounced] = useDebouncedCallback(onOptionsChange, 200); + const [geoJson] = useLoadGeoJson(options.mapUrl); + const geoJsonFields = useMemo(() => getGeoJsonFields(geoJson), [geoJson]); - const templateFormatHint = ; + const templateFormatHint = ; return (
diff --git a/client/app/visualizations/choropleth/Editor/GeneralSettings.jsx b/client/app/visualizations/choropleth/Editor/GeneralSettings.jsx index bc73b5ef89..25c4ab7f1a 100644 --- a/client/app/visualizations/choropleth/Editor/GeneralSettings.jsx +++ b/client/app/visualizations/choropleth/Editor/GeneralSettings.jsx @@ -1,88 +1,94 @@ -import { map } from "lodash"; -import React, { useMemo } from "react"; +import { isString, map, filter, find } from "lodash"; +import React, { useMemo, useState, useCallback } from "react"; +import * as Grid from "antd/lib/grid"; import { EditorPropTypes } from "@/visualizations/prop-types"; import { Section, Select } from "@/components/visualizations/editor"; -import { inferCountryCodeType } from "./utils"; + +import useLoadGeoJson from "../hooks/useLoadGeoJson"; +import availableMaps from "../maps"; +import { getGeoJsonFields } from "./utils"; + +function getMapLabel(mapUrl) { + const result = find(availableMaps, item => item.url === mapUrl); + return result ? result.name : mapUrl; +} export default function GeneralSettings({ options, data, onOptionsChange }) { - const countryCodeTypes = useMemo(() => { - switch (options.mapType) { - case "countries": - return { - name: "Short name", - name_long: "Full name", - abbrev: "Abbreviated name", - iso_a2: "ISO code (2 letters)", - iso_a3: "ISO code (3 letters)", - iso_n3: "ISO code (3 digits)", - }; - case "subdiv_japan": - return { - name: "Name", - name_local: "Name (local)", - iso_3166_2: "ISO-3166-2", - }; - default: - return {}; - } - }, [options.mapType]); + const [geoJson, isLoadingGeoJson] = useLoadGeoJson(options.mapUrl); + const geoJsonFields = useMemo(() => getGeoJsonFields(geoJson), [geoJson]); + const [customGeoJsonUrl, setCustomGeoJsonUrl] = useState(null); - const handleChangeAndInferType = newOptions => { - newOptions.countryCodeType = - inferCountryCodeType( - newOptions.mapType || options.mapType, - data ? data.rows : [], - newOptions.countryCodeColumn || options.countryCodeColumn - ) || options.countryCodeType; - onOptionsChange(newOptions); - }; + // While geoJson is loading - show last selected field in select + const targetFields = isLoadingGeoJson ? filter([options.targetField], isString) : geoJsonFields; + + const handleMapUrlChange = useCallback( + mapUrl => { + if (!mapUrl) { + setCustomGeoJsonUrl(null); + } + onOptionsChange({ mapUrl: mapUrl || null }); + }, + [onOptionsChange] + ); return (
-
- -
-
- + + + + + + + +
@@ -90,6 +96,7 @@ export default function GeneralSettings({ options, data, onOptionsChange }) { label="Value column" className="w-100" data-test="Choropleth.Editor.ValueColumn" + disabled={data.columns.length === 0} defaultValue={options.valueColumn} onChange={valueColumn => onOptionsChange({ valueColumn })}> {map(data.columns, ({ name }) => ( diff --git a/client/app/visualizations/choropleth/Editor/utils.js b/client/app/visualizations/choropleth/Editor/utils.js index a20184c860..6aacfa5c5e 100644 --- a/client/app/visualizations/choropleth/Editor/utils.js +++ b/client/app/visualizations/choropleth/Editor/utils.js @@ -1,38 +1,15 @@ /* eslint-disable import/prefer-default-export */ -import _ from "lodash"; - -export function inferCountryCodeType(mapType, data, countryCodeField) { - const regexMap = { - countries: { - iso_a2: /^[a-z]{2}$/i, - iso_a3: /^[a-z]{3}$/i, - iso_n3: /^[0-9]{3}$/i, - }, - subdiv_japan: { - name: /^[a-z]+$/i, - name_local: /^[\u3400-\u9FFF\uF900-\uFAFF]|[\uD840-\uD87F][\uDC00-\uDFFF]+$/i, - iso_3166_2: /^JP-[0-9]{2}$/i, +import { isObject, isArray, reduce, keys, uniq } from "lodash"; + +export function getGeoJsonFields(geoJson) { + const features = isObject(geoJson) && isArray(geoJson.features) ? geoJson.features : []; + return reduce( + features, + (result, feature) => { + const properties = isObject(feature) && isObject(feature.properties) ? feature.properties : {}; + return uniq([...result, ...keys(properties)]); }, - }; - - const regex = regexMap[mapType]; - - const initState = _.mapValues(regex, () => 0); - - const result = _.chain(data) - .reduce((memo, item) => { - const value = item[countryCodeField]; - if (_.isString(value)) { - _.each(regex, (r, k) => { - memo[k] += r.test(value) ? 1 : 0; - }); - } - return memo; - }, initState) - .toPairs() - .reduce((memo, item) => (item[1] > memo[1] ? item : memo)) - .value(); - - return result[1] / data.length >= 0.9 ? result[0] : null; + [] + ); } diff --git a/client/app/visualizations/choropleth/Renderer/index.jsx b/client/app/visualizations/choropleth/Renderer/index.jsx index 09d34869b4..1536430ba8 100644 --- a/client/app/visualizations/choropleth/Renderer/index.jsx +++ b/client/app/visualizations/choropleth/Renderer/index.jsx @@ -1,49 +1,21 @@ import { omit, merge } from "lodash"; import React, { useState, useEffect } from "react"; -import { axios } from "@/services/axios"; import { RendererPropTypes } from "@/visualizations/prop-types"; import useMemoWithDeepCompare from "@/lib/hooks/useMemoWithDeepCompare"; +import useLoadGeoJson from "../hooks/useLoadGeoJson"; import initChoropleth from "./initChoropleth"; import { prepareData } from "./utils"; import "./renderer.less"; -import countriesDataUrl from "../maps/countries.geo.json"; -import subdivJapanDataUrl from "../maps/japan.prefectures.geo.json"; - -function getDataUrl(type) { - switch (type) { - case "countries": - return countriesDataUrl; - case "subdiv_japan": - return subdivJapanDataUrl; - default: - return null; - } -} - export default function Renderer({ data, options, onOptionsChange }) { const [container, setContainer] = useState(null); - const [geoJson, setGeoJson] = useState(null); + const [geoJson] = useLoadGeoJson(options.mapUrl); const optionsWithoutBounds = useMemoWithDeepCompare(() => omit(options, ["bounds"]), [options]); const [map, setMap] = useState(null); - useEffect(() => { - let cancelled = false; - - axios.get(getDataUrl(options.mapType)).then(data => { - if (!cancelled) { - setGeoJson(data); - } - }); - - return () => { - cancelled = true; - }; - }, [options.mapType]); - useEffect(() => { if (container) { const _map = initChoropleth(container); @@ -58,7 +30,7 @@ export default function Renderer({ data, options, onOptionsChange }) { if (map) { map.updateLayers( geoJson, - prepareData(data.rows, optionsWithoutBounds.countryCodeColumn, optionsWithoutBounds.valueColumn), + prepareData(data.rows, optionsWithoutBounds.keyColumn, optionsWithoutBounds.valueColumn), options // detect changes for all options except bounds, but pass them all! ); } diff --git a/client/app/visualizations/choropleth/Renderer/initChoropleth.js b/client/app/visualizations/choropleth/Renderer/initChoropleth.js index 084906a960..b774513fa4 100644 --- a/client/app/visualizations/choropleth/Renderer/initChoropleth.js +++ b/client/app/visualizations/choropleth/Renderer/initChoropleth.js @@ -35,9 +35,9 @@ const CustomControl = L.Control.extend({ }); function prepareLayer({ feature, layer, data, options, limits, colors, formatValue }) { - const value = getValueForFeature(feature, data, options.countryCodeType); + const value = getValueForFeature(feature, data, options.targetField); const valueFormatted = formatValue(value); - const featureData = prepareFeatureProperties(feature, valueFormatted, data, options.countryCodeType); + const featureData = prepareFeatureProperties(feature, valueFormatted, data, options.targetField); const color = getColorByValue(value, limits, colors, options.colors.noValue); layer.setStyle({ @@ -69,6 +69,19 @@ function prepareLayer({ feature, layer, data, options, limits, colors, formatVal }); } +function validateBounds(bounds, fallbackBounds) { + if (bounds) { + bounds = L.latLngBounds(bounds[0], bounds[1]); + if (bounds.isValid()) { + return bounds; + } + } + if (fallbackBounds && fallbackBounds.isValid()) { + return fallbackBounds; + } + return null; +} + export default function initChoropleth(container) { const _map = L.map(container, { center: [0.0, 0.0], @@ -123,9 +136,12 @@ export default function initChoropleth(container) { }, }).addTo(_map); - const bounds = _choropleth.getBounds(); - _map.fitBounds(options.bounds || bounds, { animate: false, duration: 0 }); - _map.setMaxBounds(bounds); + const mapBounds = _choropleth.getBounds(); + const bounds = validateBounds(options.bounds, mapBounds); + if (bounds) { + _map.fitBounds(bounds, { animate: false, duration: 0 }); + _map.setMaxBounds(mapBounds); + } // send updated bounds to editor; delay this to avoid infinite update loop setTimeout(() => { @@ -149,8 +165,8 @@ export default function initChoropleth(container) { function updateBounds(bounds) { if (!boundsChangedFromMap) { const layerBounds = _choropleth ? _choropleth.getBounds() : _map.getBounds(); - bounds = bounds ? L.latLngBounds(bounds[0], bounds[1]) : layerBounds; - if (bounds.isValid()) { + bounds = validateBounds(bounds, layerBounds); + if (bounds) { _map.fitBounds(bounds, { animate: false, duration: 0 }); } } diff --git a/client/app/visualizations/choropleth/Renderer/utils.js b/client/app/visualizations/choropleth/Renderer/utils.js index ab44c88888..eaa4dbe14d 100644 --- a/client/app/visualizations/choropleth/Renderer/utils.js +++ b/client/app/visualizations/choropleth/Renderer/utils.js @@ -18,17 +18,17 @@ export function createNumberFormatter(format, placeholder) { }; } -export function prepareData(data, countryCodeField, valueField) { - if (!countryCodeField || !valueField) { +export function prepareData(data, keyColumn, valueColumn) { + if (!keyColumn || !valueColumn) { return {}; } const result = {}; each(data, item => { - if (item[countryCodeField]) { - const value = parseFloat(item[valueField]); - result[item[countryCodeField]] = { - code: item[countryCodeField], + if (item[keyColumn]) { + const value = parseFloat(item[valueColumn]); + result[item[keyColumn]] = { + code: item[keyColumn], value: isFinite(value) ? value : undefined, item, }; @@ -37,18 +37,18 @@ export function prepareData(data, countryCodeField, valueField) { return result; } -export function prepareFeatureProperties(feature, valueFormatted, data, countryCodeType) { +export function prepareFeatureProperties(feature, valueFormatted, data, targetField) { const result = {}; each(feature.properties, (value, key) => { result["@@" + key] = value; }); result["@@value"] = valueFormatted; - const datum = data[feature.properties[countryCodeType]] || {}; + const datum = data[feature.properties[targetField]] || {}; return extend(result, datum.item); } -export function getValueForFeature(feature, data, countryCodeType) { - const code = feature.properties[countryCodeType]; +export function getValueForFeature(feature, data, targetField) { + const code = feature.properties[targetField]; if (isString(code) && isObject(data[code])) { return data[code].value; } @@ -70,7 +70,7 @@ export function createScale(features, data, options) { // Calculate limits const values = uniq( filter( - map(features, feature => getValueForFeature(feature, data, options.countryCodeType)), + map(features, feature => getValueForFeature(feature, data, options.targetField)), isFinite ) ); diff --git a/client/app/visualizations/choropleth/getOptions.js b/client/app/visualizations/choropleth/getOptions.js index e28cc77366..5b7b7e7820 100644 --- a/client/app/visualizations/choropleth/getOptions.js +++ b/client/app/visualizations/choropleth/getOptions.js @@ -1,11 +1,12 @@ -import { merge } from "lodash"; +import { isNil, merge } from "lodash"; import ColorPalette from "./ColorPalette"; +import availableMaps from "./maps"; const DEFAULT_OPTIONS = { - mapType: "countries", - countryCodeColumn: "", - countryCodeType: "iso_a3", - valueColumn: "", + mapUrl: availableMaps.countries.url, + keyColumn: null, + targetField: null, + valueColumn: null, clusteringMode: "e", steps: 5, valueFormat: "0,0.00", @@ -33,5 +34,19 @@ const DEFAULT_OPTIONS = { }; export default function getOptions(options) { - return merge({}, DEFAULT_OPTIONS, options); + const result = merge({}, DEFAULT_OPTIONS, options); + + // backward compatibility + if (!isNil(result.mapType)) { + result.mapUrl = availableMaps[result.mapType] ? availableMaps[result.mapType].url : null; + delete result.mapType; + + result.keyColumn = result.countryCodeColumn; + delete result.countryCodeColumn; + + result.targetField = result.countryCodeType; + delete result.countryCodeType; + } + + return result; } diff --git a/client/app/visualizations/choropleth/hooks/useLoadGeoJson.js b/client/app/visualizations/choropleth/hooks/useLoadGeoJson.js new file mode 100644 index 0000000000..34bffd6e62 --- /dev/null +++ b/client/app/visualizations/choropleth/hooks/useLoadGeoJson.js @@ -0,0 +1,39 @@ +import { isString, isObject } from "lodash"; +import { useState, useEffect } from "react"; +import { axios } from "@/services/axios"; + +const defaultGeoJson = { + type: "FeatureCollection", + features: [], +}; + +// TODO: It needs some cache +export default function useLoadGeoJson(mapUrl) { + const [geoJson, setGeoJson] = useState(defaultGeoJson); + const [isLoading, setIsLoading] = useState(false); + + useEffect(() => { + if (isString(mapUrl)) { + setIsLoading(true); + let cancelled = false; + axios + .get(mapUrl) + .catch(() => defaultGeoJson) + .then(data => { + if (!cancelled) { + setGeoJson(isObject(data) ? data : defaultGeoJson); + setIsLoading(false); + } + }); + + return () => { + cancelled = true; + }; + } else { + setGeoJson(defaultGeoJson); + setIsLoading(false); + } + }, [mapUrl]); + + return [geoJson, isLoading]; +} diff --git a/client/app/visualizations/choropleth/maps/index.js b/client/app/visualizations/choropleth/maps/index.js new file mode 100644 index 0000000000..e09ca3520b --- /dev/null +++ b/client/app/visualizations/choropleth/maps/index.js @@ -0,0 +1,13 @@ +import countriesDataUrl from "./countries.geo.json"; +import subdivJapanDataUrl from "./japan.prefectures.geo.json"; + +export default { + countries: { + name: "Countries", + url: countriesDataUrl, + }, + subdiv_japan: { + name: "Japan/Prefectures", + url: subdivJapanDataUrl, + }, +}; diff --git a/client/cypress/integration/visualizations/choropleth_spec.js b/client/cypress/integration/visualizations/choropleth_spec.js index c5faa43290..bd5710230b 100644 --- a/client/cypress/integration/visualizations/choropleth_spec.js +++ b/client/cypress/integration/visualizations/choropleth_spec.js @@ -49,12 +49,12 @@ describe("Choropleth", () => { cy.clickThrough(` VisualizationEditor.Tabs.General - Choropleth.Editor.MapType - Choropleth.Editor.MapType.Countries + Choropleth.Editor.MapUrl + Choropleth.Editor.MapUrl.Countries Choropleth.Editor.KeyColumn Choropleth.Editor.KeyColumn.name - Choropleth.Editor.KeyType - Choropleth.Editor.KeyType.name + Choropleth.Editor.TargetField + Choropleth.Editor.TargetField.name Choropleth.Editor.ValueColumn Choropleth.Editor.ValueColumn.value `); From 6260601213b0adc4386361ca45d2c6a7f0321d98 Mon Sep 17 00:00:00 2001 From: Levko Kravets Date: Wed, 29 Jan 2020 12:21:45 +0200 Subject: [PATCH 02/10] Use separate input for custom map URL (pre-defined map URLs should not be saved in options, only keys) --- .../choropleth/Editor/FormatSettings.jsx | 3 +- .../choropleth/Editor/GeneralSettings.jsx | 64 +++++++++---------- .../choropleth/Renderer/index.jsx | 3 +- .../visualizations/choropleth/getOptions.js | 22 +++++-- .../choropleth/hooks/useLoadGeoJson.js | 8 +-- .../visualizations/choropleth/maps/index.js | 8 ++- .../visualizations/choropleth_spec.js | 4 +- 7 files changed, 62 insertions(+), 50 deletions(-) diff --git a/client/app/visualizations/choropleth/Editor/FormatSettings.jsx b/client/app/visualizations/choropleth/Editor/FormatSettings.jsx index 8647e634d7..5689c0a53b 100644 --- a/client/app/visualizations/choropleth/Editor/FormatSettings.jsx +++ b/client/app/visualizations/choropleth/Editor/FormatSettings.jsx @@ -15,6 +15,7 @@ import { import { EditorPropTypes } from "@/visualizations/prop-types"; import useLoadGeoJson from "../hooks/useLoadGeoJson"; +import { getMapUrl } from "../maps"; import { getGeoJsonFields } from "./utils"; function TemplateFormatHint({ geoJsonProperties }) { @@ -54,7 +55,7 @@ TemplateFormatHint.defaultProps = { export default function GeneralSettings({ options, onOptionsChange }) { const [onOptionsChangeDebounced] = useDebouncedCallback(onOptionsChange, 200); - const [geoJson] = useLoadGeoJson(options.mapUrl); + const [geoJson] = useLoadGeoJson(getMapUrl(options.mapType, options.customMapUrl)); const geoJsonFields = useMemo(() => getGeoJsonFields(geoJson), [geoJson]); const templateFormatHint = ; diff --git a/client/app/visualizations/choropleth/Editor/GeneralSettings.jsx b/client/app/visualizations/choropleth/Editor/GeneralSettings.jsx index 25c4ab7f1a..6d65e7628d 100644 --- a/client/app/visualizations/choropleth/Editor/GeneralSettings.jsx +++ b/client/app/visualizations/choropleth/Editor/GeneralSettings.jsx @@ -1,60 +1,56 @@ -import { isString, map, filter, find } from "lodash"; -import React, { useMemo, useState, useCallback } from "react"; +import { isString, map, filter } from "lodash"; +import React, { useMemo } from "react"; +import { useDebouncedCallback } from "use-debounce"; import * as Grid from "antd/lib/grid"; import { EditorPropTypes } from "@/visualizations/prop-types"; -import { Section, Select } from "@/components/visualizations/editor"; +import { Section, Select, Input } from "@/components/visualizations/editor"; import useLoadGeoJson from "../hooks/useLoadGeoJson"; -import availableMaps from "../maps"; +import availableMaps, { getMapUrl } from "../maps"; import { getGeoJsonFields } from "./utils"; -function getMapLabel(mapUrl) { - const result = find(availableMaps, item => item.url === mapUrl); - return result ? result.name : mapUrl; -} - export default function GeneralSettings({ options, data, onOptionsChange }) { - const [geoJson, isLoadingGeoJson] = useLoadGeoJson(options.mapUrl); + const [geoJson, isLoadingGeoJson] = useLoadGeoJson(getMapUrl(options.mapType, options.customMapUrl)); const geoJsonFields = useMemo(() => getGeoJsonFields(geoJson), [geoJson]); - const [customGeoJsonUrl, setCustomGeoJsonUrl] = useState(null); // While geoJson is loading - show last selected field in select const targetFields = isLoadingGeoJson ? filter([options.targetField], isString) : geoJsonFields; - const handleMapUrlChange = useCallback( - mapUrl => { - if (!mapUrl) { - setCustomGeoJsonUrl(null); - } - onOptionsChange({ mapUrl: mapUrl || null }); - }, - [onOptionsChange] - ); + const [handleCustomMapUrlChange] = useDebouncedCallback(customMapUrl => { + onOptionsChange({ customMapUrl }); + }, 200); return (
+ {options.mapType === "custom" && ( +
+ handleCustomMapUrlChange(event.target.value)} + /> +
+ )} +
diff --git a/client/app/visualizations/choropleth/Renderer/index.jsx b/client/app/visualizations/choropleth/Renderer/index.jsx index 1536430ba8..12786c535e 100644 --- a/client/app/visualizations/choropleth/Renderer/index.jsx +++ b/client/app/visualizations/choropleth/Renderer/index.jsx @@ -4,13 +4,14 @@ import { RendererPropTypes } from "@/visualizations/prop-types"; import useMemoWithDeepCompare from "@/lib/hooks/useMemoWithDeepCompare"; import useLoadGeoJson from "../hooks/useLoadGeoJson"; +import { getMapUrl } from "../maps"; import initChoropleth from "./initChoropleth"; import { prepareData } from "./utils"; import "./renderer.less"; export default function Renderer({ data, options, onOptionsChange }) { const [container, setContainer] = useState(null); - const [geoJson] = useLoadGeoJson(options.mapUrl); + const [geoJson] = useLoadGeoJson(getMapUrl(options.mapType, options.customMapUrl)); const optionsWithoutBounds = useMemoWithDeepCompare(() => omit(options, ["bounds"]), [options]); diff --git a/client/app/visualizations/choropleth/getOptions.js b/client/app/visualizations/choropleth/getOptions.js index 5b7b7e7820..5e1b44187c 100644 --- a/client/app/visualizations/choropleth/getOptions.js +++ b/client/app/visualizations/choropleth/getOptions.js @@ -1,9 +1,12 @@ -import { isNil, merge } from "lodash"; +import { isNil, merge, first, keys } from "lodash"; import ColorPalette from "./ColorPalette"; import availableMaps from "./maps"; +const defaultMap = first(keys(availableMaps)); + const DEFAULT_OPTIONS = { - mapUrl: availableMaps.countries.url, + mapType: defaultMap, + customMapUrl: null, keyColumn: null, targetField: null, valueColumn: null, @@ -36,14 +39,19 @@ const DEFAULT_OPTIONS = { export default function getOptions(options) { const result = merge({}, DEFAULT_OPTIONS, options); - // backward compatibility - if (!isNil(result.mapType)) { - result.mapUrl = availableMaps[result.mapType] ? availableMaps[result.mapType].url : null; - delete result.mapType; + if (result.mapType !== "custom") { + result.customMapUrl = null; + if (isNil(availableMaps[result.mapType])) { + result.mapType = defaultMap; + } + } + // backward compatibility + if (!isNil(result.countryCodeColumn)) { result.keyColumn = result.countryCodeColumn; delete result.countryCodeColumn; - + } + if (!isNil(result.countryCodeType)) { result.targetField = result.countryCodeType; delete result.countryCodeType; } diff --git a/client/app/visualizations/choropleth/hooks/useLoadGeoJson.js b/client/app/visualizations/choropleth/hooks/useLoadGeoJson.js index 34bffd6e62..4056e47c14 100644 --- a/client/app/visualizations/choropleth/hooks/useLoadGeoJson.js +++ b/client/app/visualizations/choropleth/hooks/useLoadGeoJson.js @@ -8,16 +8,16 @@ const defaultGeoJson = { }; // TODO: It needs some cache -export default function useLoadGeoJson(mapUrl) { +export default function useLoadGeoJson(url) { const [geoJson, setGeoJson] = useState(defaultGeoJson); const [isLoading, setIsLoading] = useState(false); useEffect(() => { - if (isString(mapUrl)) { + if (isString(url)) { setIsLoading(true); let cancelled = false; axios - .get(mapUrl) + .get(url) .catch(() => defaultGeoJson) .then(data => { if (!cancelled) { @@ -33,7 +33,7 @@ export default function useLoadGeoJson(mapUrl) { setGeoJson(defaultGeoJson); setIsLoading(false); } - }, [mapUrl]); + }, [url]); return [geoJson, isLoading]; } diff --git a/client/app/visualizations/choropleth/maps/index.js b/client/app/visualizations/choropleth/maps/index.js index e09ca3520b..3fc1464c61 100644 --- a/client/app/visualizations/choropleth/maps/index.js +++ b/client/app/visualizations/choropleth/maps/index.js @@ -1,7 +1,7 @@ import countriesDataUrl from "./countries.geo.json"; import subdivJapanDataUrl from "./japan.prefectures.geo.json"; -export default { +const availableMaps = { countries: { name: "Countries", url: countriesDataUrl, @@ -11,3 +11,9 @@ export default { url: subdivJapanDataUrl, }, }; + +export function getMapUrl(mapType, defaultUrl) { + return availableMaps[mapType] ? availableMaps[mapType].url : defaultUrl; +} + +export default availableMaps; diff --git a/client/cypress/integration/visualizations/choropleth_spec.js b/client/cypress/integration/visualizations/choropleth_spec.js index bd5710230b..e43440d840 100644 --- a/client/cypress/integration/visualizations/choropleth_spec.js +++ b/client/cypress/integration/visualizations/choropleth_spec.js @@ -49,8 +49,8 @@ describe("Choropleth", () => { cy.clickThrough(` VisualizationEditor.Tabs.General - Choropleth.Editor.MapUrl - Choropleth.Editor.MapUrl.Countries + Choropleth.Editor.MapType + Choropleth.Editor.MapType.countries Choropleth.Editor.KeyColumn Choropleth.Editor.KeyColumn.name Choropleth.Editor.TargetField From 491176466316c8438b4df5d75bb8f26a1d832e8b Mon Sep 17 00:00:00 2001 From: Levko Kravets Date: Wed, 29 Jan 2020 13:05:10 +0200 Subject: [PATCH 03/10] Keep last custom map URL when selecting predefined map type --- client/app/visualizations/choropleth/getOptions.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/client/app/visualizations/choropleth/getOptions.js b/client/app/visualizations/choropleth/getOptions.js index 5e1b44187c..1801938191 100644 --- a/client/app/visualizations/choropleth/getOptions.js +++ b/client/app/visualizations/choropleth/getOptions.js @@ -39,11 +39,8 @@ const DEFAULT_OPTIONS = { export default function getOptions(options) { const result = merge({}, DEFAULT_OPTIONS, options); - if (result.mapType !== "custom") { - result.customMapUrl = null; - if (isNil(availableMaps[result.mapType])) { - result.mapType = defaultMap; - } + if (isNil(availableMaps[result.mapType]) && result.mapType !== "custom") { + result.mapType = defaultMap; } // backward compatibility From 3b29f0c0a754cd011a10d9cc8048f66c51ab3e6e Mon Sep 17 00:00:00 2001 From: Levko Kravets Date: Wed, 29 Jan 2020 13:05:54 +0200 Subject: [PATCH 04/10] Use cache for geoJson requests --- client/app/lib/referenceCountingCache.js | 39 +++++++++++++++++++ .../choropleth/hooks/useLoadGeoJson.js | 22 ++++++----- 2 files changed, 51 insertions(+), 10 deletions(-) create mode 100644 client/app/lib/referenceCountingCache.js diff --git a/client/app/lib/referenceCountingCache.js b/client/app/lib/referenceCountingCache.js new file mode 100644 index 0000000000..5cc68c8ac8 --- /dev/null +++ b/client/app/lib/referenceCountingCache.js @@ -0,0 +1,39 @@ +import { each } from "lodash"; + +export default function createReferenceCountingCache({ cleanupDelay = 2000 } = {}) { + const items = {}; + + function cleanup() { + setTimeout(() => { + each(items, (item, key) => { + if (item.refCount <= 0) { + delete items[key]; + } + }); + }, cleanupDelay); + } + + function get(key, getter) { + if (!items[key]) { + items[key] = { + value: getter(), + refCount: 0, + }; + } + const item = items[key]; + item.refCount += 1; + return item.value; + } + + function release(key) { + if (items[key]) { + const item = items[key]; + if (item.refCount > 0) { + item.refCount -= 1; + cleanup(); + } + } + } + + return { get, release }; +} diff --git a/client/app/visualizations/choropleth/hooks/useLoadGeoJson.js b/client/app/visualizations/choropleth/hooks/useLoadGeoJson.js index 4056e47c14..a500f7e283 100644 --- a/client/app/visualizations/choropleth/hooks/useLoadGeoJson.js +++ b/client/app/visualizations/choropleth/hooks/useLoadGeoJson.js @@ -1,13 +1,15 @@ import { isString, isObject } from "lodash"; import { useState, useEffect } from "react"; import { axios } from "@/services/axios"; +import createReferenceCountingCache from "@/lib/referenceCountingCache"; const defaultGeoJson = { type: "FeatureCollection", features: [], }; -// TODO: It needs some cache +const cache = createReferenceCountingCache(); + export default function useLoadGeoJson(url) { const [geoJson, setGeoJson] = useState(defaultGeoJson); const [isLoading, setIsLoading] = useState(false); @@ -16,18 +18,18 @@ export default function useLoadGeoJson(url) { if (isString(url)) { setIsLoading(true); let cancelled = false; - axios - .get(url) - .catch(() => defaultGeoJson) - .then(data => { - if (!cancelled) { - setGeoJson(isObject(data) ? data : defaultGeoJson); - setIsLoading(false); - } - }); + + const promise = cache.get(url, () => axios.get(url).catch(() => defaultGeoJson)); + promise.then(data => { + if (!cancelled) { + setGeoJson(isObject(data) ? data : defaultGeoJson); + setIsLoading(false); + } + }); return () => { cancelled = true; + cache.release(url); }; } else { setGeoJson(defaultGeoJson); From 3f280b1f6ed87f6972e0e043beccb3ab854b4f53 Mon Sep 17 00:00:00 2001 From: Levko Kravets Date: Wed, 29 Jan 2020 13:40:03 +0200 Subject: [PATCH 05/10] Don't handle bounds changes while loading geoJson data --- .../app/visualizations/choropleth/Renderer/index.jsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/client/app/visualizations/choropleth/Renderer/index.jsx b/client/app/visualizations/choropleth/Renderer/index.jsx index 12786c535e..501699dc0e 100644 --- a/client/app/visualizations/choropleth/Renderer/index.jsx +++ b/client/app/visualizations/choropleth/Renderer/index.jsx @@ -11,7 +11,7 @@ import "./renderer.less"; export default function Renderer({ data, options, onOptionsChange }) { const [container, setContainer] = useState(null); - const [geoJson] = useLoadGeoJson(getMapUrl(options.mapType, options.customMapUrl)); + const [geoJson, isLoadingGeoJson] = useLoadGeoJson(getMapUrl(options.mapType, options.customMapUrl)); const optionsWithoutBounds = useMemoWithDeepCompare(() => omit(options, ["bounds"]), [options]); @@ -38,18 +38,18 @@ export default function Renderer({ data, options, onOptionsChange }) { }, [map, geoJson, data, optionsWithoutBounds]); // eslint-disable-line react-hooks/exhaustive-deps useEffect(() => { - if (map) { + if (map && !isLoadingGeoJson) { map.updateBounds(options.bounds); } - }, [map, options.bounds]); + }, [map, isLoadingGeoJson, options.bounds]); useEffect(() => { - if (map && onOptionsChange) { + if (map && onOptionsChange && !isLoadingGeoJson) { map.onBoundsChange = bounds => { onOptionsChange(merge({}, options, { bounds })); }; } - }, [map, options, onOptionsChange]); + }, [map, isLoadingGeoJson, options, onOptionsChange]); return (
From 6187448e6ad3694311c49ce4ca4a444df70fe111 Mon Sep 17 00:00:00 2001 From: Levko Kravets Date: Wed, 29 Jan 2020 23:36:27 +0200 Subject: [PATCH 06/10] Choropleth: fix map "jumping" on load; don't save bounds if user didn't edit them; refine code a bit --- .../choropleth/Editor/BoundsSettings.jsx | 52 ++++++++++++++++--- .../visualizations/choropleth/Editor/utils.js | 17 +++++- .../choropleth/Renderer/index.jsx | 23 ++++---- .../choropleth/Renderer/initChoropleth.js | 33 +++++------- .../visualizations/choropleth/getOptions.js | 5 +- .../choropleth/hooks/useLoadGeoJson.js | 13 ++--- 6 files changed, 88 insertions(+), 55 deletions(-) diff --git a/client/app/visualizations/choropleth/Editor/BoundsSettings.jsx b/client/app/visualizations/choropleth/Editor/BoundsSettings.jsx index 289844bd7b..b7a643f496 100644 --- a/client/app/visualizations/choropleth/Editor/BoundsSettings.jsx +++ b/client/app/visualizations/choropleth/Editor/BoundsSettings.jsx @@ -1,10 +1,14 @@ -import { isFinite, cloneDeep } from "lodash"; +import { isArray, isFinite, cloneDeep } from "lodash"; import React, { useState, useEffect, useCallback } from "react"; import { useDebouncedCallback } from "use-debounce"; import * as Grid from "antd/lib/grid"; import { Section, InputNumber, ControlLabel } from "@/components/visualizations/editor"; import { EditorPropTypes } from "@/visualizations/prop-types"; +import useLoadGeoJson from "../hooks/useLoadGeoJson"; +import { getMapUrl } from "../maps"; +import { getGeoJsonBounds } from "./utils"; + export default function BoundsSettings({ options, onOptionsChange }) { // Bounds may be changed in editor or on preview (by drag/zoom map). // Changes from preview does not come frequently (only when user release mouse button), @@ -12,13 +16,23 @@ export default function BoundsSettings({ options, onOptionsChange }) { // Therefore this component has intermediate state to hold immediate user input, // which is updated from `options.bounds` and by inputs immediately on user input, // but `onOptionsChange` event is debounced and uses last value from internal state. - const [bounds, setBounds] = useState(options.bounds); const [onOptionsChangeDebounced] = useDebouncedCallback(onOptionsChange, 200); + const [geoJson] = useLoadGeoJson(getMapUrl(options.mapType, options.customMapUrl)); + + // `options.bounds` could be empty only if user didn't edit bounds yet - through preview or in this editor. + // In this case we should keep empty bounds value because it tells renderer to fit map every time. useEffect(() => { - setBounds(options.bounds); - }, [options.bounds]); + if (options.bounds) { + setBounds(options.bounds); + } else { + const defaultBounds = getGeoJsonBounds(geoJson); + if (defaultBounds) { + setBounds(defaultBounds); + } + } + }, [options.bounds, geoJson]); const updateBounds = useCallback( (i, j, v) => { @@ -33,16 +47,28 @@ export default function BoundsSettings({ options, onOptionsChange }) { [bounds, onOptionsChangeDebounced] ); + const boundsAvailable = isArray(bounds); + return (
- updateBounds(1, 0, value)} /> + updateBounds(1, 0, value)} + /> - updateBounds(1, 1, value)} /> + updateBounds(1, 1, value)} + /> @@ -52,10 +78,20 @@ export default function BoundsSettings({ options, onOptionsChange }) { - updateBounds(0, 0, value)} /> + updateBounds(0, 0, value)} + /> - updateBounds(0, 1, value)} /> + updateBounds(0, 1, value)} + /> diff --git a/client/app/visualizations/choropleth/Editor/utils.js b/client/app/visualizations/choropleth/Editor/utils.js index 6aacfa5c5e..836b0a8907 100644 --- a/client/app/visualizations/choropleth/Editor/utils.js +++ b/client/app/visualizations/choropleth/Editor/utils.js @@ -1,6 +1,5 @@ -/* eslint-disable import/prefer-default-export */ - import { isObject, isArray, reduce, keys, uniq } from "lodash"; +import L from "leaflet"; export function getGeoJsonFields(geoJson) { const features = isObject(geoJson) && isArray(geoJson.features) ? geoJson.features : []; @@ -13,3 +12,17 @@ export function getGeoJsonFields(geoJson) { [] ); } + +export function getGeoJsonBounds(geoJson) { + if (isObject(geoJson)) { + const layer = L.geoJSON(geoJson); + const bounds = layer.getBounds(); + if (bounds.isValid()) { + return [ + [bounds._southWest.lat, bounds._southWest.lng], + [bounds._northEast.lat, bounds._northEast.lng], + ]; + } + } + return null; +} diff --git a/client/app/visualizations/choropleth/Renderer/index.jsx b/client/app/visualizations/choropleth/Renderer/index.jsx index 501699dc0e..56fa53aae8 100644 --- a/client/app/visualizations/choropleth/Renderer/index.jsx +++ b/client/app/visualizations/choropleth/Renderer/index.jsx @@ -1,5 +1,5 @@ -import { omit, merge } from "lodash"; -import React, { useState, useEffect } from "react"; +import { omit, noop } from "lodash"; +import React, { useState, useEffect, useRef } from "react"; import { RendererPropTypes } from "@/visualizations/prop-types"; import useMemoWithDeepCompare from "@/lib/hooks/useMemoWithDeepCompare"; @@ -11,7 +11,9 @@ import "./renderer.less"; export default function Renderer({ data, options, onOptionsChange }) { const [container, setContainer] = useState(null); - const [geoJson, isLoadingGeoJson] = useLoadGeoJson(getMapUrl(options.mapType, options.customMapUrl)); + const [geoJson] = useLoadGeoJson(getMapUrl(options.mapType, options.customMapUrl)); + const onBoundsChangeRef = useRef(); + onBoundsChangeRef.current = onOptionsChange ? bounds => onOptionsChange({ ...options, bounds }) : noop; const optionsWithoutBounds = useMemoWithDeepCompare(() => omit(options, ["bounds"]), [options]); @@ -19,7 +21,7 @@ export default function Renderer({ data, options, onOptionsChange }) { useEffect(() => { if (container) { - const _map = initChoropleth(container); + const _map = initChoropleth(container, (...args) => onBoundsChangeRef.current(...args)); setMap(_map); return () => { _map.destroy(); @@ -37,19 +39,12 @@ export default function Renderer({ data, options, onOptionsChange }) { } }, [map, geoJson, data, optionsWithoutBounds]); // eslint-disable-line react-hooks/exhaustive-deps + // This may come only from editor useEffect(() => { - if (map && !isLoadingGeoJson) { + if (map) { map.updateBounds(options.bounds); } - }, [map, isLoadingGeoJson, options.bounds]); - - useEffect(() => { - if (map && onOptionsChange && !isLoadingGeoJson) { - map.onBoundsChange = bounds => { - onOptionsChange(merge({}, options, { bounds })); - }; - } - }, [map, isLoadingGeoJson, options, onOptionsChange]); + }, [map, options.bounds]); return (
diff --git a/client/app/visualizations/choropleth/Renderer/initChoropleth.js b/client/app/visualizations/choropleth/Renderer/initChoropleth.js index b774513fa4..c4c702c982 100644 --- a/client/app/visualizations/choropleth/Renderer/initChoropleth.js +++ b/client/app/visualizations/choropleth/Renderer/initChoropleth.js @@ -82,7 +82,7 @@ function validateBounds(bounds, fallbackBounds) { return null; } -export default function initChoropleth(container) { +export default function initChoropleth(container, onBoundsChange) { const _map = L.map(container, { center: [0.0, 0.0], zoom: 1, @@ -95,13 +95,14 @@ export default function initChoropleth(container) { let _choropleth = null; const _legend = new CustomControl(); - let onBoundsChange = () => {}; function handleMapBoundsChange() { - const bounds = _map.getBounds(); - onBoundsChange([ - [bounds._southWest.lat, bounds._southWest.lng], - [bounds._northEast.lat, bounds._northEast.lng], - ]); + if (isFunction(onBoundsChange)) { + const bounds = _map.getBounds(); + onBoundsChange([ + [bounds._southWest.lat, bounds._southWest.lng], + [bounds._northEast.lat, bounds._northEast.lng], + ]); + } } let boundsChangedFromMap = false; @@ -138,15 +139,11 @@ export default function initChoropleth(container) { const mapBounds = _choropleth.getBounds(); const bounds = validateBounds(options.bounds, mapBounds); - if (bounds) { - _map.fitBounds(bounds, { animate: false, duration: 0 }); - _map.setMaxBounds(mapBounds); - } + _map.fitBounds(bounds, { animate: false, duration: 0 }); - // send updated bounds to editor; delay this to avoid infinite update loop - setTimeout(() => { - handleMapBoundsChange(); - }, 10); + // equivalent to `_map.setMaxBounds(mapBounds)` but without animation + _map.options.maxBounds = mapBounds; + _map.panInsideBounds(mapBounds, { animate: false, duration: 0 }); // update legend if (options.legend.visible && legend.length > 0) { @@ -177,12 +174,6 @@ export default function initChoropleth(container) { }); return { - get onBoundsChange() { - return onBoundsChange; - }, - set onBoundsChange(value) { - onBoundsChange = isFunction(value) ? value : () => {}; - }, updateLayers, updateBounds, destroy() { diff --git a/client/app/visualizations/choropleth/getOptions.js b/client/app/visualizations/choropleth/getOptions.js index 1801938191..e1cfc4d5f6 100644 --- a/client/app/visualizations/choropleth/getOptions.js +++ b/client/app/visualizations/choropleth/getOptions.js @@ -1,4 +1,4 @@ -import { isNil, merge, first, keys } from "lodash"; +import { isNil, merge, first, keys, get } from "lodash"; import ColorPalette from "./ColorPalette"; import availableMaps from "./maps"; @@ -38,6 +38,9 @@ const DEFAULT_OPTIONS = { export default function getOptions(options) { const result = merge({}, DEFAULT_OPTIONS, options); + // Both renderer and editor always provide new `bounds` array, so no need to clone it here. + // Keeping original object also reduces amount of updates in components + result.bounds = get(options, "bounds"); if (isNil(availableMaps[result.mapType]) && result.mapType !== "custom") { result.mapType = defaultMap; diff --git a/client/app/visualizations/choropleth/hooks/useLoadGeoJson.js b/client/app/visualizations/choropleth/hooks/useLoadGeoJson.js index a500f7e283..5c3894ff2d 100644 --- a/client/app/visualizations/choropleth/hooks/useLoadGeoJson.js +++ b/client/app/visualizations/choropleth/hooks/useLoadGeoJson.js @@ -3,15 +3,10 @@ import { useState, useEffect } from "react"; import { axios } from "@/services/axios"; import createReferenceCountingCache from "@/lib/referenceCountingCache"; -const defaultGeoJson = { - type: "FeatureCollection", - features: [], -}; - const cache = createReferenceCountingCache(); export default function useLoadGeoJson(url) { - const [geoJson, setGeoJson] = useState(defaultGeoJson); + const [geoJson, setGeoJson] = useState(null); const [isLoading, setIsLoading] = useState(false); useEffect(() => { @@ -19,10 +14,10 @@ export default function useLoadGeoJson(url) { setIsLoading(true); let cancelled = false; - const promise = cache.get(url, () => axios.get(url).catch(() => defaultGeoJson)); + const promise = cache.get(url, () => axios.get(url).catch(() => null)); promise.then(data => { if (!cancelled) { - setGeoJson(isObject(data) ? data : defaultGeoJson); + setGeoJson(isObject(data) ? data : null); setIsLoading(false); } }); @@ -32,7 +27,7 @@ export default function useLoadGeoJson(url) { cache.release(url); }; } else { - setGeoJson(defaultGeoJson); + setGeoJson(null); setIsLoading(false); } }, [url]); From b331c4c922f32e713e8ff37b1dbedd11b1437bbe Mon Sep 17 00:00:00 2001 From: Levko Kravets Date: Thu, 30 Jan 2020 00:02:19 +0200 Subject: [PATCH 07/10] Improve cache; fix typo --- client/app/lib/referenceCountingCache.js | 22 +++++++++---------- .../choropleth/Editor/GeneralSettings.jsx | 2 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/client/app/lib/referenceCountingCache.js b/client/app/lib/referenceCountingCache.js index 5cc68c8ac8..35ff4721d0 100644 --- a/client/app/lib/referenceCountingCache.js +++ b/client/app/lib/referenceCountingCache.js @@ -1,17 +1,15 @@ -import { each } from "lodash"; +import { each, debounce } from "lodash"; export default function createReferenceCountingCache({ cleanupDelay = 2000 } = {}) { const items = {}; - function cleanup() { - setTimeout(() => { - each(items, (item, key) => { - if (item.refCount <= 0) { - delete items[key]; - } - }); - }, cleanupDelay); - } + const cleanup = debounce(() => { + each(items, (item, key) => { + if (item.refCount <= 0) { + delete items[key]; + } + }); + }, cleanupDelay); function get(key, getter) { if (!items[key]) { @@ -30,7 +28,9 @@ export default function createReferenceCountingCache({ cleanupDelay = 2000 } = { const item = items[key]; if (item.refCount > 0) { item.refCount -= 1; - cleanup(); + if (item.refCount <= 0) { + cleanup(); + } } } } diff --git a/client/app/visualizations/choropleth/Editor/GeneralSettings.jsx b/client/app/visualizations/choropleth/Editor/GeneralSettings.jsx index 6d65e7628d..663dea7694 100644 --- a/client/app/visualizations/choropleth/Editor/GeneralSettings.jsx +++ b/client/app/visualizations/choropleth/Editor/GeneralSettings.jsx @@ -70,7 +70,7 @@ export default function GeneralSettings({ options, data, onOptionsChange }) {