Azure / react-azure-maps

React Wrapper for azure-maps-control
MIT License
50 stars 30 forks source link

Drawing Manager and Indoor Manager #70

Open dandical opened 3 years ago

dandical commented 3 years ago

Hi,

A project I am working in is using this wrapper but it will require azure maps drawing and indoor modules. Will the project be open to PRs with these features? I have looked into drawing and think the best way would be to make a AzureMapsDrawingManagerProvider following the same pattern so far with layers and data source components.

psrednicki commented 3 years ago

Hi @dandical Sure we are open for PR and features request. If you can add it by yourself and create PR it will be great otherwise we can add start work on it but in next week. Fell free to open PR with this feature.

dandical commented 3 years ago

No worries I will work on it. This is waiting on your latest PR to update azure-maps-control to 2.0.31 as there are some dependencies that the latest release of drawing modules preview relies on.

psrednicki commented 3 years ago

@dandical Ok I will merge this one so you can work on the latest version.

dandical commented 3 years ago

@psrednicki Thankyou. A drawing manager creates it's own datasource by default. Features can be added to this data source to provide initial drawings to the user that they can modify. My suggestion is for it to be used as follows when providing initial features in the canvas.

 <AzureMapDrawingManagerProvider options={drawingOptions}>
      <AzureMapFeature
          id={"polygonExample MapFeature"}
          type="Polygon"
          coordinates={[
               [-50, -20],
               [0, 40],
               [50, -20],
               [-50, -20],
           ]}
        />
</AzureMapDrawingManagerProvider>

To do this The AzureMapDrawingManagerProvider creates both a drawingManagerRef and dataSourceRef (the dataSourceRef is the data source created by the instantiation of the drawing manager).

The only problem i have encountered in this is the use of the useCheckRef hook in AzureMapFeature's useFeature which is equivalent to this useEffect

  useEffect(() => {
   if(dataSourceRef && featureRef)
    dataSourceRef.add(featureRef)
    return () => {
      dataSourceRef.remove(featureRef)
    }
  }, [featureRef])

I need to add the dataSourceRef to the dependency array here as the dataSource is not created by the component like it is in AzureDataSourceProvider, but is asynchronously created when the drawingManager is created when the map is ready. Any concerns with this?

EDIT: To make it easier, I submitted a draft PR with my suggestions

psrednicki commented 3 years ago

Im started work on Drawing Manager and its working, maybe not perfect but working. Still have some issues to fix, like toolbar because now its looking like this: image

@dandical So your use case is - already have some feature(point, polygon etc.) and by drawing manager you want to mutate it. Am I right? About DrawingManger - its create dataSource and Layers by itself Im think on this stage of development lets start by just add drawing tool, and then lets try to resolve your case. I will create PR with and mention it here.

psrednicki commented 3 years ago

Im fixed styles errors and created PR with my changes. @dandical you can start work form that. I also update playground for that feature(remember to use local version of lib). https://github.com/WiredSolutions/react-azure-maps-playground/pull/42

Piyush925 commented 3 years ago

@psrednicki i already implemented all mode of drawing toolbar and only edit mode is throwing error so do you have any suggestions. Error : react-azure-maps.es5.js:132 Uncaught Error: The layer '[object Object]' has not been added to the map and its rendered features cannot be retrieved. this error is coming on edit the polygons

psrednicki commented 3 years ago

@Piyush925 Nice, good job! Can you open PR with your changes? Maybe I can check it by myself. Maybe downgrade drawing tools version will fix it.

ambientlight commented 3 years ago

Error : react-azure-maps.es5.js:132 Uncaught Error: The layer '[object Object]' has not been added to the map and its rendered features cannot be retrieved. this error is coming on edit the polygons

@Piyush925, @psrednicki: this issue occurs because there is multiple versions of azure-maps-control in the target bundle. (LayerManager.prototype.getRenderedShapes has var layerId = layer instanceof Layer ? layer.getId() : layer; where instances Layer is false because the layer is constructed with azure-maps-drawing-tools's azure-maps-control's Layer)

This can be verified by running:

yarn list azure-*

I was able to get:

├─ azure-maps-control@2.0.32
└─ azure-maps-drawing-tools@0.1.6
   └─ azure-maps-control@2.0.31
✨  Done in 1.05s.

azure-maps-drawing-tools lists azure-maps-control as dependency rather then peer dependency which may result in two version of dependencies at resolution.

The fix to this issue is described in my comment to related how to use spatial layer in Azure Maps from type script

ambientlight commented 3 years ago

@psrednicki, @msasinowski:

Actually the thing that solved deduplication at bundling for me was adding azure-maps-drawing-tools to plugins.externals.exclude in rollup config (https://github.com/ambientlight/react-azure-maps/blob/4b868b6335d120b4f549f4afa0d873cb6989b6d1/rollup.config.js#L38)

I am unfamiliar with rollup, I'm checking the docs of rollup-plugin-node-externals docs and trying to make sense why externals are used for everything except azure-maps-control and azure-maps-drawing-tools, can you give me a small explanation for this?

lcoursey commented 3 years ago

Are we likely to see this PR being merged soon? Noticed it's been open for a while now. Really keen to see support for drawing tools.

aria-aghaei commented 1 month ago

Seconding this, can it be added soon?