fix(tooltip): Transcludes contents to allow dynamic selector text

[joshdmiller]

A dynamic selector did not work as the contents of the directive element
were not transcluded. This fix adds transclusion through a new directive
controller method. It also includes a new test for this case (through an
ngRepeat) as well as a modified demo to show this use case.

My apologies for the earlier PR I closed. The bug fix was correct, but the test was useless. This new PR contains the better test.

Closes #69

[pkozlowski-opensource]

Landed as ab44a86. Had a look at the fix and wouldn't think of this :-)
I'm starting to wonder if we are not making it more complex than needed... but at the same time don't have any idea of simplifying things... so just a gut feeling at the moment.

[petebacondarwin]

Isn't it easier just to use

{
transclude: true,
...
}

in the directive definition and then

var template = 
    '<tooltip-popup ng-transclude></tooltip-popup>';

which is basically what this new directive controller does anyway?

[joshdmiller]

@petebacondarwin Actually, it's not transcluded into the tooltip popup; it's transcluded into the element on which the tooltip directive is applied. The tooltip-popup is only in the DOM on mouseover. The issue I was trying to solve was transclusion on an element that had no template (i.e. the tooltip directive).

I agree with @pkozlowski-opensource that it feels overly-complicated. I also considered removing the isolate scope, but then I would have to include the $watch logic myself, so complexity increased either way, so I opted for code security complexity instead.

But I'm sure there's a better way to do this.

[lanterndev]

Thanks for getting a fix for this in so fast! Would you mind pushing it to https://github.com/angular-ui/bootstrap/blob/gh-pages/ui-bootstrap-tpls-0.1.0-SNAPSHOT.js too?

(What's the difference between that version and the one without tpls, by the way? Couldn't find that documented, sorry if I missed it.)

[pkozlowski-opensource]

@Skivvies just pushed demo & build updates to the gh-pages branch. You can grab updated files from http://angular-ui.github.com/bootstrap/ui-bootstrap-tpls-0.1.0-SNAPSHOT.min.js

As for the file naming we've got:

• ui-bootstrap-[version].js
• ui-bootstrap-tpls-[version].js
• ui-bootstrap-[version].min.js
• ui-bootstrap-tpls-[version].min.js

Files with the min suffix are minified versions to be used in production (self-explaining). Then we've got files with the -tpls. To explain the difference let's start by explaining that we want our directives to be highly customizable. To achieve this we are trying hard not to hard-code markup nor CSS inside JavaScript code. Instead most of the directives are coming with a dedicated template(s) so people can change them. We are providing default templates based on Twitter's markup and CSS but you should be able to prepare your own partials to change the look&feel of widgets from this repo.

Now it should be clear that files with the -tpls in their name have bootstrap-specific templates bundled with directives. For people who don't want to take all the directives and don't need to customize anything the solution is to grab a file named ui-bootstrap-tpls-[version].min.js. If, on the other hand default templates are not what you need you could take ui-bootstrap-[version].min.js and provide your own templates, taking the default ones (https://github.com/angular-ui/bootstrap/tree/master/template) as a starting point.

[lanterndev]

Thanks @pkozlowski-opensource! Saw you added this info to the README in the gh-pages branch, nice.

I also saw you pushed the fix in ab61230, but my plunkr (which points to http://angular-ui.github.com/bootstrap/ui-bootstrap-tpls-0.1.0-SNAPSHOT.min.js) doesn't seem to have been fixed by it, hm.

[joshdmiller]

@Skivvies The fix is in the gh-pages branch, but has not yet been pulled by Github Pages. This should fix itself pretty soon.

[pkozlowski-opensource]

@joshdmiller Normaly pages are built just seconds after the push... Maybe a cash refresh problem?

[joshdmiller]

@pkozlowski-opensource Indeed you're right. I wonder why Github is telling the browser the file is unchanged...

[petebacondarwin]

@joshdmiller Thanks for that. I understand now.
This raises another question. Why is the tooltip using isolate scope in the first place. It is not needed since we are not using a template for it. We should be manually interpolating and parsing the attributes and leaving the scope alone. Then you do not need to worry about transclusion at all.

[joshdmiller]

@petebacondarwin We are using a template, but just not for the primary directive. Right now, the popup does not have an isolate scope while the tooltip does; we could swap this, removing the isolate scope from the tooltip and therefore also the need to worry about transclusion, and add an isolate scope to the popup directive. It increases complexity, but it's probably more "correct". What do you think?

[lanterndev]

I just tried the latest version with the example in http://plnkr.co/edit/1GiwHv locally and it's still broken for me. Would anyone be able to check if #75 is actually a duplicate of this issue, and if not, reopen?

[joshdmiller]

@Skivvies There is a typo in your Plunker. On line 17, you use "path" as the expression when it should be "url". Here's a corrected version: http://plnkr.co/edit/PNpdIg?p=preview.

[lanterndev]

Oops, sorry for sending you a bad version! Here's the version I meant to send:
http://plnkr.co/edit/1GiwHv
That version demonstrates there's still a problem when you give a tooltip to a link whose href is bound to a value that is initially falsey: even after the value changes to something truthy, the href attribute stays set to nothing.