ui-sref on a parent element breaks all ui-srefs on children

[rluba]

Example use case:

• There’s a table where each row links to a sub-state.
• For the user’s convenience the whole <tr> is made clickable by adding ui-sref="A" to it.
• Some of the table columns contain <a> links to other states (implemented by using ui-sref="B" on those link tags).

When you click on any of those links – because you want to visit state B – the link to A on the <tr> elements takes precedence because ui-sref’s click handler does not call e.stopPropagation() when it handles the click event. So the transition to B gets superseded by a transition to A when the parent element processes the event. Therefore, you always end up in state A – no matter where you click.

Adding e.stopPropagation() somewhere in the click handler solves the problem.

Tested with [email protected] and [email protected]

[rluba]

@christopherthielen, I just discovered a new problem that arises when e.stopPropagation() is added – which is probably why it wasn’t added originally:

When the link is part of a dropdown (eg. angular-bootstrap’s one), calling e.stopPropagation() will prevent the dropdown from closing after the click. This seems to be a bigger issue than the nested ui-sref case, so we should probably revert the patch.

A possible fix might be to let ui-sref check if a transition is in progress and don’t transition in that case, but I’m sure that this will have other negative side effects. 😕 (Eg. clicking on links while a long-running transition is in progress will no longer work…)

Any better ideas?

[rluba]

Another way might be to share the transition timeout variable between all ui-sref clickhooks and set and clear it within the handler. Then any ui-sref could check if another ui-sref already initiated a transition during this cycle, so it won’t interfere with other transitions.

But that still feels a bit messy.

[shql]

@rluba & @christopherthielen

We also run into breaking issues due to this change. We have a ui-sref element that changes the state but its container has also a click event listener that does stuff. With this change the container event is never fired. I strongly recommend to revert this as you cannot rely on event propagation anymore.

What do you guys think?

[christopherthielen]

what if you just wrote a stop-propagation directive for those ui-sref that are doubly nested?

.directive('stopPropagation', () => ({
  restrict: 'A',
  link: (scope, elem) => elem.on('click', event => event.stopPropagation())
}));

<a ui-sref="foo">
  <a ui-sref="foo.bar" stop-propagation></a>
</a>

[christopherthielen]

@rluba I've reverted the commit for beta.3. Can you try the stopPropagation directive technique?

[rluba]

I'm currently using my "transition timeout as shared variable" fix in a local fork and it works great. I'll submit a PR as soon as I'm confident that there are no side effects.

[shql]

Thanks guys

[christopherthielen]

Here's a minimal reproduction: http://plnkr.co/edit/vp0aMZgYIuRK2fXvx96b?p=preview

Clicking 'home.foo' ui-sref in the <td> activates the ui-sref on the <tr>

[christopherthielen]

Here's a viable workaround (<stop-propagation> directive)
http://plnkr.co/edit/mxtjxiq3eVssYhfnKsSf?p=preview

[christopherthielen]

I'll submit a PR as soon as I'm confident that there are no side effects.

I'd like to see your solution. However, know that I'm wary of adding more code (and managing more state) for a case which I don't think is very common and which there is a simple workaround.