-
-
Notifications
You must be signed in to change notification settings - Fork 170
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
Add support for Matomo user commands #191
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your effort! I just had a few comments, then I think this is ready for merging.
for var_code in ['_paq.push(["setIgnoreClasses", "no-tracking"]);', | ||
'_paq.push(["setDocumentTitle", "foobar"]);', | ||
'_paq.push(["tackGoal", 1, 1000.01]);']: | ||
assert var_code in r |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please split this up into 3 separate assert statements. Then the actual failing test will be easier to spot. Thank you!
'matomo_commands': [['setIgnoreClasses', 'no-tracking'], | ||
['setDocumentTitle', 'foobar'], | ||
['tackGoal', 1, 1000.01]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe like this?
'matomo_commands': [['setIgnoreClasses', 'no-tracking'], | |
['setDocumentTitle', 'foobar'], | |
['tackGoal', 1, 1000.01]] | |
'matomo_commands': [ | |
['setIgnoreClasses', 'no-tracking'], | |
['setDocumentTitle', 'foobar'], | |
['tackGoal', 1, 1000.01], | |
] |
I actually have this in my PR to fix #141 but for some reason git isn't letting me push. You would also, if you wanted to bind events that push matomo events to
|
@bittner Thanks for looking at my PR. Looks like #193 implements this in more generic way. So I'll close this PR. |
Yeah, I just need to get the points Bitner hit before I can merge. If you want, you can just take the code and install the package locally then modify it to use my PR. Until I have to time to hit all Bitners points, I can't work on this. |
Yeah, I just need to get the points Bitner hit before I can merge and I'll add in the docs before pushing next. If you want, you can just take the code and install the package locally then modify it to use my PR. Until I have to time to hit all Bitners points, I can't work on this. |
Ability to push custom commands to Matomo (#190)
Might be a work around for #141