Skip to content
This repository has been archived by the owner on Mar 18, 2021. It is now read-only.

COM-20077-GA-event-doesnt-work #149

Merged
merged 1 commit into from Dec 19, 2018

Conversation

ciastektk
Copy link
Collaborator

@ciastektk ciastektk force-pushed the COM-20077-GA-event-doesnt-work branch from 9904177 to ae02081 Compare December 6, 2018 16:40
@ciastektk ciastektk force-pushed the COM-20077-GA-event-doesnt-work branch 7 times, most recently from 36dd551 to 2d1c1c9 Compare December 10, 2018 18:13
@ciastektk ciastektk force-pushed the COM-20077-GA-event-doesnt-work branch 3 times, most recently from f304209 to 7514c51 Compare December 11, 2018 08:59
Copy link
Contributor

@damianz5 damianz5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@ciastektk ciastektk force-pushed the COM-20077-GA-event-doesnt-work branch from 7514c51 to bf4e774 Compare December 11, 2018 15:16
Copy link
Contributor

@sunpietro sunpietro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the first part of review.

app/Resources/views/themes/ezplatform/full/home.html.twig Outdated Show resolved Hide resolved
web/assets/js/GoogleAnalyticsService.js Outdated Show resolved Hide resolved
web/assets/js/GoogleAnalyticsService.js Outdated Show resolved Hide resolved
web/assets/js/GoogleAnalyticsService.js Outdated Show resolved Hide resolved
web/assets/js/GoogleAnalyticsService.js Outdated Show resolved Hide resolved
web/assets/js/app.js Outdated Show resolved Hide resolved
@ciastektk ciastektk force-pushed the COM-20077-GA-event-doesnt-work branch 2 times, most recently from d7bf42a to 341a5e7 Compare December 13, 2018 14:14
@@ -0,0 +1,12 @@
class GoogleAnalyticsItem {

constructor(hitType = 'event', eventCategory, eventAction, eventLabel = null, transport = {}) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @sunpietro. This is should be an object or map, not a class.

web/assets/js/GoogleAnalyticsService.js Outdated Show resolved Hide resolved
@ciastektk ciastektk force-pushed the COM-20077-GA-event-doesnt-work branch 2 times, most recently from 8245197 to 93590a6 Compare December 14, 2018 09:04
@ciastektk
Copy link
Collaborator Author

@sunpietro, @dew326 many thanks for your help! I provided changes requested by you. Can you check them?

test/googleAnalyticsService.spec.js Outdated Show resolved Hide resolved
web/assets/js/GoogleAnalyticsService.js Outdated Show resolved Hide resolved
web/assets/js/Utils.js Outdated Show resolved Hide resolved
@ciastektk ciastektk force-pushed the COM-20077-GA-event-doesnt-work branch 3 times, most recently from 8edac06 to d5bc942 Compare December 18, 2018 09:39
@ciastektk ciastektk force-pushed the COM-20077-GA-event-doesnt-work branch from d5bc942 to bb84a4e Compare December 18, 2018 10:28
@ciastektk ciastektk force-pushed the COM-20077-GA-event-doesnt-work branch from bb84a4e to e15f5f5 Compare December 18, 2018 10:30
@ciastektk
Copy link
Collaborator Author

@dew326 can you check the new changes?

@ciastektk
Copy link
Collaborator Author

ciastektk commented Dec 18, 2018

@sunpietro, do you have other suggestions?

@damianz5 damianz5 merged commit ecd51ec into ezsystems:master Dec 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

None yet

5 participants