Skip to content
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 ArcxAnalyticxProvider and an example CRA app #21

Merged
merged 6 commits into from
Nov 17, 2022

Conversation

gtupak
Copy link
Collaborator

@gtupak gtupak commented Nov 11, 2022

The SDK as-is was a bit of a pain to use as users need to make a custom react provider in their react apps. For this reason, I made a utility ArcxAnalyticsProvider that does that for you. It requires the apiKey and can optionally take a config object.

The provider initializes the SDK and exposes a hook useArcxAnalytics() that children of the provider can use.

The tests for the provider will be in a subsequent PR because of some configuration changes regarding the test framework and I don't want to make this PR too big.

In addition, I've added an example Create React App showcasing how to use the provider, with a console-like view showing what events have been fired. I tested this app using yarn link, but once the npm package is updated we'll be able to test it using the official npm package.

image

Copy link
Contributor

@classicus-eth classicus-eth left a comment

Choose a reason for hiding this comment

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

Love this 🚀 The Provider Component is super proffesional, and example app is a great idea.

Two things:

  • I can't get the example app to compile tho? From a fresh clone on this branch, might be me?
    image

  • Also would be neat to update tab metadata:
    image

@gtupak
Copy link
Collaborator Author

gtupak commented Nov 14, 2022

This is because the npm package is not yet published. The way to make it work on this branch is to:

  1. At the root folder, run yarn link, then yarn build
  2. Go to the example repo and run `yarn link "@arcxmoney/analytics"

Then try again. The idea however is to publish this package and make the example repo pull the package from npm directly.

@gtupak gtupak requested a review from classicus-eth November 14, 2022 18:15
package.json Outdated Show resolved Hide resolved
@gtupak gtupak force-pushed the refactor/do-not-save-url-in-window branch from 3c2355e to 922fd39 Compare November 15, 2022 01:15
Base automatically changed from refactor/do-not-save-url-in-window to main November 15, 2022 18:36
add window object typing

rm comments and log

do not save the url on the window object and add medium option to attribute call

make the current url private

add arcx analyticx provider and CRA example project

log page events

track event clicks on console view

update readme

fix installations

add missing peer dependencies

ignore webpack config ts to avoid eslint errors

update header and favicon

support react versions >=16
@gtupak gtupak force-pushed the feat/add-provider-and-example branch from 3fe072e to 2722a6a Compare November 15, 2022 23:57
example/README.md Outdated Show resolved Hide resolved
example/package.json Outdated Show resolved Hide resolved
package.json Outdated
Comment on lines 19 to 21
"@babel/core": "^7.0.0-0",
"@babel/plugin-syntax-flow": "^7.14.5",
"@babel/plugin-transform-react-jsx": "^7.14.9",
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need babel?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

peer dependencies requirement

Copy link
Contributor

Choose a reason for hiding this comment

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

From which package? :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

eslint-config-react-app

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

.. but now I realized we don't use it in this scope, so removed it! :) good catch!

@@ -0,0 +1 @@
/// <reference types="react-scripts" />
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@xitronix xitronix left a comment

Choose a reason for hiding this comment

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

Nothing critical, but address the comments at your discretion

Copy link
Contributor

@classicus-eth classicus-eth left a comment

Choose a reason for hiding this comment

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

Discussed via dm's

@gtupak gtupak merged commit 51982e0 into main Nov 17, 2022
@gtupak gtupak deleted the feat/add-provider-and-example branch November 17, 2022 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants