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

Added support to ui-router ($state) #225

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

danieleratti
Copy link

Added support to ui-router in order to read the $state.current.doNotTrack and decide whether send or not the pageView.
This control is also done in the first page loading.

Added support to ui-router in order to read the $state.current.doNotTrack and decide whether send or not the pageView.
This control is also done in the first page loading.
@justinsa
Copy link
Collaborator

justinsa commented Apr 6, 2018

This is a good looking change but there needs to be some unit tests to go along with it and appropriate additions to the Readme.md for documenting the added functionality.

if (readFromState) {
// Evaluate whether send or not the first pageview
// I didn't find a better approach than $timeout since in this context the $state hasn't been loaded yet
$timeout(function() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of the timeout is there an event you could listen to? Is the $stateChangeSuccess event sufficient for tracking the first route setup by ui-router? Could this first call simply not happen for ui-router as it will be caught by the event?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants