Feature: Proposed new implementation of DrawingManager, discovered issues with Shapes

[davydnorris]

Is your feature request related to a problem? Please describe.

I submitted the original Drawing Manager component, which mutated the shapes passed down into it. As this was an anti-pattern, it was refactored but this has quite badly broken its functionality. After looking at how the original Drawing Manager could be properly refactored, I have decided that it would be better to make it a simple wrapper and instead move the responsibility for actually managing the shapes up to the parent

Describe the solution you'd like

I propose that the Drawing Manager no longer actively manages the shapes, and simply wraps the Google Drawing Manager. The component would be set up the same way, and would still use <shape>Options and position props, but the shapes prop would be removed. The default slot would still be set up to allow you to provide your own drawing toolbar as it does now, but the deleteSelection event handler would be removed, leaving just the setDrawingMode handler. Instead of managing all the shapes internally, it would emit a created:<shape> event inside the overlaycomplete handler, and it would pass the key properties of the newly created shape up to the parent as payload in a way that could be used directly as parameters to the various gmap shapes controls. The parent would be responsible for the actual drawing of these.

Describe alternatives you've considered

I have looked at the use of a local array to maintain the shapes, however the effort to deep copy the prop data is significant and adds little value. In addition you would then have to set up deep watchers for every individual element of the shapes prop, and well as the prop array as a whole in order to detect additions and removals. And right now the prop doesn't even accept the gmap shape types.

Far better to simplify and let the parent deal with the shapes completely.

Additional context

If the parent is going to deal with the shapes, then it will manage when a shape is selected, and when it is editable. This led me to uncover an issue with all the current shape components - none of them correctly handle being put into edit mode. Each shape will need to be set up so that it will correctly update its props if it gets edited in edit mode.

[diegoazh]

@davydnorris I liked the suggestion, I'm not sure but I suppose that you are currently working on a PR, is that right? If the answer is yes I wait for it and focus on the new v4.0.0 for Vue 3 version, otherwise the feature will need to wait until I finish that work.

I hope that you are glad to contribute to the project again and fix this weird behaviour on the drawing manager component and its shapes.

Thank you and Regards.

[create-issue-branch[bot]]

Branch issue-265-Feature_Proposed_new_implementation_of_DrawingManager_discovered_issues_with_Shapes created!

[davydnorris]

Yep happy to do this. I'm currently looking at all the shapes and I think there's a problem with mutation in many because the positioning and size props are all arrays or objects, so they'll need to change.

Also I am looking at the clickable and draggable props - we should probably only attach the appropriate event handlers if these are true. There's a lot of initialiser code going on and this may be related to the performance issues for markers

[diegoazh]

Hi, @davydnorris are you working on this?

[davydnorris]

@diegoazh yes - the Drawing Manager is finished but there were some issues with shapes that needed to be fixed. There are two things I think we need to do:

• make a generic Shape component that has a shapeType prop
• change the editable and draggable prop actions to bind and unbind the appropriate events

I am also going to set the edit event emits so that they do not fire if you are dragging instead of editing. Right now both fire if you drag a shape, and this results in an event storm, especially for polygons with lots of points.

I have also found that the changes made to the initialisation code compared to the original have caused quite a bit of performance degradation, and have also made it a lot harder to maintain and understand. My vote would be to go back to the original code

[diegoazh]

@davydnorris great job, let me know when you finish with the refactor.

About the second problem, performance on the initialization, we should discuss it in a different issue, I would like to analyse it to detect where is the performance issue, and discuss which is the better approach to fix it.

Regards.

[davydnorris]

@diegoazh hey Diego - had a chance to work some more on this and I've hit a bit of a problem...

One of the things I have found is that when isDraggable is turned on, as well as the drag events being emitted, the shape components emit a storm of update events when you drag the shapes around. This is especially bad for polylines and polygons.

Ideally you really only want the update event to fire at drag end, which means adding some code to the change events - but I've discovered that these are created automatically within the bindProps code. I'll need to examine the props and work out which ones really should be automatically bound and which shouldn't

[diegoazh]

Maintenance for Vue v2 is stopped. The new version for Vue v3 is landed @gmap-vue/v3 please try it.