search

segment-integrations / analytics.js-integration-facebook-pixel

The Facebook Pixel analytics.js integration. https://segment.com/docs/integrations/facebook-pixel
MIT License
8 stars 16 forks source link

Support InitiateCheckout standard event #10

Closed vincentchio closed 8 years ago

vincentchio commented 8 years ago

Hi @hankim813,

Since you have the last commit, I will ping you for this. Thanks.

hankim813 commented 8 years ago

Hey @vincentchio, thanks for the PR. Could you write up a description for what this PR is trying to accomplish/add with links to relevant docs?

Started Order isn't a spec'd event so this implementation won't work. If you provide more details on what you are trying to do (with links to FB docs) we can see if we can add support!

vincentchio commented 8 years ago

Hi @hankim813, Started Order is supported as a standard analytics event (https://github.com/segmentio/analytics-events/blob/master/index.js#L8) but is not publicly documented currently. This event is currently used within Google Analytics (https://github.com/segment-integrations/analytics.js-integration-google-analytics/blob/master/lib/index.js#L79).

Facebook Pixel also has a InitiateCheckout standard event which maps to Started Order semantically. Therefore, it is natural to also have this supported within the Facebook Pixel Integration.

hankim813 commented 8 years ago

Gotcha! Thanks for the links! We'll merge your other PR #12 first. This PR seems to have overlapping changes as that one.

Will also defer to @segment-integrations/core team for review as well @f2prateek @thehydroimpulse @sperand-io

sperand-io commented 8 years ago

nice lgtm! if you can drop the agent changes from this PR we can move ahead with merging and deploying regardless of what we decide there

vincentchio commented 8 years ago

Thanks for the feedbacks! Since rebasing master fork will break existing dependency, I created a new one with just the InitiateCheckout event change (https://github.com/segment-integrations/analytics.js-integration-facebook-pixel/pull/13). Closing this one.