[Feat] Create polygons - Geo-Operations 1 #595

macrigiuseppe · 2019-07-02T23:07:21Z

This pull requests introduce a new Edit mode which will allow users to draw custom polygons on a map to be used as layer filter (to be implemented later).

We support the ability to create/move/delete rectangle or customd shape polygons,

Context menu and delete action

Selecting a feature and clicking on delete button will remove the feature.

Layers

The current implementation supports adding new points to existing features

Implementation details:

In order to implement the dropdown menu for editing mode i re-used the toolbar implementation currently used for sharing features.
I extracted the react component from sharing and moved into it's own file to be re-used in multiple both sharing and editing mode.

I created a new editor folder containing all react components for the editor mode, including a new context menu action panel.

The Editor component which contains react-map-gl-draw is located in the same MapContainer as deck.gl layer to facilitate logic to switch from editor to read mode

** There are still few edge cases to handle but the feature is pretty much integrated.

macrigiuseppe · 2019-07-12T15:30:06Z

Depends on uber/nebula.gl#254

macrigiuseppe · 2019-07-14T23:41:32Z

@heshan0131 this is still WIP but it's touching a lof of files and I would love to have an early feedback

heshan0131 · 2019-07-15T00:31:53Z

docs/user-guides/c-types-of-layers/e-polygon.md

@@ -1,6 +1,6 @@
-# Polygon


is it by accident that all Polygon is renamed to DrawPolygon?

Yeah, maybe a replace all side effect. I will make sure I revert the right ones

macrigiuseppe · 2019-07-15T02:49:35Z

@heshan0131 Reverted all changes and fixed lint errors and warnings

macrigiuseppe · 2019-07-15T02:56:08Z

src/components/map-container.js

@@ -173,86 +259,8 @@ export default function MapContainerFactory(MapPopover, MapControl) {
    };

    /* component render functions */
-    /* eslint-disable complexity */
-    _renderMapPopover() {


i converted this method into a react component to take advantage of react render optimization

macrigiuseppe · 2019-07-15T02:57:24Z

src/components/map/map-control.js


-const MapLegendPanel = ({items, isActive, scale, toggleMenuPanel, isExport}) =>
+const MapLegendPanel = React.memo(({items, isActive, scale, onToggleMenuPanel, isExport}) =>


React.memo aka PureComponent for stateless react

macrigiuseppe · 2019-07-15T02:59:39Z

src/components/modals/export-data-modal.js

@@ -39,8 +39,7 @@ const propTypes = {
  onClose: PropTypes.func.isRequired,
  onChangeExportSelectedDataset: PropTypes.func.isRequired,
  onChangeExportDataType: PropTypes.func.isRequired,
-  onChangeExportFiltered: PropTypes.func.isRequired,
-  onChangeExportConfig: PropTypes.func.isRequired


macrigiuseppe · 2019-07-19T04:05:29Z

@heshan0131 updated pull request, let me know what you think

heshan0131 · 2019-07-19T04:25:45Z

src/components/common/action-panel.js

+  font-size: 12px;
+  line-height: 14px;
+  padding: 8px;
+  height: 32px;


switchHeight and switchWidth is in base.js, this should be added too

heshan0131 · 2019-07-19T04:57:01Z

src/components/common/toolbar-item.js

+`;
+
+const ToolbarItem = React.memo(({active, className, icon, label, onClick}) => (
+  <StyledDiv active={active} className="save-export-dropdown__item" onClick={(e) => {


might want to change the class name here

heshan0131 · 2019-07-19T04:59:49Z

src/components/common/toolbar-item.js

+  </StyledDiv>
+));
+
+ToolbarItem.displayName = 'ToolbarItem';


displayName is not working for memo

indeed an issue with React facebook/react#14319.

I tried to use react-tools on my browser and this is what i have:

same thing for ActionPanelItem:

I wonder what could be the difference

heshan0131 · 2019-07-19T05:38:38Z

src/components/map-container.js

-            mapStyle={mapStyle.bottomMapStyle}
-            getCursor={this.props.hoverInfo ? () => 'pointer' : undefined}
-            transitionDuration={TRANSITION_DURATION}
-            onMouseMove={this.props.visStateActions.onMouseMove}


onMouseMove is removed. tooltop can't render without mouse pos

I think i accidentally removed it when i rebased. It's back now

heshan0131 · 2019-07-19T05:40:15Z

src/components/map-container.js

+          layerHoverProp={layerHoverProp}
+          coordinate={interactionConfig.coordinate.enabled && ((pinned || {}).coordinate || coordinate)}
+          freezed={Boolean(clicked || pinned)}
+          onClose={this._onCloseMapPopover}


onClose needs to be passed in from prop, this is accessible

heshan0131 · 2019-07-19T05:44:01Z

src/components/map-container.js

+      // project lnglat to screen so that tooltip follows the object on zoom
+      const viewport = new WebMercatorViewport(mapState);
+      const lngLat = clicked ? clicked.lngLat : pinned.coordinate;
+      position = this._getHoverXY(viewport, lngLat);


this is not accessible anymore

heshan0131 · 2019-07-19T05:47:36Z

src/components/map-container.js

 ];

 export default function MapContainerFactory(MapPopover, MapControl) {
+  /* eslint-disable complexity */
+  const MapTooltip = React.memo(({


instead of defining it inside MapContainer, it makes more sense to create a factory for MapTooltip, pass it to MapContainer as dep, then pass MapPopover as its dep? so

MapContainer -> MapTooltip -> MapPopover

heshan0131 · 2019-07-19T05:49:45Z

src/components/kepler-gl.js

@@ -244,6 +244,8 @@ function KeplerGlFactory(
        mapboxApiAccessToken,
        mapboxApiUrl,
        mapState,
+        uiState,
+        visState,


items from visState is already picekd out and passed in mapFieds. If you need editor from it, just pass editor

heshan0131 · 2019-07-19T05:54:05Z

src/components/map-container.js

        </StyledMapContainer>
      );
    }
  }

+  MapContainer.displayName = 'MapContainer';


you don't need displayName, you automatically get it by defining it as a class

eslint was complaining about all missing display names. Either we add displayName to all classes or disable the eslint.

heshan0131 · 2019-07-19T06:03:52Z

src/reducers/combined-updaters.js

+
+export const setFeaturesUpdater = (state, payload) => ({
+  ...state,
+  visState: visStateFeaturesUpdater(state.visState, payload),


we don't need a composed updater if an action is handled in each subreducer independently. You can just add [SET_FEATURE] action handlers to visState and uiState reducer

visState = { .... [ActionTypes.SET_FEATURE]: visStateFeaturesUpdater } uiState = { .... [ActionTypes.SET_FEATURE]: uiStateFeaturesUpdater }

The time when we need to use combined updaters is when 1 action needs to be handled in multiple reducer is a certain sequence. e.g. in updateVisData we use layers created by visState to update viewport in mapState1

i initially started with independent actions and I was trying to see whether there was a performance improvement by using a composed action. But at the end i don't think it made any different

you are still creating setFeaturesComposed and deleteFeatureComposed. this 2 actions don't need to be composed here. They can be handled independently inside each (visState, uiState) reducer...

You can delete all new lines inside this file, and it will work

The idea behind these actions is to be used the way we use addDataToMapComposed, directly accessing kepler.gl store rather than using actions

An application can call these composed updaters when it is directly kepler.gl and want to modify the features. I am not using them at all and since i changed the logic in the second PR i can get rid of them
i am not even using those actions anywhere, i removed them

macrigiuseppe · 2019-07-22T06:08:04Z

@heshan0131 update with your feedback. Waiting on uber/nebula.gl#260 as well for better interaction handler

heshan0131 · 2019-07-22T17:38:08Z

Selection works after yarn installed latest react-map-gl-draw

Selected polygon should show solid line instead of dash

heshan0131 · 2019-07-22T17:39:15Z

After drawing polygon, select rectangle, draw and drag doesn't create rectangle

heshan0131 · 2019-07-22T17:42:14Z

The mouse cursor should be crosshair in draw mode. In selection mode, cursor should be pointer, upon selecting a feature, the cursor should change to move, and back to pointer when a corner is hovered

macrigiuseppe · 2019-07-23T04:19:55Z

Selection works after yarn installed latest react-map-gl-draw

Selected polygon should show solid line instead of dash

@heshan0131

Polygons are turned solid when selected

macrigiuseppe · 2019-07-23T04:38:14Z

crosshair

@heshan0131 Updated cursor pointers

heshan0131 · 2019-12-05T21:59:37Z

src/components/common/icons/base.js

@@ -51,15 +51,18 @@ export default class Base extends Component {
    width: null,
    viewBox: '0 0 64 64',
    predefinedClassName: '',
-    className: ''
+    className: '',


setting fill: 'currentColor'in defaultProps means if user passes in a style object, it will override default style (instead of merging with it), not ideal. use style.fill = 'currentColor'; in render simply adds it to user defined value

The problem with style.fill = 'currentColor' is that i cannot override the default value;
I need a way to override the fill property.

we can do something like

style = {} ... } = this.props; style.fill = style.fill || 'currentColor'

or I can create a helper to merge defaults values when no properties exist in the original object

heshan0131 · 2019-12-05T22:02:08Z

src/components/common/icons/rectangle.js

+
+  static defaultProps = {
+    height: '16px',
+    predefinedClassName: 'data-ex-icons-polygon',


'data-ex-icons-rectangle'

heshan0131 · 2019-12-05T22:03:01Z

src/components/common/styled-components.js

-  .mapboxgl-map .mapboxgl-missing-css {
-    display: none;
+  .mapboxgl-map {
+    z-index: -1;


why z-index: -1; here?

this is due to an issue with deckgl where deckgl capture mouse click events despite of other existing layers and their z-index value. So I have to make sure deckgl and its container mapbox-gl has -1 when in editor mode in order to be able to use the mouse to draw polygons.

When in regular mode we have the following:
mapboxgl: -1
deckgl: 0
editor: -1

In editor mode:
mapboxgl: -1
deckgl: -1
editor: 0

heshan0131 · 2019-12-05T22:04:16Z

src/components/common/toolbar.js

+`;
+
+const Toolbar = React.memo(({children, className, show, direction = 'row'}) => (
+  <StyledPanelDropdown className={`${className || ''} save-export-dropdown`} show={show} direction={direction}>


same here about using classnames

heshan0131 · 2019-12-05T23:39:52Z

src/components/map-container.js

+              onSelect={uiStateActions.setSelectedFeature}
+              onUpdate={visStateActions.setFeatures}
+              style={{zIndex: isEdit ? 0 : -1}}
+              onToggleFeatureLayer={visStateActions.toggleFeatureLayer}


I can't find toggleFeatureLayer is not defined in vis-state-actions, which causes this error:

oh i guess it's in geo-operation 2 pr?

it's in geo-operation 2 branch

I removed it from this PR

heshan0131 · 2019-12-05T23:59:53Z

src/components/map-container.js

+              onDeleteFeature={uiStateActions.deleteFeature}
+              onSelect={uiStateActions.setSelectedFeature}
+              onUpdate={visStateActions.setFeatures}
+              style={{zIndex: isEdit ? 0 : -1}}


making zIndex to -1 when !isEdit cauing deckgl overlay showing on top of the draw element

changing the z-index, in this case, will not fix the issue because all layers are rendered onto a canvas and the editor is a regular html element.
In order to have the effect you describe we need to absolute position the editor container but in that case deck.gl will not receive mouse over and the tooltip will not show up

heshan0131 · 2019-12-06T00:22:34Z

src/components/map/map-control.js

+    <div style={{position: 'relative'}}>
+      {isActive ? (
+        <StyledToolBar show={isActive} direction="column">
+          <ToolbarItem


Now that clicking on polygon automatically selects it. and click outside polygon deselects it, Do we still need this to manually enter selection mode?

We need this for two reasons:

enable drawing on the map

the selection of a polygon only works when we are in editor mode. when we turn editor mode off we are not able to select polygons by clicking on it

heshan0131 · 2019-12-06T00:24:43Z

src/components/map/map-control.js

+          <ToolbarItem
+            onClick={() => onSetEditorMode(EDITOR_MODES.DRAW_RECTANGLE)}
+            label="rectangle"
+            icon={(<Rectangle height="22px"/>)}


A better way is to pass in Rectangle and render it in <ToolbarItem><props.icon height="22px"></ToolbarItem>

how would we handle all toolbarItems, each has a different icon.

Should we pass a default prop items with a list of button objects? something like this:

{ onClick: () => onSetEditorMode(EDITOR_MODES.DRAW_POLYGON), label:'polygon', icon: Rectangle, activeCondition: EDITOR_MODES.DRAW_RECTANGLE }

heshan0131 · 2019-12-06T00:27:28Z

src/components/map/map-tooltip.js

@@ -0,0 +1,112 @@
+// Copyright (c) 2019 Uber Technologies, Inc.


Is this used anywhere?

I forgot to add this to MapContainer. this will replace _renderMapPopover method

I will remove it from this diff

macrigiuseppe · 2019-12-10T19:03:45Z

@heshan0131 I found a more simple and elegant solution for z-index and mouse events.

The Draw component will be placed using absolute position over deck.gl (canvas).

Modes

Read: Draw pointer-events css property will set to 'none' so deck.gl will capture mouse events and display tooltip correctly.
Edit: Draw pointer-events css property will set to 'all', users will be able to draw and apply filters.

Signed-off-by: Giuseppe Macri <[email protected]>

* Apply Geo-Polygon filters Co-authored-by: Shan He <[email protected]> Signed-off-by: Giuseppe Macri <[email protected]>

macrigiuseppe force-pushed the 389_create_polygons branch 2 times, most recently from ae03840 to 78f3e1a Compare July 2, 2019 23:10

macrigiuseppe self-assigned this Jul 2, 2019

macrigiuseppe force-pushed the 389_create_polygons branch from 9a5d7fd to 1376d7a Compare July 8, 2019 22:03

macrigiuseppe requested a review from heshan0131 July 14, 2019 23:40

heshan0131 reviewed Jul 15, 2019

View reviewed changes

macrigiuseppe force-pushed the 389_create_polygons branch 4 times, most recently from d1289bd to 11fe20e Compare July 15, 2019 02:48

macrigiuseppe commented Jul 15, 2019

View reviewed changes

macrigiuseppe force-pushed the 389_create_polygons branch from 11fe20e to 832376a Compare July 15, 2019 03:02

macrigiuseppe changed the title ~~[WIP] 389 create polygons~~ Create polygons Jul 17, 2019

macrigiuseppe force-pushed the 389_create_polygons branch from 510d062 to 9704108 Compare July 18, 2019 04:55

heshan0131 reviewed Jul 19, 2019

View reviewed changes

macrigiuseppe force-pushed the 389_create_polygons branch from 337365e to 0864853 Compare July 22, 2019 06:07

macrigiuseppe changed the title ~~Create polygons~~ [Feat] Create polygons Jul 23, 2019

heshan0131 reviewed Dec 5, 2019

View reviewed changes

heshan0131 reviewed Dec 6, 2019

View reviewed changes

macrigiuseppe force-pushed the 389_create_polygons branch from 346d614 to 22bb2ba Compare December 6, 2019 21:57

macrigiuseppe force-pushed the 389_create_polygons branch 2 times, most recently from d6972f5 to f755b1e Compare December 10, 2019 19:16

macrigiuseppe requested a review from heshan0131 December 11, 2019 17:15

macrigiuseppe force-pushed the 389_create_polygons branch from f755b1e to efec164 Compare December 14, 2019 00:42

heshan0131 approved these changes Dec 18, 2019

View reviewed changes

macrigiuseppe force-pushed the 389_create_polygons branch from efec164 to 9d9a1b5 Compare December 26, 2019 11:34

macrigiuseppe force-pushed the 389_create_polygons branch 2 times, most recently from 7edc2c6 to cbbbab9 Compare January 14, 2020 23:14

macrigiuseppe force-pushed the 389_create_polygons branch 3 times, most recently from 4071267 to 5401267 Compare January 17, 2020 22:56

macrigiuseppe force-pushed the master branch from d1511b4 to 1bcdd2d Compare January 17, 2020 23:11

Able to draw polygons

324f51a

Signed-off-by: Giuseppe Macri <[email protected]>

macrigiuseppe force-pushed the 389_create_polygons branch from 5401267 to 324f51a Compare January 21, 2020 06:11

[Feat] Apply polygon filter - Geo-Operations 2 (#678)

ff5596e

* Apply Geo-Polygon filters Co-authored-by: Shan He <[email protected]> Signed-off-by: Giuseppe Macri <[email protected]>

macrigiuseppe force-pushed the 389_create_polygons branch from 15e77fe to ff5596e Compare January 21, 2020 06:38

macrigiuseppe merged commit 3a01a4f into master Jan 21, 2020

delete-merged-branch bot deleted the 389_create_polygons branch January 21, 2020 07:11

macrigiuseppe mentioned this pull request Feb 15, 2020

Allow drawing on map to create paths and polygons #389

Closed


		const MapLegendPanel = ({items, isActive, scale, toggleMenuPanel, isExport}) =>
		const MapLegendPanel = React.memo(({items, isActive, scale, onToggleMenuPanel, isExport}) =>

		@@ -0,0 +1,112 @@
		// Copyright (c) 2019 Uber Technologies, Inc.

[Feat] Create polygons - Geo-Operations 1 #595

[Feat] Create polygons - Geo-Operations 1 #595

Conversation

macrigiuseppe commented Jul 2, 2019 • edited Loading

Implementation details:

macrigiuseppe commented Jul 12, 2019

macrigiuseppe commented Jul 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

macrigiuseppe commented Jul 15, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

macrigiuseppe commented Jul 19, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

macrigiuseppe Jul 22, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

macrigiuseppe Jul 22, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heshan0131 Dec 6, 2019 • edited Loading

Choose a reason for hiding this comment

macrigiuseppe Dec 6, 2019 • edited Loading

Choose a reason for hiding this comment

macrigiuseppe commented Jul 22, 2019

heshan0131 commented Jul 22, 2019

heshan0131 commented Jul 22, 2019

heshan0131 commented Jul 22, 2019 • edited Loading

macrigiuseppe commented Jul 23, 2019

macrigiuseppe commented Jul 23, 2019

heshan0131 Dec 5, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

macrigiuseppe Dec 6, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heshan0131 Dec 6, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

macrigiuseppe commented Dec 10, 2019 • edited Loading

macrigiuseppe commented Jul 2, 2019 •

edited

Loading

macrigiuseppe Jul 22, 2019 •

edited

Loading

macrigiuseppe Jul 22, 2019 •

edited

Loading

heshan0131 Dec 6, 2019 •

edited

Loading

macrigiuseppe Dec 6, 2019 •

edited

Loading

heshan0131 commented Jul 22, 2019 •

edited

Loading

heshan0131 Dec 5, 2019 •

edited

Loading

macrigiuseppe Dec 6, 2019 •

edited

Loading

heshan0131 Dec 6, 2019 •

edited

Loading

macrigiuseppe commented Dec 10, 2019 •

edited

Loading