alex3165 / react-leaflet-draw

React component for leaflet-draw on top of react-leaflet
228 stars 152 forks source link

Duplicated polygons onCreated when using them on State #50

Open galmeida12 opened 6 years ago

galmeida12 commented 6 years ago

Hi there,

I'm having trouble trying to create new polygons. The way that I'm using this library in conjunction with react-leaflet is that I save all the polygons in the component's state, with some additional info for a pop-up with text, when the area is clicked. I show all those polygons in the respective layer in use, where the is also used.

The unwanted behavior is that upon creating a new shape, it duplicates the new shape, rendering the shape that was saved in the state, and also the shape created with the draw:created.

I could refresh the page and only show the polygons contained in the state but that is plain stupid, or I could leave the new shapes out the state, but that way I'm not able to inject the popup with the text.

I'm trying to remove the polygon that is automatically created, so that only the one that I copy to the state is shown, but with no luck so far.

I'll leave here the code of the component in question, which is still "in construction" and using demo values, for reference.

Any help is much appreciated. Thank you

import React, {Component} from 'react'
import PropTypes from 'prop-types'
import {withStyles} from 'material-ui/styles'
import TextField from 'material-ui/TextField'
import Typography from 'material-ui/Typography'
import {Map, TileLayer, Marker, Popup, LayersControl, Polygon, FeatureGroup} from 'react-leaflet'
import L from 'leaflet'
import {EditControl} from 'react-leaflet-draw'
import update from 'react-addons-update'

const styles = theme => ({
  container: {
    display: 'flex',
    flexWrap: 'wrap'
  },
  textField: {
    marginLeft: theme.spacing.unit,
    marginRight: theme.spacing.unit,
    width: '200px',
    fontSize: ''
  },
  menu: {
    width: 200
  }
})

const mapData = {
  name: 'My Working Place',
  location: {
    lat: 37.050300,
    lng: -7.881713
  },
  areas: [{
    id: 1,
    name: 'My working area 1',
    crop: 'Citrinos',
    area: 1025,
    points: [
      [37.050698940024866, -7.882744073867799],
      [37.04947874365056, -7.8816014528274545],
      [37.050163768591695, -7.880088686943055],
      [37.050942976945635, -7.882298827171327],
      [37.050698940024866, -7.882744073867799]
    ]
  },
  {
    id: 2,
    name: 'My working area 2',
    crop: 'Citrinos',
    area: 1025,
    points: [
      [37.04626760682191, -7.8815263509750375],
      [37.04565961628692, -7.8798043727874765],
      [37.04571527759473, -7.879697084426881],
      [37.04739794247914, -7.879133820533753],
      [37.04763342695574, -7.879659533500672],
      [37.04754779632151, -7.879825830459596],
      [37.04766767918239, -7.8803032636642465],
      [37.04626760682191, -7.8815263509750375]
    ]
  }
  ]

}

class RegistrationMap extends Component {
  constructor (props) {
    super(props)
    this.state = {
      zoom: 15,
      lat: mapData.location.lat,
      lng: mapData.location.lng,
      areas: mapData.areas
    }
  }

  /** > Create polygon - Calculate area and add polygon to state -------------------------------------------------- */
  _onCreate = (data) => {
    let latlngs = []
    let points = []
    let nextID = this.state.areas[this.state.areas.length - 1].id + 1
    data.layer._latlngs.map((a) => {
      latlngs.push(a)
      a.map((coordinate) => {
        points.push([coordinate.lat, coordinate.lng])
      })
    })
    let newShape = {
      'id': nextID,
      'name': '',
      'crop': '',
      'area': this._shapeArea(latlngs[0]),
      'points': points
    }
    let newAreas = update(this.state.areas, {$push: [newShape]})
    let layer = L.geoJson()
    layer.clearLayers()
    this.setState({areas: newAreas})
    console.log('onCreated', data)
    console.log('state: ', this.state)
  }
  /** > Change path - Calculate new area and update points in state ----------------------------------------------- */
  _onEditPath = (data) => {
    let areas = this.state.areas
    for (var layer in data.layers._layers) {
      console.log('edit path latlng', data.layers)
      let latlngs = []
      let points = []
      data.layers._layers[layer]._latlngs.map((a) => {
        latlngs.push(a)
        a.map((coordinate) => {
          points.push([coordinate.lat, coordinate.lng])
        })
      })
      let id = data.layers._layers[layer].options.name
      areas.find(x => x.id === id).points = points
      areas.find(x => x.id === id).area = this._shapeArea(latlngs[0])
      this.setState({areas: areas})
    }
  }

  _onEdit = (data) => {
    console.log('onEdit', data)
    console.log('state: ', this.state)
  }

  _onDeleted = (data) => {
    console.log('onDeleted', data)
    console.log('state: ', this.state)
  }
  /** > Convert an array of pair of points into an array of LatLngs ----------------------------------------------- */
  _toLatLngs = (dataPoints) => {
    let latlngs = []
    dataPoints.map((point) => {
      latlngs.push(L.latLng(point))
    })
    return (latlngs)
  }
  /** > Calculate AREA from an array of LatLng points ------------------------------------------------------------- */
  _shapeArea = (latlngs) => {
    let a = L.GeometryUtil.geodesicArea(latlngs)
    return (L.GeometryUtil.readableArea(a, true, 10))
  }

  _handleTextChange = name => e => {
    let areas = this.state.areas
    areas.find(x => x.id === e.target.form.id)[name] = e.target.value
    this.setState({areas: areas})
  }

  render () {
    const {classes} = this.props
    const position = [this.state.lat, this.state.lng]
    const osmUrl = 'https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png'
    const googleSat = 'http://{s}.google.com/vt/lyrs=s&x={x}&y={y}&z={z}'
    const googleSubdomains = ['mt0', 'mt1', 'mt2', 'mt3']
    const mapHeight = {height: '300px'}

    console.log('MAP STATE: ', this.state)
    let definedAreas = this.state.areas.map((a) => {
      let latlngs = this._toLatLngs(a.points)
      a.area = this._shapeArea(latlngs)
      return (
        <Polygon positions={a.points} name={a.id}>
          <Popup style={{width: '200'}}>
            <form id={a.id} className={this.props.container} noValidate autoComplete='off'>
              {this.props.isAdmin
                ? <div>
                  <TextField
                    id='name'
                    label='Sector'
                    className={this.props.textField}
                    value={a.name}
                    onChange={this._handleTextChange('name')}
                    margin='normal'
                    style={{fontSize: '1.5rem'}}
                  />
                  <TextField
                    id='crop'
                    label='Cultura'
                    className={this.props.textField}
                    value={a.crop}
                    onChange={this._handleTextChange('crop')}
                    margin='normal'
                    style={{fontSize: '1.5rem'}}
                  />
                </div>
                : <div>
                  <Typography variant='subheading'>
                    Sector : {a.name}
                  </Typography>
                  <Typography variant='subheading'>
                    Cultura : {a.crop}
                  </Typography>
                </div>}
              <Typography variant='subheading'>
                Area : {a.area}
              </Typography>
            </form>
          </Popup>
        </Polygon>
      )
    })

    return (
      <Map center={position} zoom={this.state.zoom} style={mapHeight} onMeasureFinish={this.handleMeasureFinish}>
        <LayersControl position='topright'>
          <LayersControl.BaseLayer name='OpenStreet Map'>
            <TileLayer
              url={osmUrl}
              attribution='&copy <a href=&quot;http://osm.org/copyright&quot;>OpenStreetMap</a> contributors'
              maxZoom={22}
            />
          </LayersControl.BaseLayer>
          <LayersControl.BaseLayer checked name='Google Satellite'>
            <TileLayer
              url={googleSat}
              subdomains={googleSubdomains}
              maxZoom='22'
            />
          </LayersControl.BaseLayer>
          <LayersControl.Overlay checked name='Areas'>
            <FeatureGroup>
              { this.props.isAdmin
                ? <EditControl
                  position='topleft'
                  onEdited={(e) => this._onEditPath(e)}
                  onCreated={(e) => this._onCreate(e)}
                  onDeleted={(e) => this._onDeleted(e)}
                  onEditVertex={(e) => this._onEdit(e)}
                  draw={{
                    rectangle: false,
                    polyline: false,
                    circle: false,
                    marker: false,
                    circlemarker: false,
                    polygon: {
                      allowIntersection: false,
                      showArea: true
                    }
                  }}
                /> : null}
              {definedAreas}
            </FeatureGroup>
          </LayersControl.Overlay>
        </LayersControl>
        <Marker position={position}>
          <Popup>
            <span>
              {this.props.building.name}
            </span>
          </Popup>
        </Marker>
      </Map>
    )
  }
}

RegistrationMap.propTypes = {
  classes: PropTypes.object.isRequired
}

export default withStyles(styles)(RegistrationMap)
madebydavid commented 5 years ago

I had a similar issue to this - I think it is because the newly created shape is persisted inside the state of the <Map> itself whereas preloaded shapes exist within the state of our own component. When we get an onCreated event, we update our state, and then and add a <Polygon> - a duplicate is therefore created.

A "fix" I used was to edit the key property of the <Map> based on if our state contains a shape - this forces a new instance of the component to be made and therefore any internal state is lost. I am using the map to only edit a single shape at a time - so my use-case is more straightforward than yours I think @gpiri - you may have to do something more complex (although you have probably solved this / worked around this by now).

The code looks similar to this - I am storing the shape in this.state.coordinates:

const key = this.state.coordinates ? 'map-populated' : 'map-empty';

return (
  <Map {...this.getMapOptions()} key={key}>

    {this.getTileLayer()}

    <FeatureGroup>

      <EditControl
        position='topright'
        onEdited={::this.handlePathEdited}
        onCreated={::this.handlePathCreated}
        onDeleted={::this.handlePathDeleted}
        draw={this.getDrawOptions()} />

      { this.state.coordinates ?
        <Polygon positions={this.state.coordinates} /> : null
      }

    </FeatureGroup>

  </Map>
)
gordonwoodhull commented 5 years ago

Thanks @madebydavid, this helped a lot.

Possibly more efficient: I found that updating a key on the FeatureGroup after onCreated was sufficient to rerender the shapes.

maximesahagian commented 5 years ago

Same issue with multiple points.

@madebydavid, Your solution is only working for single point (and it's updating map center and bounds when creating the first element).

Did you find a way to solve it @gpiri ?

@alex3165 Can you check this issue? Sounds weird.

madebydavid commented 5 years ago

@maximesahagian - there was a great suggestion by @gordonwoodhull - try setting the key on the feature group instead?

const key = this.state.coordinates ? 'map-populated' : 'map-empty';

return (
  <Map {...this.getMapOptions()}>

    {this.getTileLayer()}

    <FeatureGroup key={key}>

      <EditControl
        position='topright'
        onEdited={::this.handlePathEdited}
        onCreated={::this.handlePathCreated}
        onDeleted={::this.handlePathDeleted}
        draw={this.getDrawOptions()} />

      { this.state.coordinates ?
        <Polygon positions={this.state.coordinates} /> : null
      }

    </FeatureGroup>

  </Map>
)
gordonwoodhull commented 5 years ago

@maximesahagian, I keep a counter for my FeatureGroup key and I increment it each time a shape is added. Since we want to remove the temporary shapes and replace them with react components, I think this is minimal and correct.

FWIW I also put keys on my shapes (<Polygon /> etc.); otherwise remove doesn’t seem to work right.

maximesahagian commented 5 years ago

Hello @gordonwoodhull, alright, this fix works on FeatureGroup to only create state draws.

Remove works before creating a new shape, so I need to delete them from the state onDelete, how to get the index(s) deleted to delete them from the state?

Thanks :)

andrewdodd commented 5 years ago

I don't really feel good about leaving this comment (as I don't really want to "scoop" this repo, and I think it is a good lib), but I made a repo (https://github.com/andrewdodd/react-leaflet-draw) to show a possible different implementation that I think solves this issue.

If you run the example-full locally (i.e. git clone....yarn run example-full) you should see the behaviour you are after. The code in the example-full/edit-control.js shows managing a set of objects in state; dealing with adding, deleting and editing; and triggering on the state changes of the draw plugin.

maximesahagian commented 5 years ago

Hello @andrewdodd,

I've seen this repo yesterday, but I had some dependencies issues into the EditControlFeatureGroup, I'm sure we can find a way to handle shapes in the state easier..

andrewdodd commented 5 years ago

@maximesahagian cool. As I mention in my repo...it might just be easier to copy and adjust the EditControl from this library to suit your specific needs (that's what I did), rather than using this dep as is.

galmeida12 commented 5 years ago

@madebydavid Thank you so much for your replies. The method with keys helps with this bug. I since stopped working on this project so I haven't tried @gordonwoodhull 's solution of changing the key in the FeatureGroup instead.

gordonwoodhull commented 5 years ago

@maximesahagian, good point, I am keeping a map from layer._leaflet_id to my own IDs. I build this map in componentDidMount/Update (using Refs on the shapes components).

Kind of horrible but it works so I’m leaving it that way for now. Next time I run into trouble I will definitely check out the @andrewdodd approach.

It’s always a pain dealing with one component system wrapping another one. I’m just happy I haven’t run into any absolute blockers on my React journey...

maximesahagian commented 5 years ago

I've juste installed the @andrewdodd dependency, and it's working well :)