Closed zmagyar closed 9 years ago
Hi @zmagyar ,
Thanks for the pull request. I merged it and also added the updated android dependency required for the enableAdvertisingIdCollection within Android. One change I made from in your PR was renaming the function name to enableAdvertisingIdCollection from setIDFAEnabled so that it is a little more descriptive. Its kind of a long name but oh well. :) Thanks a lot for your changes and if you can think of any other improvements for the plugin, let me know.
Thanks!
-Craig
Hi @cmackay,
Thanks for the merge. Now I can switch our project to use your repo again instead of the fork. :)
What I was thinking about is to add a separate function to set the UID param but then decided to set it using the already existing functions. If our marketing team comes up with more requests what can not be achieved without the change of the plugin I will raise it here before start the fork.
Currently I'm investigating the possibility of GTM integration. I couldn't spot any plugin for that and I think that is out of the scope of this plugin. What do you think about that?
Cheers, Zoltan
Hi Zoltan,
Yeah, if there are additional changes needed let me know or feel free to submit PRs.
I have seen the tag manager and briefly looked at in the past but I wasn't too familiar with it. After reading your message today, I watched a few overview videos and it seems like it could be pretty powerful. I think it could replace a lot of the functionality the plugin currently provides since it appears that all the analytics events can be passed through the tag manager. I think there could be a lot of interesting hooks added to apps to trigger tags. I use ionic a good bit with cordova and the ui-router state change events seems like a good possible trigger for tags. I am definitely interested in integration with the tag manager. If you would like to work together on this I am happy to work on it also or if you would like to submit a PR that works also. :) Thanks a lot for your feedback!
-Craig
Hi Craig,
Yes, GTM can supersede some of the reporting bits. However I'm rather looking into GTM to support our AB testing plans. If we can design a consistent js API to access the features I think I can help implementing the iOS part.
Zoltan
Hi Zoltan,
That would be great if you want to do an iOS implementation. I have a lot more experience with Android than iOS. Much of the iOS work for this plugin I made by looking at how other core cordova modules were implemented. :) If you ever see anything in the iOS that looks odd or non-standard, please feel free to submit updates. It would be great to have someone with iOS expertise helping on the project :) I am happy to help in any way with getting this implemented. If you ever want to do a google hangout to discuss or if you want to just do a first pass on your own thats fine also. Whatever you prefer. Thanks!
-Craig
Hi Craig,
I have started GTM additions on my fork. Can you review it so we can discuss it. Then I can do a pull request if you are ok with this.
Cheers, Z
Hi Zoltan,
Great. Thanks! I had checked out your master. The changes look great. One thing I noticed is that the cordova.define call in the analytics.js is causing a module already defined error. I think that is because it is already defined with the plugin.xml maybe. I am kind of new to the tag manager so I am attempting to see it work. I setup a tag manager acct got my id and configured it. It looks to connect successfully. I have tried adding different macro types to see if I can get a config value to show but I may not be doing it correctly. Do you know how you set one a value that can be read by the getConfigXX methods? I appreciate the updates. If you want to submit a PR, I can add the required changes for the android tag manager functionality.
Thanks,
-Craig
Hi Zoltan,
I just created a branch called gtm. If you want to submit a pull request to that branch that would be great. I'll then make the android changes in there.
Thanks,
-Craig
A few improvements: