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

Improve tooltip "on placement" functionality #5779

Closed
OpenSiteMobile opened this issue Apr 11, 2016 · 5 comments
Closed

Improve tooltip "on placement" functionality #5779

OpenSiteMobile opened this issue Apr 11, 2016 · 5 comments

Comments

@OpenSiteMobile
Copy link

Came across what I think might be a slight improvement to the uib-tooltip module. I noticed on the Popup demo that when someone selects a new placement to use, the new one doesn't display correctly if there was a popup already visible. That is, the little arrow indicator doesn't seem to update correctly in any of the cases. It isn't shown or is shown within the body of the tooltip. In other words, the popover isn't updating automatically (correctly) when the new selected placement is fired. Maybe it can be fixed, but maybe this will suffice. I just added this line of code to the tooltip.js file starting at line 450:

if (ttScope.isOpen) { hide(); } ...as so:

observers.push(
   attrs.$observe(prefix + 'Placement', function(val) {
       if (ttScope.isOpen) { hide(); }    // added this
       ttScope.placement = val ? val : options.placement;
       var placement = $position.parsePlacement(ttScope.placement);
      lastPlacement = placement[1] ? placement[0] + '-' + placement[1] : placement[0];
      if (ttScope.isOpen) {
          positionTooltip();
      }
  })
);

All it does is close the Popup Placement Selection Demo displayed popup so that when the popup demo button is clicked again, the indicator arrow shows correctly (as it does for any new click).

Example: https://angular-ui.github.io/bootstrap/#/popover (at least in Google Chrome)

Angular: 1.5.3

UIBS: 1.3.1

Bootstrap: 3.3.6

@wesleycho
Copy link
Contributor

Wouldn't it be better to figure out when exactly the arrow isn't updating properly and fix it at the root cause? This seems like a bit of a hack to work around that without addressing the true logic issue. What are your thoughts @RobJacobs ?

@OpenSiteMobile
Copy link
Author

Yes, it probably would be better to fix it so the tooltip module updates correctly, but the best way to do that is beyond what I can help with. Still learning these modules. Although hiding, then rebuilding may work, or some combination of auto firing the button after change is detected.

@RobJacobs
Copy link
Contributor

There's a lot of overhead involved with destroying and recreating the tooltip, so I wouldn't recommend that approach. This can be fixed by not setting the lastPlacement value here:

https://github.com/angular-ui/bootstrap/blob/master/src/tooltip/tooltip.js#L454

A plunk demonstrating the proposed fix. I'll create a PR shortly.

@OpenSiteMobile
Copy link
Author

I tried the plnkr and it still hangs occasionally with the little arrow indicator not showing, or inside the button. It doesn't do it every time, so it might be a timing issue. I could not find a repeatable pattern to "make it fail", but it would usually fail after 4 or 5 tries. Sometimes on the first try, and sometime on the 4 or 5th. I tried setting the $timeout ($Browser delay) to 10ms on my version of Angular, and it makes it fail every time, so my best guess is that at least in my browser (and over my crappy internet connetion), something is changing slightly about when the positioning updating is firing relative to the rest of the tooltip.

My browsers:
Firefox for Ubuntu 39.0 &
Version 43.0.2357.130 Built on Ubuntu 14.10, running on Ubuntu 15.04

@RobJacobs
Copy link
Contributor

I don't think the use case for changing the placement while the popover/tooltip is open is very often, if ever. We offer the auto placement option to take care of placing the popover/tooltip where it fits for you before it displays. The demo page is an oddball implementation meant to demonstrate how the positioning behaves. Maybe it would be better to close the popover/tooltip on the demo page when a position placement is selected to avoid confusion.

@icfantv icfantv removed the type: bug label Apr 14, 2016
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

5 participants