Matomo tracking #109
Merged
Matomo tracking #109
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
donni106
force-pushed
the
feature/SVA-63-matomo-tracking
branch
2 times, most recently
from
064d1d9 to
4393a1d
Compare
- moved `const namespace...` to be available in the whole file - added `matomoUrl` and `matomoSiteId` in secrets SVA-63
- added tracking for app start - added tracking for mounting home screen SVA-63
- added package `matomo-tracker-react-native` - removed Matomo files in code used before SVA-63
- added __DEV__ to globals in eslint config, to not argue about it being not defined when using it elsewhere
donni106
force-pushed
the
feature/SVA-63-matomo-tracking
branch
from
ab0507c to
bce6b07
Compare
- added react-native-get-random-values - https://github.com/LinusU/react-native-get-random-values - works with Expo since SDK 39 - uuidjs/uuid#515 - with that package we are able to create uuids for several use cases, like the matomo user id SVA-63
- created a central place for the namespace to avoid redundant creation of consts
- added storage helper methods for accessing the local storage
for `matomoSettings`
- created a general method `matomoSettings` with reading and writing matomo
settings from and to the local storage on app startup
- create user id for matomo tracking using
react-native-get-random-values
- created methods for directly creating and deleting the
user id in the matomo settings in the local storage
SVA-63
donni106
force-pushed
the
feature/SVA-63-matomo-tracking
branch
from
9b81aba to
52429ea
Compare
- refactored matomo setup to be done at very first when starting the app,
because otherwise the tracking for app start is fired too late
- removed everything matomo related from /src/index.js
- added two comments describing the order of processing the effects
- added matomo setup in App.js
- used the new matomo settings helper for creating the instance
with an user ID and regarding the consent for enabling/disabling
- created a new component `AppWithFonts` for setting up fonts,
with the addition to set `fontsLoaded` always to `true`, even if
there is an error, because we can show the app without the correct font
in that case
- maybe we want to add fallback font families elsewhere?
- added the first matomo tracking for app start in `AppWithFonts` which is
a possible here as it is a child component of the matomo provider with
the created matomo instance
SVA-63
- updated method calls for `SplashScreen` as we get deprecation warnings: - SplashScreen.preventAutoHide() is deprecated in favour of SplashScreen.preventAutoHideAsync() - SplashScreen.hide() is deprecated in favour of SplashScreen.hideAsync()
donni106
force-pushed
the
feature/SVA-63-matomo-tracking
branch
from
52429ea to
8961d1d
Compare
- created hooks for matomo tracking actions - refactored app start and home screen tracking to use new hooks - added web screen tracking with new hook SVA-63
digorath
requested changes
Nov 18, 2020
- if matomo setup fails, we still want to render the app - therefore we add an error object, which we can check at rendering, to avoid the matomo provider SVA-63
donni106
commented
Nov 19, 2020
- removed the hook as it is just used once in App.js, where it can be implemented directly SVA-63
|
ℹ️ user consent is still missing. this will follow in a separate PR, when the settings screen is done. therefore i will also checkup @digoraths implementation of |
- added consts to use in tracking strings
- added helper to build tracking strings from an array with data
- added `useMatomoTrackScreenView` in every screen and detail component
to send the request for tracking on mounting
- added `isConnected` in the hook to jsut send data if the user
has network connectivity
- in `HtmlScreen` and `IndexScreen` the hook could not be used, as we need
useEffect dependencies there
- added custom integration for these two screens
- added keys to queries for event records, news items and tours, which are
needed in order to set a correct tracking string
SVA-63
donni106
force-pushed
the
feature/SVA-63-matomo-tracking
branch
from
276034b to
4a57d01
Compare
digorath
requested changes
Nov 20, 2020
There was a problem hiding this comment.
Not everything has to be fixed in this PR.
But if you agree on the general structure for FCs then we should move the matomo hooks to the "correct" spot inside of the components.
most of the points where useCallbacks should be used are related to the future introduction of the refresh time hook, and could be ignored in this pr, because they only mean minor performance improvements.
agree! let's track that for the refresh time update first with keeping in mind for other context. cannot handle this refactoring related to the matomo topics here. |
- moved the method out of the component because it has no dependency on it, so it does not need to be re-created everytime with re-rendering
- moved `useMatomoTrackScreenView` up in components to have it structured in one place before `useEffect`s - moved destructing `consts` out of components, because there is no dependency SVA-63
- added `useEffect` for `WebScreen` to handle `webUrl` dependency - added check for `title` in `HtmlScreen` to not track if it is not present - moved early return because of undefined `query` in `IndexScreen` and added check for `query` inside of `useEffect` that depends on that variable in order to not track if it is not present SVA-63
|
👉 View analysis in DeepCode’s Dashboard | Configure the bot |
digorath
approved these changes
Nov 20, 2020
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.
Implemented a Matomo class and created a
useMatomohook to provide several methods for tracking.I have created an own package for our solution https://github.com/donni106/matomo-tracker-react-native. This makes it possible to use Matomo with Expo. The Matomo code is inspired by https://github.com/matomo-org/matomo-nodejs-tracker and https://github.com/Amsterdam/matomo-tracker.
https://github.com/BonifyByForteil/react-native-matomo does not work with Expo as it needs native modules. A feature request at Expo is not answered yet: https://expo.canny.io/feature-requests/p/matomo-tracking-integration
This PR was in process with different TODOs:
add user consent=> will be a new PR later => combine settings and matomo #125ℹ️ Please read the commit messages for further descriptions.
Listings in Matomo web interface: