From 06c3c92fd616c19d3cc12ffa589a90d1eaa96b1b Mon Sep 17 00:00:00 2001 From: Alexander Clausen <alex@gc-web.de> Date: Sun, 6 Jan 2019 05:35:11 +0100 Subject: [PATCH 1/7] Ensure key uniqueness in FeaturePropertyPopup --- src/components/map/FeaturePropertyPopup.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/map/FeaturePropertyPopup.jsx b/src/components/map/FeaturePropertyPopup.jsx index 5d5c9e5..a018349 100644 --- a/src/components/map/FeaturePropertyPopup.jsx +++ b/src/components/map/FeaturePropertyPopup.jsx @@ -22,7 +22,7 @@ function renderProperties(feature) { } function renderFeature(feature) { - return <div key={feature.id}> + return <div key={`${feature.sourceLayer}-${feature.id}`}> <div className="maputnik-popup-layer-id">{feature.layer['source-layer']}{feature.inspectModeCounter && <span> × {feature.inspectModeCounter}</span>}</div> <InputBlock key={"property-type"} label={"$type"}> <StringInput value={feature.geometry.type} style={{backgroundColor: 'transparent'}} /> From 7ff0524bb7e32e4161b4c79ede372d6649949fd8 Mon Sep 17 00:00:00 2001 From: Alexander Clausen <alex@gc-web.de> Date: Sun, 6 Jan 2019 05:35:43 +0100 Subject: [PATCH 2/7] Allow inspect mode to count properly Example: https://maputnik.github.io/editor/#12.86/54.38618/9.76697 --- src/components/map/FeaturePropertyPopup.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/map/FeaturePropertyPopup.jsx b/src/components/map/FeaturePropertyPopup.jsx index a018349..71b9983 100644 --- a/src/components/map/FeaturePropertyPopup.jsx +++ b/src/components/map/FeaturePropertyPopup.jsx @@ -43,7 +43,7 @@ function removeDuplicatedFeatures(features) { if(featureIndex === -1) { uniqueFeatures.push(feature) } else { - if(uniqueFeatures[featureIndex].hasOwnProperty('counter')) { + if(uniqueFeatures[featureIndex].hasOwnProperty('inspectModeCounter')) { uniqueFeatures[featureIndex].inspectModeCounter++ } else { uniqueFeatures[featureIndex].inspectModeCounter = 2 From 84654e81af22409233d32f1ed94188cd0b65077f Mon Sep 17 00:00:00 2001 From: Alexander Clausen <alex@gc-web.de> Date: Sun, 6 Jan 2019 05:39:19 +0100 Subject: [PATCH 3/7] MapboxGlMap: remove unused state variables --- src/components/map/MapboxGlMap.jsx | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/components/map/MapboxGlMap.jsx b/src/components/map/MapboxGlMap.jsx index 0a482f9..44ba32a 100644 --- a/src/components/map/MapboxGlMap.jsx +++ b/src/components/map/MapboxGlMap.jsx @@ -77,9 +77,6 @@ export default class MapboxGlMap extends React.Component { this.state = { map: null, inspect: null, - isPopupOpen: false, - popupX: 0, - popupY: 0, } } From 8f391d7d523c5f8df6d0d048d59ef09889269d0f Mon Sep 17 00:00:00 2001 From: Alexander Clausen <alex@gc-web.de> Date: Sun, 6 Jan 2019 05:49:03 +0100 Subject: [PATCH 4/7] Cleanup popup nodes Before, the component instances used for rendering popup content were kept around, slowly leaking memory. This could be observed using react developer tools. --- src/components/map/MapboxGlMap.jsx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/components/map/MapboxGlMap.jsx b/src/components/map/MapboxGlMap.jsx index 44ba32a..59349d0 100644 --- a/src/components/map/MapboxGlMap.jsx +++ b/src/components/map/MapboxGlMap.jsx @@ -17,10 +17,12 @@ import '../../libs/mapbox-rtl' const IS_SUPPORTED = MapboxGl.supported(); -function renderPropertyPopup(features) { +function renderPopup(popup) { var mountNode = document.createElement('div'); - ReactDOM.render(<FeaturePropertyPopup features={features} />, mountNode) - return mountNode.innerHTML; + ReactDOM.render(popup, mountNode) + var content = mountNode.innerHTML; + ReactDOM.unmountComponentAtNode(mountNode); + return content; } function buildInspectStyle(originalMapStyle, coloredLayers, highlightedLayer) { @@ -162,11 +164,9 @@ export default class MapboxGlMap extends React.Component { buildInspectStyle: (originalMapStyle, coloredLayers) => buildInspectStyle(originalMapStyle, coloredLayers, this.props.highlightedLayer), renderPopup: features => { if(this.props.inspectModeEnabled) { - return renderPropertyPopup(features) + return renderPopup(<FeaturePropertyPopup features={features} />); } else { - var mountNode = document.createElement('div'); - ReactDOM.render(<FeatureLayerPopup features={features} onLayerSelect={this.props.onLayerSelect} />, mountNode) - return mountNode + return renderPopup(<FeatureLayerPopup features={features} onLayerSelect={this.props.onLayerSelect} />); } } }) From 1375240bfa7461333043be541d7b5714d8285cb3 Mon Sep 17 00:00:00 2001 From: Alexander Clausen <alex@gc-web.de> Date: Sun, 6 Jan 2019 06:01:04 +0100 Subject: [PATCH 5/7] For better debuggability, actually keep one popup node around --- src/components/map/MapboxGlMap.jsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/components/map/MapboxGlMap.jsx b/src/components/map/MapboxGlMap.jsx index 59349d0..0e2afa5 100644 --- a/src/components/map/MapboxGlMap.jsx +++ b/src/components/map/MapboxGlMap.jsx @@ -17,11 +17,9 @@ import '../../libs/mapbox-rtl' const IS_SUPPORTED = MapboxGl.supported(); -function renderPopup(popup) { - var mountNode = document.createElement('div'); - ReactDOM.render(popup, mountNode) +function renderPopup(popup, mountNode) { + ReactDOM.render(popup, mountNode); var content = mountNode.innerHTML; - ReactDOM.unmountComponentAtNode(mountNode); return content; } @@ -149,6 +147,8 @@ export default class MapboxGlMap extends React.Component { const nav = new MapboxGl.NavigationControl(); map.addControl(nav, 'top-right'); + const tmpNode = document.createElement('div'); + const inspect = new MapboxInspect({ popup: new MapboxGl.Popup({ closeOnClick: false @@ -164,9 +164,9 @@ export default class MapboxGlMap extends React.Component { buildInspectStyle: (originalMapStyle, coloredLayers) => buildInspectStyle(originalMapStyle, coloredLayers, this.props.highlightedLayer), renderPopup: features => { if(this.props.inspectModeEnabled) { - return renderPopup(<FeaturePropertyPopup features={features} />); + return renderPopup(<FeaturePropertyPopup features={features} />, tmpNode); } else { - return renderPopup(<FeatureLayerPopup features={features} onLayerSelect={this.props.onLayerSelect} />); + return renderPopup(<FeatureLayerPopup features={features} onLayerSelect={this.props.onLayerSelect} />, tmpNode); } } }) From 6cf861d44e1f07eee9c58795100dde1acc0c6996 Mon Sep 17 00:00:00 2001 From: Alexander Clausen <alex@gc-web.de> Date: Sun, 6 Jan 2019 06:02:10 +0100 Subject: [PATCH 6/7] Keep inspect mode consistent across renderer changes If you were in inspect mode, switched to open layers, and back to MapboxGlMap, the state in MapboxGlMap and MapboxInspect diverged, meaning MapboxGlMap thought it was in inspect mode, while MapboxInspect had inspector mode disabled. --- src/components/map/MapboxGlMap.jsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/components/map/MapboxGlMap.jsx b/src/components/map/MapboxGlMap.jsx index 0e2afa5..5765f84 100644 --- a/src/components/map/MapboxGlMap.jsx +++ b/src/components/map/MapboxGlMap.jsx @@ -174,6 +174,9 @@ export default class MapboxGlMap extends React.Component { map.on("style.load", () => { this.setState({ map, inspect }); + if(this.props.inspectModeEnabled) { + inspect.toggleInspector(); + } }) map.on("data", e => { From 14e0385575772e92cfb956a65448c2d2e75b4b6b Mon Sep 17 00:00:00 2001 From: Alexander Clausen <alex@gc-web.de> Date: Sun, 6 Jan 2019 06:20:03 +0100 Subject: [PATCH 7/7] Reflect current view in dropdown Useful if the view is toggled between inspect and map via keyboard shortcut. --- src/components/Toolbar.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Toolbar.jsx b/src/components/Toolbar.jsx index cba1350..e76c984 100644 --- a/src/components/Toolbar.jsx +++ b/src/components/Toolbar.jsx @@ -198,7 +198,7 @@ export default class Toolbar extends React.Component { <ToolbarSelect wdKey="nav:inspect"> <MdFindInPage /> <IconText>View </IconText> - <select onChange={(e) => this.handleSelection(e.target.value)}> + <select onChange={(e) => this.handleSelection(e.target.value)} value={currentView.id}> {views.map((item) => { return ( <option key={item.id} value={item.id}>