1.3.0-beta11 and ngIf broke angular-ui-bootstrap's typeahead

[wuzzeb]

Not sure if this is a bug in angular or angular-ui/bootstrap, but I manually bisected this where 1.3.0-beta10 works and 1.3.0-beta11 fails so I am reporting this here.

http://plnkr.co/edit/RykkeZqMnyHXLbPfciV9 shows the bad behavior. Change to beta10 and you can see it work. What happens for me with beta11+ is if you type something into the typeahead and then click on one of the states that pop up, the state is not selected.

I debugged it to the following:

In https://github.com/angular-ui/bootstrap/blob/master/template/typeahead/typeahead-popup.html the ul tag uses ng-if directive to decide to show the dropdown or not. What happens in beta11+ is that the scope for the typeahead-popup directive (located https://github.com/angular-ui/bootstrap/blob/master/src/typeahead/typeahead.js#L327) is not rebound to the ul element when the popup is opened. Therefore, the ng-click directive on the embedded li element does nothing, i.e. it should call scope.selectMatch but since the scope has not been rebound correctly the function is not called. Next the click event is propogated and eventually the typeahead directive click handler (https://github.com/angular-ui/bootstrap/blob/master/src/typeahead/typeahead.js#L303) is run to close the popup.

If I change the ng-if directive on the ul tag to ng-show then the typeahead works (tested with beta14) since the li element now has the right scope so the call to selectMatch works. This leads me to believe that d71df9f is the problem.

[btford]

I think this is an issue with angular-ui-bootstrap. I'd file an issue there.

[Narretz]

This is related to the fixes in $transclude with ng-if (& ng-repeat). it seems like quite a number of people (this is not the first issue about this) have used transcluded content with a different intention that accidentially worked. It's not a breaking change per se, but maybe we should add few words to the changelog for 1.3.ß-beta.11 that compares what previously worked with how it works now? @petebacondarwin what do you think?

[petebacondarwin]

Perhaps a BREAKING CHANGE block would be acceptable in the CHANGELOG - breaking in the sense that, if you were using it wrong before it definitely wouldn't work now. We should point out that this really is a bug in angular-ui-bootstrap since even before we made this change the scope would have been incorrectly assigned if there had been nested transclusions.

[myitcv]

@Narretz @petebacondarwin - agree with @Narretz's suggestion here. A simple document that includes the trap(s) people fell into as well as the correct implementation might be useful. A simple wiki as part of the AngularJS Github repo (that can easily be added to) might just do the trick?

[btford]

wiki as part of the AngularJS Github repo

A wiki, just like Disqus comments in the past, would have the downside of being easily out-of-date and cumbersome to review.

This should be added to the documentation via a PR.

[pkozlowski-opensource]

Hmm, this is kind of bad coincidence since the typeahead directive used to use ngShow instead of ngIf - the actual change was introduced in angular-ui/bootstrap@3111501. But looking at this again I don't think there are any true advantages of using ngIf so I'm probably going to revert the commit on the angular-ui side.

[myitcv]

@btford - presumably you would agree, as @Narretz pointed out, this is something that could catch out a number of people who migrate to v.1.3.0*?

At the moment there is no document, regardless of where it lives, that people can be directed towards to refer to breaking changes between v1.2.x and v1.3.x (I note the historic document for migration between v1.0 and v1.2). Given the more fluid nature of changes pre release of v1.3.0 I thought this could better be worked on in a Github wiki, the document itself migrating to the real documentation on release. But I very much defer to you and the other owners of the project on where this document lives now or in the future.

The benefit of it being a more fluid, living document in the short term would of course be that other edges cases, as they are found, can easily be added.

[petebacondarwin]

The CHANGELOG contains all the breaking changes (with the possible exception of this one). Just look for the BREAKING CHANGES section in each 1.3.0-beta.x release.

[myitcv]

@btford @petebacondarwin - apologies, I retract everything I just said. Thanks for pointing me towards this.