-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Rename Piwik -> Matomo in JS tracker where possible #16052
Conversation
@@ -1464,22 +1465,26 @@ if (typeof window.Piwik !== 'object') { | |||
|
|||
var content = { | |||
CONTENT_ATTR: 'data-track-content', | |||
CONTENT_CLASS: 'piwikTrackContent', | |||
CONTENT_CLASS: 'matomoTrackContent', |
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.
will need to later update docs to mention these new attributes... I don't think we actually deprecate the old ones
@@ -7121,12 +7152,12 @@ if (typeof window.Piwik !== 'object') { | |||
} | |||
|
|||
if (window | |||
&& 'object' === typeof window.piwikPluginAsyncInit | |||
&& window.piwikPluginAsyncInit.length) { | |||
&& 'object' === typeof window.matomoPluginAsyncInit |
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.
this was never really documented (only a few weeks ago) so barely any plugin would make use of it and we can simply break BC
@@ -7135,12 +7166,16 @@ if (typeof window.Piwik !== 'object') { | |||
window.piwikAsyncInit(); |
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.
we probably keep BC for piwikAsyncInit
a long time past Matomo 5 etc
// Piwik.getAsyncTrackers() would return unconfigured trackers | ||
window.Piwik.addTracker(); | ||
// Matomo.getAsyncTrackers() would return unconfigured trackers | ||
window.Matomo.addTracker(); | ||
} else { | ||
_paq = {push: function (args) { |
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.
be good to keep using _paq
I reckon.
@@ -7152,16 +7187,16 @@ if (typeof window.Piwik !== 'object') { | |||
} | |||
} | |||
|
|||
window.Piwik.trigger('PiwikInitialized', []); | |||
window.Piwik.initialized = true; | |||
window.Matomo.trigger('MatomoInitialized', []); |
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.
PiwikInitialized
was never really documented or supported until few weeks ago so we can easily break BC
@@ -3901,7 +3927,7 @@ if (typeof window.Piwik !== 'object') { | |||
*/ | |||
function getClassesRegExp(configClasses, defaultClass) { | |||
var i, | |||
classesRegExp = '(^| )(piwik[_-]' + defaultClass; | |||
classesRegExp = '(^| )(piwik[_-]' + defaultClass + '|matomo[_-]' + defaultClass; |
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.
will need to document the support of these new attributes eg matomo_download
. We won't remove support for the piwik
class names any time soon I suppose
build js |
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.
Left a comment, otherwise looks good
* @param mixed customData | ||
*/ | ||
if (typeof piwik_log !== 'function') { | ||
piwik_log = function (documentTitle, siteId, piwikUrl, customData) { | ||
piwik_log = function (documentTitle, siteId, matomoUrl, customData) { |
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.
Do we want there to be a matomo_log, matomo_ignore_classes, etc. globals?
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.
@diosmosis that's there for BC so we can't change it. I guess 0.X or 1.X versions used this piwik_log()
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.
I see 👍
refs #12420