From 5baa1bb3d5094021cfc72890bd51688320b57839 Mon Sep 17 00:00:00 2001 From: Bjorn Sandvik Date: Tue, 18 Feb 2020 10:19:38 +0100 Subject: [PATCH] fix: map context menu positioning and refactoring (#447) * chore: refactor plugin context menu to use hooks * chore: refactor context menu * fix: upgrade to latest maps-gl --- package.json | 2 +- src/components/map/BoundaryLayer.js | 12 ------ src/components/map/ContextMenu.js | 6 ++- src/components/map/FacilityLayer.js | 10 ----- src/components/map/Layer.js | 18 ++++++++ src/components/map/Map.js | 8 +++- src/components/map/ThematicLayer.js | 10 ----- src/components/plugin/ContextMenu.js | 62 ++++++++++++++-------------- yarn.lock | 8 ++-- 9 files changed, 65 insertions(+), 71 deletions(-) diff --git a/package.json b/package.json index 8caabbcb2..15fdead26 100644 --- a/package.json +++ b/package.json @@ -54,7 +54,7 @@ "@dhis2/d2-ui-org-unit-dialog": "5.2.10", "@dhis2/d2-ui-org-unit-tree": "5.2.10", "@dhis2/gis-api": "^34.0.11", - "@dhis2/maps-gl": "^1.0.3", + "@dhis2/maps-gl": "^1.0.4", "@dhis2/ui-core": "^4.1.1", "@dhis2/ui-widgets": "^2.0.5", "@material-ui/core": "^3.9.3", diff --git a/src/components/map/BoundaryLayer.js b/src/components/map/BoundaryLayer.js index 3be9fff47..b3c94db26 100644 --- a/src/components/map/BoundaryLayer.js +++ b/src/components/map/BoundaryLayer.js @@ -100,20 +100,8 @@ export default class BoundaryLayer extends Layer { this.setState({ popup: evt }); } - onFeatureRightClick(evt) { - const { id, layer } = this.props; - - this.props.openContextMenu({ - ...evt, - layerId: id, - layerType: layer, - }); - } - removeLayer() { this.layer.off('click', this.onFeatureClick, this); - this.layer.off('contextmenu', this.onFeatureRightClick, this); - super.removeLayer(); } } diff --git a/src/components/map/ContextMenu.js b/src/components/map/ContextMenu.js index 10385142e..f69d21aac 100644 --- a/src/components/map/ContextMenu.js +++ b/src/components/map/ContextMenu.js @@ -70,9 +70,11 @@ const ContextMenu = (props, context) => { let attr = {}; if (position) { + const [x, y] = position; + anchorEl.style.position = 'fixed'; - anchorEl.style.left = position[0] + 'px'; - anchorEl.style.top = position[1] + 'px'; + anchorEl.style.left = `${x}px`; + anchorEl.style.top = `${y}px`; } if (feature) { diff --git a/src/components/map/FacilityLayer.js b/src/components/map/FacilityLayer.js index ab974fee1..ee8e62df5 100644 --- a/src/components/map/FacilityLayer.js +++ b/src/components/map/FacilityLayer.js @@ -112,16 +112,6 @@ class FacilityLayer extends Layer { onFeatureClick(evt) { this.setState({ popup: evt }); } - - onFeatureRightClick(evt) { - const { id, layer } = this.props; - - this.props.openContextMenu({ - ...evt, - layerId: id, - layerType: layer, - }); - } } export default FacilityLayer; diff --git a/src/components/map/Layer.js b/src/components/map/Layer.js index a7ba326ee..83b341429 100644 --- a/src/components/map/Layer.js +++ b/src/components/map/Layer.js @@ -12,11 +12,13 @@ class Layer extends PureComponent { dataFilters: PropTypes.object, id: PropTypes.string.isRequired, index: PropTypes.number, + layer: PropTypes.string, editCounter: PropTypes.number, opacity: PropTypes.number, isVisible: PropTypes.bool, config: PropTypes.object, labels: PropTypes.bool, + openContextMenu: PropTypes.func, }; static defaultProps = { @@ -138,6 +140,8 @@ class Layer extends PureComponent { removeLayer() { const map = this.context.map; + this.layer.off('contextmenu', this.onFeatureRightClick, this); + if (map.hasLayer(this.layer)) { map.removeLayer(this.layer); } @@ -149,6 +153,20 @@ class Layer extends PureComponent { return null; } + onFeatureRightClick(evt) { + const [x, y] = evt.position; + const { id, layer } = this.props; + const { map } = this.context; + const { left, top } = map.getContainer().getBoundingClientRect(); + + this.props.openContextMenu({ + ...evt, + position: [left + x, top + y], + layerId: id, + layerType: layer, + }); + } + // Called when a map popup is closed onPopupClose = popup => { // Added check to avoid closing a new popup diff --git a/src/components/map/Map.js b/src/components/map/Map.js index 520b71aa6..e494d6d4d 100644 --- a/src/components/map/Map.js +++ b/src/components/map/Map.js @@ -176,7 +176,13 @@ class Map extends Component { } onRightClick = evt => { - this.props.openContextMenu(evt); + const [x, y] = evt.position; + const { left, top } = this.map.getContainer().getBoundingClientRect(); + + this.props.openContextMenu({ + ...evt, + position: [left + x, top + y], + }); }; onMapReady = map => this.setState({ map }); diff --git a/src/components/map/ThematicLayer.js b/src/components/map/ThematicLayer.js index 3b0e12071..6ed715a57 100644 --- a/src/components/map/ThematicLayer.js +++ b/src/components/map/ThematicLayer.js @@ -153,16 +153,6 @@ class ThematicLayer extends Layer { onFeatureClick(evt) { this.setState({ popup: evt }); } - - onFeatureRightClick(evt) { - const { id, layer } = this.props; - - this.props.openContextMenu({ - ...evt, - layerId: id, - layerType: layer, - }); - } } export default ThematicLayer; diff --git a/src/components/plugin/ContextMenu.js b/src/components/plugin/ContextMenu.js index 64ade7c57..9fc119b2b 100644 --- a/src/components/plugin/ContextMenu.js +++ b/src/components/plugin/ContextMenu.js @@ -1,43 +1,43 @@ -import React, { Component } from 'react'; +import React, { useEffect, useRef } from 'react'; import PropTypes from 'prop-types'; import i18n from '@dhis2/d2-i18n'; import './ContextMenu.css'; -class ContextMenu extends Component { - componentDidUpdate() { - const { position } = this.props; +const ContextMenu = props => { + const { position, feature, onDrill } = props; + const menuEl = useRef(); - if (position) { - this.el.style.left = position[0] + 'px'; - this.el.style.top = position[1] + 'px'; - } - } - - render() { - const { position, feature, onDrill } = this.props; + useEffect(() => { + if (menuEl && position) { + const { style } = menuEl.current; + const [x, y] = position; - if (!position || !feature) { - return null; + style.left = `${x}px`; + style.top = `${y}px`; } + }, [menuEl, position]); - const { hasCoordinatesUp, hasCoordinatesDown } = feature.properties; - - return ( -
(this.el = el)}> - {hasCoordinatesUp && ( -
onDrill('up')}> - {i18n.t('Drill up one level')} -
- )} - {hasCoordinatesDown && ( -
onDrill('down')}> - {i18n.t('Drill down one level')} -
- )} -
- ); + if (!position || !feature) { + return null; } -} + + const { hasCoordinatesUp, hasCoordinatesDown } = feature.properties; + + return ( +
+ {hasCoordinatesUp && ( +
onDrill('up')}> + {i18n.t('Drill up one level')} +
+ )} + {hasCoordinatesDown && ( +
onDrill('down')}> + {i18n.t('Drill down one level')} +
+ )} +
+ ); +}; ContextMenu.propTypes = { feature: PropTypes.object, diff --git a/yarn.lock b/yarn.lock index 15f83ccd4..26a05d7da 100644 --- a/yarn.lock +++ b/yarn.lock @@ -457,10 +457,10 @@ leaflet.markercluster "^1.4.1" whatwg-fetch "^3.0.0" -"@dhis2/maps-gl@^1.0.3": - version "1.0.3" - resolved "https://registry.yarnpkg.com/@dhis2/maps-gl/-/maps-gl-1.0.3.tgz#67d6131e0e2bec0e6796dddfadc436745bc8b073" - integrity sha512-bw37JJr1uUMHszhBSjhGkpqtZ6g0UOPjvcjWTQdfg8r1g1SeMniX1fv7PAcrkziGOQbt46sg2hvA3DDg5nkf8A== +"@dhis2/maps-gl@^1.0.4": + version "1.0.4" + resolved "https://registry.yarnpkg.com/@dhis2/maps-gl/-/maps-gl-1.0.4.tgz#83cc0d91e1c1cbaa26c61616f0e09842a3c62f36" + integrity sha512-4JKH4jMiEJ5v8BOv3iftkLP6CkI5wQaEN7n1K8VlDKTKtMjmjGG6QeofeD7oCpoVUtGfvUiTVEb+n+6bsabYHQ== dependencies: "@mapbox/sphericalmercator" "^1.1.0" "@turf/area" "^6.0.1"