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

Google Ads Tag #50

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Google Ads Tag #50

wants to merge 8 commits into from

Conversation

Findus23
Copy link
Member

@Findus23 Findus23 commented Aug 4, 2018

followup to #12 (many thanks to @pebosi)

TODO:

  • add AdwordsConversionTagHelp
  • check if google_conversion_format needs to be a string
  • add docs to all parameters and check if they and the validators are correct.
  • test if it works

@Findus23
Copy link
Member Author

Findus23 commented Aug 4, 2018

@peterbo, @duracelltomi
I know nothing about Adwords, but I implemented the tag explained in https://tosbourn.com/tracking-adwords-conversion-javascript/

Can you please take a look if it works and if something is broken or missing?

Many thanks!

@duracelltomi
Copy link

This is great for az initial version but with that, users can not implement dynamic remarketing yet.
To be able to do so, this tag template will need a list of user entered key-value pairs. The output should be a JSON object as shown on this page:

https://developers.google.com/adwords-remarketing-tag/implementation

var google_tag_params = { parameter1: 'abc123', parameter2: 29.99 };

@peterbo
Copy link
Contributor

peterbo commented Aug 8, 2018

LGTM - I'm not very familiar with the remarketing feature of the pixel, but it seems quite straight forward. But for the initial version, it looks good!

@Findus23
Copy link
Member Author

Findus23 commented Aug 9, 2018

@duracelltomi Thanks for the feedback.

Thankfully Matomo has a new UI element that's perfect for entering key-value pairs:

grafik

results in

[{"index":"key1","value":"val"},{"index":"morekey","value":"othervalue"},{"index":"","value":""}]

But I am not sure what exactly you mean with google_tag_params. Do you mean that I should add google_tag_params to the dictionary passed to google_trackConversion?

I'm a bit confused because it doesn't start with google_conversion as the others

@duracelltomi
Copy link

Does this key-value element also allow to add variables in the "Value" column?

For dynamic remarketing to work, people will need to add dynamic values into the data layer and this will need a variable on the UI and this variable will need to be attached the a value in this table.

The generated output is however not OK, you should add a transformation from this:

[
  {
    "index":"key1",
    "value":"val"
  },
  {
    "index":"morekey",
    "value":"othervalue"
  },
  {
    "index":"","value":""
  }
]

... into this:

{
  "key1": "val",
  "morekey": "othervalue"
}

(ignoring empty lines as well)

@Findus23
Copy link
Member Author

Does this key-value element also allow to add variables in the "Value" column?

It does now.

you should add a transformation from this:

Thanks for the reminder, I ran out of time yesterday to do it properly.

Is the way I am defining google_tag_params now correct?

@duracelltomi
Copy link

I am not sure whether google_conversion_label should include the conversion ID as well:

configObject.google_conversion_label = conversionID + '/' + parameters.get('conversionLabel');

google_tag_params should be a global variable within window

@Findus23
Copy link
Member Author

configObject.google_conversion_label = conversionID + '/' + parameters.get('conversionLabel');

Isn't that what I am doing in https://github.com/matomo-org/tag-manager/pull/50/files#diff-460e870295a18cef54721b951bbf0bbeR42?

google_tag_params should be a global variable within window

Fixed in f1ee502

@mattab
Copy link
Member

mattab commented Apr 11, 2019

Just created #159 (didn't notice this PR.) @Findus23 be great if you could finish the PR and get this merged as a few users have requested the feature 👌

@tsteur
Copy link
Member

tsteur commented Apr 11, 2019

@mattab what we would need is someone to test it which we can't really do ourselves.

@mattab
Copy link
Member

mattab commented Apr 11, 2019

Feedback:

@tsteur
Copy link
Member

tsteur commented Jun 19, 2019

@Findus23 are you still working on this?

@Findus23
Copy link
Member Author

@tsteur No, as I have no way to test it. But I can put updating Logo/Name on my todo list.

@mattab
Copy link
Member

mattab commented Nov 19, 2019

@Findus23 Be great if you can update name/logo and then we'll work on finding someone to help this new feature. It keeps getting asked from time to time. Thanks!

@Findus23 Findus23 changed the title Adwords Conversion Tag Google Ads Tag Nov 19, 2019
@Findus23
Copy link
Member Author

@mattab Done ✔️

@mattab
Copy link
Member

mattab commented Nov 25, 2019

@duracelltomi @peterbo would you be able to test this new Google Ads Tag in Matomo Tag Manager? To test you can apply this patch to your Tag Manager: https://patch-diff.githubusercontent.com/raw/matomo-org/tag-manager/pull/50.patch

Or create the 3 new files, and replace plugins/TagManager/lang/en.json by this file: https://raw.githubusercontent.com/matomo-org/tag-manager/8ee55f85b4678f8dd205d92ebca34452bdb24e60/lang/en.json

Be awesome if you can test and confirm if this new tag works for you?

@mattab mattab added this to the Current sprint milestone Nov 25, 2019
@duracelltomi
Copy link

I have a super busy week, I can not test this before next week

@mattab
Copy link
Member

mattab commented Jan 16, 2020

Hi @duracelltomi - Do you think you'd be able to test this PR in the next few days? That would be very helpful to us!

@maba4891
Copy link

I was wondering: What happened to this feature? Would be super helpful I believe (also see https://forum.matomo.org/t/google-ads-tag-in-matomo-tag-manager/40284/2)

@Findus23
Copy link
Member Author

@maba4891 Noone who knows Google Ads well did yet have time to test it extensively, so it could not yet be merged.

@Chardonneaur
Copy link
Contributor

Hey, any news dealing with this support ticket, I have a client of mine who would be super interested in having it.

@innocraft-automation innocraft-automation removed this from the Current sprint milestone Jan 19, 2023
@Chardonneaur
Copy link
Contributor

Hi guys, any news dealing with the integration of this template? there is a high demand about it.

@snake14
Copy link
Contributor

snake14 commented Mar 27, 2023

That's a good question @Chardonneaur . Do we know what happened with this feature @mattab ? This PR looks pretty old and it's set to merge into master instead of the development branch. Should we try to salvage it?

@Chardonneaur
Copy link
Contributor

there is a bing ads tracker template, normally, a Google Ads tracker shouldn't be complicated to publish, there are two additional fields I guess.

@ronan-hello
Copy link

Hi,
Google Ads tag is ready in this community plugin https://plugins.matomo.org/TagManagerExtended#preview
You can copy this template and integrate it into the TagManager?

@AltamashShaikh
Copy link
Contributor

@Stan-vw This would definitely be a good addition, @ronan-hello is there anyway you can send a PR to include this tag ?
We would be happy to merge a PR for this feature

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

Successfully merging this pull request may close these issues.

None yet