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

ui-sref cause redundant state transition #350

Closed
buunguyen opened this issue Aug 23, 2013 · 37 comments
Closed

ui-sref cause redundant state transition #350

buunguyen opened this issue Aug 23, 2013 · 37 comments
Labels

Comments

@buunguyen
Copy link

I noticed that whenever ui-sref is used, a state is transitioned to twice. Upon examining the source code, it looks like the issue is caused by the fact that $StateRefDirective not only generates a href but also binds onclick event. By removing one of those, the issue is solved.

Is this a bug or by design? Given that the built-in ng-href only generates href, is there a reason ui-sref has to bind the onclick event?

@nateabele
Copy link
Contributor

If you look here, you'll notice that ui-sref suppresses the default click behavior, so for instances where you're clicking the link, the transition is handled by $state.go(), not by the link changing the URL.

@buunguyen
Copy link
Author

Hmm, you're right. Still, I have that issue and removing either the href or onclick stops it. Let me see if I can reproduce it in a plunker.

@laurelnaiad
Copy link

AFAIK, you can't disable the browser's response to href in html.

@buunguyen
Copy link
Author

This plunker demonstrates the problem. Interestingly, the issue only happens when the function inside resolve runs asynchronously. I include both cases in that plunker, just comment out the first or second case and notice the difference when clicking the Next link.

@buunguyen
Copy link
Author

When I replace e.preventDefault() with return false, the problem disappears. But... only when jquery is referenced. If jquery is not used, the problem persists.

Examining jquery source, return false also causes e.stopPropagation() to be invoked. Adding e.stopPropagation() after e.preventDefault() fixes the issue regardless of whether jquery is used or not (see this plunker). But stopping propagation is obviously not a good idea.

I have no idea why propagation has a part in this (again, this issue occurs in both the simple plunker provided as well as my production app). Besides, back to the original question, why should ui-router generates href and onclick at the same time, why not just href like ng-href?

@nateabele
Copy link
Contributor

Because states don't have to have URLs.

@timkindberg
Copy link
Contributor

Maybe the answer is to only do one or the other depending on if a URL is defined.

@nateabele
Copy link
Contributor

Strongly disagree. One of the core concepts of this library is bridging the gap between new-school applications and standard Web conventions, which is what this enables.

@buunguyen
Copy link
Author

I'm not exactly sure why you think @timkindberg's suggestion defeats that purpose? I don't see how a user can possibly be affected by that change. (Doesn't mean I'm not interested in learning about the root cause & solving it.)

@nateabele
Copy link
Contributor

Sorry, I misread it. I'll try and figure out a solution for this as soon as I can. Probably this weekend.

@ghost ghost assigned nateabele Aug 26, 2013
buunguyen added a commit to buunguyen/ui-router that referenced this issue Aug 26, 2013
@buunguyen
Copy link
Author

I just submitted a PR, please take a look. (Sorry about multiple references in this thread; the PR has only 1 commit however.)

@nateabele
Copy link
Contributor

Please see my comments on the PR. Unfortunately it doesn't really satisfy the requirements. I'll have to add some additional test coverage to uiSref so we don't have any regressions.

@ubenzer
Copy link

ubenzer commented Sep 11, 2013

@buunguyen Thanks for a quick-fix. I'll use yours until offical fix is applied. Since I have some ajax queries on my states' resolve, I don't want to fetch them twice. :)

@buunguyen
Copy link
Author

@ubenzer you're welcome. I'm also using the stopPropagation() hack in my app. Hope @nateabele have a chance to take a look at this soon.

@eupharis
Copy link

I also ran into this issue. I couldn't make the event.stopPropagation() hack work in my use case. Here's my workaround:

var off = scope.$on('$stateChangeStart', foo);

Calling off will remove the listener. So just call off in foo. Then it doesn't matter if the state change transition starts twice.

Obviously, there are lots of instances where this wouldn't work.

Looking forward to the real fix in ~ 0.4.0!

@foiseworth
Copy link

Thanks @buunguyen - I just needed this line: if (isForm || nav) return;

Could we get this merged? Double triggering of state is a no-win.

@lubiluk
Copy link

lubiluk commented Apr 17, 2014

I ran into similar problem. A state transition is performed twice. It happens when my ui-sref doesn't include all of the state parameters. My intention is to have optional parameter.
The first redirection supplies null as a value of the missing parameter and then the second redirections supplies "" in place of null.
I know that ui-router doesn't really support optional parameters but this behaviour is odd. Maybe it will somewhat helpful for finding the cause

@aramonc
Copy link

aramonc commented May 6, 2014

I can confirm that the (as stated by @lubiluk) is with the ui-sref not including all of the state parameters. In my case I was trying to rely on parameters from parent routes already being resolved.

@charandas
Copy link
Contributor

+1 @lubiluk

The first redirection supplies null as a value of the missing parameter and then the second redirections supplies "" in place of null.

I am using the master branch which supposedly should support optional parameters. However, I get the same behavior with null being passed once, and "" the second time. I am working around this by passing in a "" instead of omitting the optional parameter and checking for "" in my controller.

@afreeland
Copy link

Still seeing an issue with the empty optional params that @lubiluk mentioned...would be nice to see a fix without a workaround. Not the end of the world as there is a workaround as @kbdaitch brings up, markup ends up looking like <li ui-sref="smart({id: ''})" >

@eekboom
Copy link

eekboom commented Sep 4, 2014

I got bitten by this even in 0.2.11.
I tried to define default values for parameters, but that did not help :-(

@pthanos
Copy link

pthanos commented Sep 9, 2014

+1
Any advice for a quick fix or workaround? We heavily rely on resolve with async calls, so all our services are called twice.

@3plusalpha
Copy link

+1
Is there any update on this issue yet?

@putermancer
Copy link

+1. Thanks @lubiluk, your observation about missing parameters saved me a lot of time today.

@cristiam86
Copy link

+1 @lubiluk
I had the $viewContentLoaded event fired twice because of this issue.
Maybe it can be interesting to note that state parameters aren't optional. I dind't knew that.

@kavarell
Copy link

kavarell commented Nov 9, 2014

+1 @lubiluk
I'm also experiencing this problem. Just curious, but why is it that parameters, like query params for example, cannot be optional?

@mtpultz
Copy link

mtpultz commented Nov 15, 2014

Hi @kavarell, query params are optional in v0.2.11, which works without loading the state twice.

I recently found a optional parameter syntax somewhere in github for ui-router that uses {}, so something like url: '/my/:param/{optionalParam}', and it works but it also loads the state twice, which is currently a issue for me too, so +1

@justincy
Copy link

#1476

@illegalnumbers
Copy link

+1

@stoyanvi
Copy link

+1. Including ui-sref="stateName({ "optionalParam": "" })" fixes it, but we cannot rely on this.

@ghost
Copy link

ghost commented Feb 9, 2015

+1 The tips saved me a lot of time :)

PM
www.gastromatic.de

@eddiemonge
Copy link
Contributor

@itmilos
Copy link

itmilos commented Dec 23, 2015

Try to use $locationChangeStart instead of $stateChangeStart

So try calling off as @eupharis, but with locationChangeStart

var off = scope.$on('$locationChangeStart', foo);

@christopherthielen christopherthielen removed this from the 1.5.0 milestone Feb 6, 2016
@haynesgt
Copy link

haynesgt commented Nov 9, 2016

Issue observed in angular-ui-router v0.2.18
To remove the second click, add a target='_self' attribute to the element.
For example, see http://plnkr.co/edit/XiXzGeBasGrxsqyscOTN?p=preview

@iMomen
Copy link

iMomen commented Jan 23, 2017

Issue is still observed in angular-ui-router v0.3.2
any updates ?

@patrick-99
Copy link

I use angular-ui-router v0.3.1, and register on state change using this code implemented in my controller:

$rootScope.$on('$stateChangeStart', function(event, toState, toParams, fromState, fromParams) {
    if (fromState.name === 'home' && toState.name === 'campaigns') {
      // my code to run upon this specific state transition
      clientService.quit();
    }
})

The HTML to trigger the state transition is like:
<a ui-sref="campaigns" target="_self"><img class="logo" src="images/logo_small.png" alt="Logo"></a>

The whole thing is embedded in a Single Page App.
I noticed the '$stateChangeStart' event listener is first called once, next time twice, and so on, called 5 times the 5th time...

I was suspecting the issue to be the state change listener being registered each time the navigation is routing to this page. So I updated the code to unregister in the callback itself:

  var stateChangeListener = $rootScope.$on('$stateChangeStart', function(event, toState, toParams, fromState, fromParams) {
    stateChangeListener();
    if (fromState.name === 'home' && toState.name === 'campaigns') {
      clientService.quit();
    }
  })

It now works as expected, as the callback function is only triggered once.

@nhutdinh
Copy link

nhutdinh commented Sep 6, 2017

@patrick-99 : event.preventDefault() wont work in this case

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

Successfully merging a pull request may close this issue.