tracking consent #56
Open
tracking consent #56
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mraveux
force-pushed
the
matheo/tracking-consent
branch
4 times, most recently
from
32c871b
to
ae4a3d7
Compare
mraveux
force-pushed
the
matheo/tracking-consent
branch
2 times, most recently
from
709a7a4
to
7bc1ad2
Compare
sisou
requested changes
Jul 27, 2020
| type: Boolean, | ||
| default: false, | ||
| }) | ||
| public ignoreDoNotTrack: boolean; |
There was a problem hiding this comment.
Is it a good idea to make this a setting in the first place? Shouldn't we respect this setting?
There was a problem hiding this comment.
We do respect this setting by default. But if someone want to not respect it, then he's free to do so. (see Daniel's comment #56 (comment))
src/components/TrackingConsent.vue
Outdated
Comment on lines
212
to
230
| const cookie = [cookieName + '=' + cookieValue]; | ||
|
|
||
| if (options) { | ||
| if (!options.domain) options.domain = TrackingConsent.DEFAULT_COOKIE_DOMAIN; | ||
| if (!options.expirationDays) options.expirationDays = TrackingConsent.DEFAULT_COOKIE_EXPIRATION_DAYS; | ||
|
|
||
| cookie.push('domain=' + options.domain); | ||
| cookie.push('max-age=' + options.expirationDays * 24 * 60 * 60); | ||
| } | ||
|
|
||
| cookie.push('path=/'); | ||
| document.cookie = cookie.join(';'); |
There was a problem hiding this comment.
I believe this needs the sameSite setting as well nowadays.
There was a problem hiding this comment.
SameSite require Secure sometimes, so i'll add both 👍
Comment on lines
+392
to
+407
| export const LOCALSTORAGE_KEYS = [ | ||
| 'tracking-consent', | ||
| 'tracking-consensus', | ||
| ]; |
There was a problem hiding this comment.
Why two keys? Maybe add a comment to explain.
src/components/TrackingConsent.vue
Outdated
| export const COOKIE_STORAGE_KEY = 'tracking-consent'; | ||
|
|
||
| export const DEFAULT_COOKIE_DOMAIN = document.location.hostname; | ||
| export const DEFAULT_COOKIE_EXPIRATION_DAYS = 365 * 20; |
There was a problem hiding this comment.
Actually let me change it to 10 years, because 20 years won't work on 32bits systems.
mraveux
added a commit
that referenced
this pull request
Jul 28, 2020
Requested changes by Sisou #56 (review) - `options.setSiteId` doesn’t have a default value anymore, and is now required - Renamed `DEFAULT_TRACKING_URL` to `DEFAULT_TRACKING_SCRIPT_URL` - Moved the default `geoIpServer` value to a constant named `DEFAULT_GEOIP_SERVER_URL` - Added possibility to set the cookie's attributes `secure` and `sameSite` - Added a comment explaining the two `localStorage` keys - Changed default lifetime of the cookie from 20 years to 10 years since 20 years would not work on 32bits systems. (the cookie would just disappear instantly)
mraveux
added a commit
that referenced
this pull request
Jul 28, 2020
PR #56 Requested changes by Sisou #56 (review) - `options.setSiteId` doesn’t have a default value anymore, and is now required - Renamed `DEFAULT_TRACKING_URL` to `DEFAULT_TRACKING_SCRIPT_URL` - Moved the default `geoIpServer` value to a constant named `DEFAULT_GEOIP_SERVER_URL` - Added possibility to set the cookie's attributes `secure` and `sameSite` - Added a comment explaining the two `localStorage` keys - Changed default lifetime of the cookie from 20 years to 10 years since 20 years would not work on 32bits systems. (the cookie would just disappear instantly)
mraveux
force-pushed
the
matheo/tracking-consent
branch
from
8f88ad4
to
397ddb7
Compare
- turn public methods into static ones - turn "domain" prop into static const
- PR requested changes - Listening for tab / window focus in case the user allows tracking on another tab / window or change browser settings / vpn ToDo: - Should use BottomOverlay - Component doc comment
- types fix - setTrackerUrl & trackingScriptUrl urls fix - add default setTrackerUrl value - now using a window `focus` listener instead of document `visibilitychange` one
…null and string[] as options value’s type
PR #56 Requested changes by Sisou #56 (review) - `options.setSiteId` doesn’t have a default value anymore, and is now required - Renamed `DEFAULT_TRACKING_URL` to `DEFAULT_TRACKING_SCRIPT_URL` - Moved the default `geoIpServer` value to a constant named `DEFAULT_GEOIP_SERVER_URL` - Added possibility to set the cookie's attributes `secure` and `sameSite` - Added a comment explaining the two `localStorage` keys - Changed default lifetime of the cookie from 20 years to 10 years since 20 years would not work on 32bits systems. (the cookie would just disappear instantly)
mraveux
force-pushed
the
matheo/tracking-consent
branch
from
397ddb7
to
05cefe2
Compare
- now loading Matomo tracking script before checking for UI requirement, but disable cookie usage before user give consent (if in EEA) - removed `allowBrowserData` since we’re tracking browser data by default, without asking for consent - fixed `geoIpServer` @prop default value - now checking if `_paq` & `_mtm` are objects that have a `push` method, instead of checking if they’re Arrays (since they’re not Arrays, but objects with a single `push` method) - removed the public static getter `allowsBrowserData` and the public static method `allowBrowserData ` - added a static public getter `isOutsideEEA`, since we can enable Cookies without consent for countries outside the EEA - turned the `execFunction` method into a async one, allowing to `await` for the function result
- add a `trackPageView` public static method for easy page tracking on single-page apps - set trackPageView by default to track first page view
mraveux
force-pushed
the
matheo/tracking-consent
branch
from
13fefd4
to
05af0f1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This Pr add the TrackingConsent component, used to setup matomo tracking easily.
You can try it locally in the wallet, branch
matheo/matomo_tracking, or using "Curd's Fancy Deployment Selector".