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 piwik.js public api method setPagePerformanceTiming. #17901
Conversation
@geekdenz it wouldn't be a separate request immediately but the information would be sent along the next tracking request. It's mentioned in the issue. |
@geekdenz I guess this might need to be implemented in another way. The Lines 3773 to 3777 in da4df21
When appending custom values we also need to set |
build js |
build js |
build js |
Only failure is now this: https://app.travis-ci.com/github/matomo-org/matomo/jobs/533572041#L1120-L1122 Looks like it's not caused by my. So OK to merge? |
to work as expected added comments from @sgiehl because this is not obvious
build js |
Added some more tests here: |
Not 100% sure but I think this test failure is probably not related to this issue: |
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.
Getting there 🎉 Left a comment re different behaviour of values.
build js |
build js |
Considerations that went into this:
|
I think there are still some improvements that should be made here before merging. |
we could write more functions like this in the future, e.g. mapIn, filter or map, but currently this is not needed because there is just this one use-case
Documented here: |
build js |
Saw that these seem fine to download: https://builds-artifacts.matomo.org/matomo-org/matomo/4.x-dev/48972/ Not sure which change made these fail, but it seems not related to this PR. |
@geekdenz Those look like some random failures. Might be good to merge in the latest changes from 4.x-dev into your branch as on 4.x-dev all tests are currently passing. |
Tested it locally and IE8 etc and seems to work 👍 |
@tsteur Your last commit actually would have required another build of the javascript files. Will fix that directly on 4.x-dev |
fixes #16859
Description:
Still need to add documentation.
Is it correct to sendRequest immediately?
Are there other parameters we need to take care of?
Review