Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drawing Manager and Indoor Manager #70

Open
dandical opened this issue Oct 26, 2020 · 12 comments · May be fixed by #93
Open

Drawing Manager and Indoor Manager #70

dandical opened this issue Oct 26, 2020 · 12 comments · May be fixed by #93
Assignees
Labels
enhancement New feature or request

Comments

@dandical
Copy link

dandical commented Oct 26, 2020

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
Copy link
Contributor

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.

@psrednicki psrednicki added the enhancement New feature or request label Oct 26, 2020
@dandical
Copy link
Author

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
Copy link
Contributor

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

@dandical
Copy link
Author

dandical commented Oct 29, 2020

@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 psrednicki self-assigned this Oct 31, 2020
@psrednicki
Copy link
Contributor

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
Copy link
Contributor

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). Azure/react-azure-maps-playground#42

@Piyush925
Copy link

@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
Copy link
Contributor

@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
Copy link
Member

ambientlight commented Apr 12, 2021

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
Copy link
Member

@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?

@ambientlight ambientlight linked a pull request Apr 13, 2021 that will close this issue
@lcoursey
Copy link

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
Copy link

Seconding this, can it be added soon?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants