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.

Sign up for GitHub

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

Jump to bottom

fix(ga): expose ga settings in ga provider #87 #281

Merged
merged 4 commits into from
Aug 30, 2018
Merged

fix(ga): expose ga settings in ga provider #87 #281

merged 4 commits into from
Aug 30, 2018

Conversation

rossedfort
Copy link
Contributor

  • What kind of change does this PR introduce?
  • Add a feature to the ga provider.
  • What is the current behavior? Link to open issue?
  • What is the new behavior?
  • A new method in the google analytics provider that allows a user to set a list of additional account names. These account names are used in any subsequent tracking events, exceptions, etc.

closes #87

feat(core/ga provider): implement method to set additionalAccountNames
f8e6f84 
implement a 'setAdditionalAccountNames' method in GA provider, which sets the account names as the
'additionalAccountNames' property of 'angulartics2.settings.ga'

closes angulartics#87
@rossedfort
Copy link
Contributor Author

This is an updated re-write of #88

@rossedfort rossedfort mentioned this pull request Jul 23, 2018
Closed
@scttcper
Copy link
Collaborator

scttcper commented Jul 24, 2018 •
edited

Does this have an advantage over importing angulartics2 and setting it directly?

this.angulartics2.settings.ga.additionalAccountNames = ['1', '2']

@rossedfort
Copy link
Contributor Author

@scttcper I didn't realize you can access the Google Analytics provider settings from the main Angulartics2 service. Now that I do, this PR is kind of pointless. However, it does provide one nice advantage, in that before I only had to inject my provider into my main AppComponent, and now, I have to the inject the Provider and the Angulartics2 Service.

@scttcper
Copy link
Collaborator

But then every setting on every provider will want a setter in core and it'll just never end. maybe could make the ga specific settings public inside of the provider.

@rossedfort
Copy link
Contributor Author

Yeah, true, that's a good idea. I can take a look at implementing that.

Ross Edfort added 3 commits July 25, 2018 09:03
Merge remote-tracking branch 'upstream/next' into issue/87-multiple-t…
f9fa368 
…rackers
Revert "feat(core/ga provider): implement method to set additionalAcc…
ffad767 
…ountNames"

This reverts commit f8e6f84.
feat(ga provider): allow modification of GA specific settings from GA…
f43dc96 
… provider

add a public instance member 'settings' on the ga provider. It is just a reference to the 'ga'
settings on the core angulartics2 service

closes angulartics#87
@rossedfort rossedfort changed the base branch from master to next July 25, 2018 15:34
@scttcper scttcper changed the title Implement method to set additional account names in ga provider #87 feat(ga): expose ga settings in ga provider #87 Jul 25, 2018
@scttcper scttcper changed the base branch from next to master July 25, 2018 15:39
@scttcper scttcper changed the base branch from master to next July 25, 2018 15:39
@scttcper scttcper changed the title feat(ga): expose ga settings in ga provider #87 fix(ga): expose ga settings in ga provider #87 Aug 30, 2018
@scttcper scttcper merged commit 53b7213 into angulartics:next Aug 30, 2018
@scttcper
Copy link
Collaborator

scttcper commented Oct 9, 2018

🎉 This PR is included in version 7.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scttcper scttcper scttcper approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@rossedfort @scttcper