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

Tooltip remains when location is changed while tooltip is visible #345

Closed
choffmeister opened this issue Apr 22, 2013 · 11 comments
Closed

Comments

@choffmeister
Copy link

When a tooltip is visible at the moment and one changes the current location (for example by pressing backspace) then the tooltip remains an screen. It does not disappear until full page reload.

@pkozlowski-opensource
Copy link
Member

Similar to #335

We should be watching $location change events or use a similar technique. Should be an easy fix.

@choffmeister
Copy link
Author

Sounds good.

Haven't tested yet, but this bug might be common to all overlay elements…?

On Apr 22, 2013, at 6:39 PM, Pawel Kozlowski [email protected] wrote:

Similar to #335

We should be watching $location change events or use a similar technique. Should be an easy fix.


Reply to this email directly or view it on GitHub.

@pkozlowski-opensource
Copy link
Member

@choffmeister yes, this would be a common problem for tooltips, popovers and the like. After thinking about it some more I'm a bit puzzled why those elements stay in the DOM, Are you attaching tooltips to the <body>?

@jhiemer
Copy link

jhiemer commented Apr 23, 2013

@pkozlowski-opensource when I read it first, I thought that he has the tooltip bound not to the view being refreshed/replaced and instead in a parent element with a different controller. Because changing the view itself should always result in a re-render of the DOM, or?

Something I was fighting last week... :-)

@choffmeister
Copy link
Author

After work this afternoon I will submit the concrete HTML markup.

@pkozlowski-opensource
Copy link
Member

@jhiemer I must say that at present I don't know what is going on, didn't debug this. I suspect that the tooltip is attached to the <body> element (there is such an option in the tooltip directive) and we are not removing it when location changes.

@choffmeister yes, a reproduce scenario would be awesome, need to make sure that we are fixing the right thing...

@choffmeister
Copy link
Author

Yes I bound the tooltip to the body (or more precisely, a special DIV container that is meant to contain the tooltip overlays). I do this, because the tooltip shall show when the mouse is over a SVG element.

On Apr 23, 2013, at 8:59 AM, Johannes Hiemer [email protected] wrote:

@pkozlowski-opensource when I read it first, I thought that he has the tooltip bound not to the view being refreshed/replaced and instead in a parent element with a different controller. Because changing the view itself should always result in a re-render of the DOM, or?

Something I was fighting last week... :-)


Reply to this email directly or view it on GitHub.

@joshdmiller
Copy link
Contributor

@choffmeister - What do you mean you're attaching tooltips to a special DIV? The current version does not have that capability.

@pkozlowski-opensource
Copy link
Member

@choffmeister @joshdmiller I kind of managed to reproduce it in this plunk: http://plnkr.co/edit/KJ0KZeOA9RK3c883NPz0?p=preview It uses the latest version from master as we need to be able to append tooltips directly to body.

In order to reproduce click on the "second" link, route changes, activate tooltip and press back to change the route. You will notice that the tooltip remains displayed. This is very minor for tooltips but might be more problematic for popovers.

We should fix it as it is a real problem and should be easy to tackle. Still, I'm not sure if this is what @choffmeister reported... Anyway, we can fix the issue from this plunk at least if we don't hear from the person.

@joshdmiller
Copy link
Contributor

@pkozlowski-opensource Yeah, I see that now. It should be this simple:

scope.$on('$locationChangeSuccess', function closeOnLocationChange () {
  if ( scope.tt_isOpen ) {
    hide();
  }
});

@pkozlowski-opensource
Copy link
Member

@joshdmiller I'm starting to cut a new 0.3.0 release and got a fix for this one in:
https://github.com/angular-ui/bootstrap/tree/issue345

But I think that this is one is not very urgent as this is a corner case really. So I will just open a PR for this one so you can review and we can do some more testing.

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

Successfully merging a pull request may close this issue.

4 participants