Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

bug(tooltip): triggers are global for all tooltips #692

Closed
Hviid opened this issue Jul 22, 2013 · 7 comments
Closed

bug(tooltip): triggers are global for all tooltips #692

Hviid opened this issue Jul 22, 2013 · 7 comments
Milestone

Comments

@Hviid
Copy link

Hviid commented Jul 22, 2013

In the tooltip directive the following are set "globally" in the directive and not in the returned "instance".

var triggers = setTriggers( undefined );

UPDATE:
Because triggers are set as a variable in the tooltip function and not in the returned obj, then "triggers" are shared accross all tooltips. Which for instance won't work as expected in the following.

attrs.$observe( prefix+'Trigger', function ( val ) {
  element.unbind( triggers.show );
  element.unbind( triggers.hide );

  triggers = setTriggers( val );

  if ( triggers.show === triggers.hide ) {
    element.bind( triggers.show, toggleTooltipBind );
  } else {
    element.bind( triggers.show, showTooltipBind );
    element.bind( triggers.hide, hideTooltipBind );
  }
});

tooltip1 and tooltip2 might here had bound show to "mouseover".
Then when triggers are updated by tooltip1 to say "click", then tooltip2 won't get "mouseover" unbound, when it's trigger attribute gets changed, because triggers.show will resolve to "click" and not "mouseover".

Proposed fix is to move:

var triggers = setTriggers( undefined );

into the returned obj, so the variable is locale to the individuale tooltip.

@pkozlowski-opensource
Copy link
Member

@Hviid sorry, but I've got no idea what you are talking about here... Is is a bug report? If so please prepare a minimal reproduce scenario using http://plnkr.co/

@Hviid
Copy link
Author

Hviid commented Jul 22, 2013

Just updated the issue, hope it makes sense now. I can create a plunker later, if it still doesn't make sense.

@pkozlowski-opensource
Copy link
Member

@Hviid Actually you can set triggers in 2 places - globally and for each directive instance. I'm still not super-clear about how exactly you are using this thing in your code so I can't confirm if this is a bug or not. On the demo page you can actually see a mixture of popovers using different triggers (click and mouseover).

Please provide a reproduce scenario using http://plnkr.co/ so we can progress on this one.

@Hviid
Copy link
Author

Hviid commented Jul 23, 2013

Here the repro:
http://plnkr.co/edit/rE1btD

Problem:
See how mouseover still works on the second input, after pressing the button.

First both gets set to tooltip-trigger="mouseenter"
Then when you click the button, they both gets set to tooltip-trigger="click"

attrs.$observe( prefix+'Trigger', function ( val ) {
  element.unbind( triggers.show );
  element.unbind( triggers.hide );

  triggers = setTriggers( val );

  if ( triggers.show === triggers.hide ) {
    element.bind( triggers.show, toggleTooltipBind );
  } else {
    element.bind( triggers.show, showTooltipBind );
    element.bind( triggers.hide, hideTooltipBind );
  }
});

Will then set triggers.show="click" and triggers.hide="click" for the first tooltip. So when the next tooltip runs the $observe function triggers.show and triggers.hide will be "click". Because triggers is a shared variable.

element.unbind( triggers.show );

Will try to unbind "click" on the element, not "mouseenter"

@pkozlowski-opensource
Copy link
Member

@Hviid Awesome, thnx for a plunk, this makes it clear! If you would be up to sending a pull request, this would be awesome. Otherwise I will try to look into this one later this week.

@pkozlowski-opensource
Copy link
Member

thnx @Hviid this was a legitimate bug, thnx for the report. The fix is in master, a new release is in preparation.

@Hviid
Copy link
Author

Hviid commented Aug 3, 2013

No problem.
Thanks for the good work on angular ui. Sorry I didn't get to send a pull request. I'm on a tight project atm. but after that I'll try to send a couple of pulls.

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

No branches or pull requests

2 participants