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

"Remove tab" problem #5

Closed
max-mykhailenko opened this issue Oct 14, 2012 · 31 comments
Closed

"Remove tab" problem #5

max-mykhailenko opened this issue Oct 14, 2012 · 31 comments

Comments

@max-mykhailenko
Copy link

Didn't removes tab after removing item from array. Content removes correctly

@ajoslin
Copy link
Contributor

ajoslin commented Oct 15, 2012

Hi max, I'm currently redoing this right now. I'll push it later today.

@ajoslin ajoslin closed this as completed Oct 15, 2012
@ajoslin ajoslin reopened this Oct 15, 2012
@ajoslin
Copy link
Contributor

ajoslin commented Oct 15, 2012

Try the new build. Tabs are different now, with a simpler layout and a template being used. This is the direction all of the directives are going.

@max-mykhailenko
Copy link
Author

My tab template didn't see root scope. Maybe I do something wrong. Please add some documentation.

<tabs>
<pane ng-repeat="lang in article.languages" title="Version {{lang.name}}" > </pane>
</tabs>

{{lang.name}} shows as text, not variable

<div class="tab-pane" ng-class="{active: selected}" ng-show="selected" ng-transclude>
<span ng-model="lang.name"></span>
</div>

lang.name didn't shows

@max-mykhailenko
Copy link
Author

If I set .. scope : true ... in pane directive, values shows. It correct solution?

@pkozlowski-opensource
Copy link
Member

Reproduce scenario for the title not interpolated: http://plnkr.co/edit/1Ea4ni?p=preview
Bumping into the same issue with accordion.

@ajoslin, any ideas? It looks like isolated scope + @ + ng-repeat issue
@max-mykhailenko changing scope definition to use a non-isolated scope would solve the issue here but it would mean that the directive can really influence existing scopes with its internal state and this is not good :-(

@petebacondarwin
Copy link
Member

Bizarrely, sometimes it does seem to interpolate ok.
Pete

On 16 October 2012 22:03, Pawel Kozlowski notifications@github.com wrote:

Reproduce scenario for the title not interpolated:
http://plnkr.co/edit/1Ea4ni?p=preview
Bumping into the same issue with accordion.

@ajoslin https://github.com/ajoslin, any ideas? It looks like isolated
scope + @ + ng-repeat issue
@max-mykhailenko https://github.com/max-mykhailenko changing scope
definition to use a non-isolated scope would solve the issue here but it
would mean that the directive can really influence existing scopes with its
internal state and this is not good :-(


Reply to this email directly or view it on GitHubhttps://github.com//issues/5#issuecomment-9506762.

@pkozlowski-opensource
Copy link
Member

Yeh, under debugger I can see that the first interpolation is OK, then it
gets overridden... I guess I'm missing sth fundamental here...

@ajoslin
Copy link
Contributor

ajoslin commented Oct 16, 2012

Thanks for looking into this more Pawel, been busy.

It does work if we manually $watch interpolation on the parent scope: http://plnkr.co/edit/CWlsvA?p=preview

But this seems to basically be what angular core does too.. it sets up an $observe and makes the scope of it be the parent.
https://github.com/angular/angular.js/blob/master/src/ng/compile.js#L723
https://github.com/angular/angular.js/blob/master/src/ng/compile.js#L1028

Not sure..

@johlrich
Copy link

I wonder if this isn't some other issue related to AngularJS + TemplateURL. When using a string supplied template this issue does not seem to appear. See: http://plnkr.co/edit/PJirWI?p=preview

I haven't looked at AngularJS directly, but hopefully this helps you narrow down the cause.

@pkozlowski-opensource
Copy link
Member

@johlrich Awesome, how did you figure this out? Is it sth you've bumped into before? I think I saw an issue opened for sth similar on the angular.js side but I can't find it...

@pkozlowski-opensource
Copy link
Member

There is a similar issue opened: angular/angular.js#906

@max-mykhailenko
Copy link
Author

Thanks to all, without using "templateUrl" all works good

@johlrich
Copy link

@pkozlowski-opensource I had seen/used a similar implementation of bs-tabs (which was just https://github.com/CaryLandholt/AngularFun/blob/master/src/scripts/directives/tabs.coffee#L32 minus the CoffeeScript) in my first sandbox apps when figuring out angular. Literally the only difference is back then I was using RequireJS's Text plugin to bundle my templates as strings rather than as templateUrl's so I never ran across this issue. Seeing that being the only diff made me try it out on plunker. Glad it helped.

@ajoslin
Copy link
Contributor

ajoslin commented Oct 17, 2012

Nice find @johlrich !

The question is how will we fix this?

@petebacondarwin
Copy link
Member

I have seen problems with templateUrl before and I wonder if actually there
is a bug in angular?

... sent from my tablet
On 17 Oct 2012 14:55, "Andy Joslin" notifications@github.com wrote:

Nice find @johlrich https://github.com/johlrich !

The question is how will we fix this?


Reply to this email directly or view it on GitHubhttps://github.com//issues/5#issuecomment-9527999.

@pkozlowski-opensource
Copy link
Member

Hmm, it is either a bug on the AngularJS side or some serious lack of knowledge on our side. To see what is going on we should try to prepare a minimal reproduce scenario.

But yeh, it is kind of strange that it works with the inlined template but not with the templateUrl, The only difference I can see is the additional call to $http.

@petebacondarwin
Copy link
Member

There is a huge difference in the code that runs between template and
templateUrl. They work in completely different ways, which is why I smell
a rat.

In the case of template, see compile.js - lines
588-625https://github.com/angular/angular.js/blob/master/src/ng/compile.js#L588,
apart from a bit of messing about with curly braces and copying over
attributes in the case of replace=true, it simple sets the html content of
element to the value of the template.
In the case of templateUrl, it see compile.js - lines
627-633https://github.com/angular/angular.js/blob/master/src/ng/compile.js#L627,
it calls compileTemplateUrlhttps://github.com/angular/angular.js/blob/master/src/ng/compile.js#L893,
which creates a whole new linking function for this element with lots of
stuff - even a comment about how part of it seems wrong.

On 17 October 2012 18:21, Pawel Kozlowski notifications@github.com wrote:

Hmm, it is either a bug on the AngularJS side or some serious lack of
knowledge on our side. To see what is going on we should try to prepare a
minimal reproduce scenario.

But yeh, it is kind of strange that it works with the inlined template but
not with the templateUrl, The only difference I can see is the additional
call to $http.


Reply to this email directly or view it on GitHubhttps://github.com//issues/5#issuecomment-9535879.

@pkozlowski-opensource
Copy link
Member

Yes! Same conclusion here, this seems to be a completely different path in
the code (!). Somehow saw it before while working on the ui-input
but completely forgotten. I guess that we will need core's team help
here...

In the meantime the question remains: do we "fix" it (by either using a
literal template instead of a url or by not creating isolated scopes) or
take it to the AngularJS team?

@pkozlowski-opensource
Copy link
Member

OK, so we are not the first one bumping into this issue:
https://groups.google.com/forum/?fromgroups=#!topic/angular/FotfCK_eaQo

and the jsFiddle that reproduces this: http://jsfiddle.net/WAN2e/1/
Clearly there is something going on with the combination of ng-repeat + templateUrl + transclude + @

There are number of issues for it already (with more jsFiddle examples):
angular/angular.js#1166
angular/angular.js#836

but honestly I can't pinpoint what is going on here :-(

@pkozlowski-opensource
Copy link
Member

Narrowed it down to a (almost) minimal reproduce scenario: http://jsfiddle.net/52nrQ/9/

It looks like it has nothing to do with transclusion, it is some combination of ng-repeat + @ + templateUrl
I don't know what you think but for me it qualifies as a bug in AngularJS....

@ajoslin
Copy link
Contributor

ajoslin commented Oct 21, 2012

We can narrow it down a bit more, even: http://jsfiddle.net/52nrQ/10/

@pkozlowski-opensource
Copy link
Member

So, does is qualify as an issue? I know that those guys are under pressure now and Misko will probably close this one but we can always try :-)

@petebacondarwin
Copy link
Member

This is so definitely an issue. I think they would listen even more if we
could provide a fix :-)

On 21 October 2012 17:07, Pawel Kozlowski notifications@github.com wrote:

So, does is qualify as an issue? I know that those guys are under pressure
now and Misko will probably close this one but we can always try :-)


Reply to this email directly or view it on GitHubhttps://github.com//issues/5#issuecomment-9644008.

@ajoslin
Copy link
Contributor

ajoslin commented Oct 21, 2012

Yes, we should try to fix it. Not sure how...!

@petebacondarwin
Copy link
Member

Here is a comprehensive plnk: http://plnkr.co/edit/xgVRAs?p=preview. It
includes AngularJS source as a local file so that we can have a hack on it.
Pete

On 21 October 2012 17:20, Andy Joslin notifications@github.com wrote:

Yes, we should try to fix it. Not sure how...!


Reply to this email directly or view it on GitHubhttps://github.com//issues/5#issuecomment-9644151.

@dandoyon
Copy link

I've been sniffing around today with this. And as I was refreshing browser, the titles showed up... and then remembered Petes comment 'Bizarrely, sometimes it does seem to interpolate ok'. To me it smells like an async problem and the tab title somehow gets lost in the shuffle and weirdly the content shows up.

@pkozlowski-opensource
Copy link
Member

OK guys, good news: the strange interpolation problem turned out to be a bug in AngularJS and was fixed today by Igor. Here is more info:

I've tested this on the accordion and the patch provided by Igor fixes the issue. We will just need to wait for the new AngularJS release now.

@dandoyon
Copy link

Sweet


From: Pawel Kozlowski notifications@github.com
To: angular-ui/bootstrap bootstrap@noreply.github.com
Cc: Dan Doyon dandoyon@yahoo.com
Sent: Thursday, October 25, 2012 11:05 AM
Subject: Re: [bootstrap] "Remove tab" problem (#5)

OK guys, good news: the strange interpolation problem turned out to be a bug in AngularJS and was fixed today by Igor. Here is more info:
* PR with the fix: angular/angular.js#1494
* Patched version of AngularJS: http://ci.angularjs.org/job/angular.js-igor/689/artifact/build/
I've tested this on the accordion and the patch provided by Igor fixes the issue.

Reply to this email directly or view it on GitHub.

@pkozlowski-opensource
Copy link
Member

Now when 1.0.3 was released we will be able to add proper title support for both tabs and accordion (!).

@ajoslin
Copy link
Contributor

ajoslin commented Nov 27, 2012

Yay :-)

@pkozlowski-opensource
Copy link
Member

I believe that this is fixed now after switching to 1.0.3!

codedogfish pushed a commit to codedogfish/angular-ui-bootstrap that referenced this issue Sep 15, 2015
chore(stuff): add grunt/bower/npm/karma infrastructure
kayhadrin pushed a commit to kayhadrin/bootstrap that referenced this issue Feb 8, 2016
…m release/0.13.4-1 to develop

* commit 'd8cf1e8f92f8a579642fbe3d9ad4e002cf29c93a':
  chore(package): release new custom version 0.13.4-1
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants