mapsplugin / cordova-plugin-googlemaps

Google Maps plugin for Cordova
Apache License 2.0
1.66k stars 918 forks source link

A way to hide _pluginDOMid reference number attributes in debugger #2745

Closed tryhardest closed 4 years ago

tryhardest commented 4 years ago

Perhaps there is a way to do this in the debugger I am unaware of, (I have searched) or a boolean that we can add to the plugin to toggle on/off the _pluginDOMid reference numbers.

I understand the logic of why they exist but for the debugging process, having these added visually to so many div elements in the DOM creates a really noise space.

Just an idea as debugging is already an inherently noisy endeavor and no one on our team has ever dug into the ref number of one of these. It might just make debugging a little cleaner.

wf9a5m75 commented 4 years ago

Unfortunately I have no enough time to improve this plugin recently.

tryhardest commented 4 years ago

Maybe worth putting an open call to other maintainers?

tryhardest commented 4 years ago

I say that respectfully with all the work you have put into this. But also in that many ppl critically rely on the plugin and sometimes (most times) constant support and maintenance is needed.

When the refactor discussion turned a bit sour, it was also a catalyst moment for this plugins partial rewrite.

But despite the contention in that discussion, both guys were willing to get involved and that's a good thing. Sometimes and I think in that scenario, good intention can (and was) lost do to unnecessary added critical plugin commentary (although I didn't mind it) but with all the work you do, I can see how you did.

Regardless, my intention here was a easy suggestion but your totally understandable response makes me think now might be the good time to find additional torch bearers of your incredibly great work so it can be carried on and supported with the love and attention it desirves and needs (especially as critical as it is to so many).

wf9a5m75 commented 4 years ago

Hiding the plugindomid is not so easy. I need to recreate lots of things. And I have never met the person who fully understand of this plugin all the code.

Unfortunately I have no enough time to make a big change.

If you want to do that, please check the code. All code is open. Then please send a pull request. That's the open source community

tryhardest commented 4 years ago

Let's discuss and maybe yes.

What are you thoughts on doing it? You seem to have considered it (as you refer to it as a large task).

Curious what your approach would be. I can dig in a bit more.

tryhardest commented 4 years ago

What about during a reference event check (by map) for required DOM El changes (so it knows position), everything with a pluginDOMId broadcast to a JSON reference on change, and map instead refers to that?

wf9a5m75 commented 4 years ago

Thank you for trying. I don't touch the code recently, so let me allow small mistakes.

The purpose of the plugindomid is to catch the hierarchy of HTML elements. This plugin inserts a transparent layer over the webview, then detect which is clicked, map or HTML element. In order to detect, the plugin must know the hierarchy of all HTML elements.

If a HTML element change, but it might be a parent element of something. So, it's kind of virtual Dom tree.

Without the plugindomid, the plugin needs to trace all HTML elements when any elements are changed.

Normal virtual Dom approach uses two trees, but this plugin doesn't use it. So, it needs to change to that. Compare before and after, then need to track which ones are changed.

tryhardest commented 4 years ago

Hmmm. Good breakdown. I'd hate to introduce a virtualDOM for this for all those (like us) who have done everything in their power to avoid using a virtual DOM. Lot of reasons why.

Everything is best on the vanilla DOM, although Tagged Template String Literals (HyperHTML or LitHTML) might work as a worst case. I'll consider a bit but we def would hope to avoid introducing a virtualDOM if avoidable.

CodeWithOz commented 4 years ago

Hi @wf9a5m75 thanks so much for your good work. I noticed this commit may have been the result of this conversation, but I just upgraded to the latest commit from the multiple_maps branch and I get this error in the console:

js_CordovaGoogleMaps-for-android_ios.js:530 Uncaught (in promise) TypeError: Cannot redefine property: __pluginDomId
    at Function.defineProperty (<anonymous>)
    at js_CordovaGoogleMaps-for-android_ios.js:530
    at Array.forEach (<anonymous>)
    at CordovaGoogleMaps.removeDomTree (js_CordovaGoogleMaps-for-android_ios.js:521)
    at js_CordovaGoogleMaps-for-android_ios.js:83
    at Array.forEach (<anonymous>)
    at js_CordovaGoogleMaps-for-android_ios.js:78
    at Array.forEach (<anonymous>)
    at js_CordovaGoogleMaps-for-android_ios.js:63

Some of the styling in our app has also been messed up. I haven't been able to do a simple reproduction yet, but I thought maybe the error message could offer some suggestions. Thanks!

wf9a5m75 commented 4 years ago

@CodeWithOz Thank you for the report. Could you share an example project files that I can reproduce the situation?