Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

ngShow in directive with isolate scope is never shown #2500

Closed
arunjitsingh opened this issue Apr 24, 2013 · 29 comments
Closed

ngShow in directive with isolate scope is never shown #2500

arunjitsingh opened this issue Apr 24, 2013 · 29 comments

Comments

@arunjitsingh
Copy link

In a directive with an isolate scope with ng-show="someVar" (someVar from a parent ng-controller's $scope), the directive's tag is never shown ("display: none" isn't removed) (example). Other directives (like ng-repeat) bind values correctly in custom directives with isolate scope.

@chrisnicola
Copy link
Contributor

If I had to guess exactly the problem it's because you have an isolate scope so it expects that ng-show will be bound inside the directive template and not outside of it.

So as a quick-fix the following works if you put it inside your directive template instead:

template: '<div class="item" ng-transclude ng-show="list.length"></div>'

However the whole point of the isolate scope is to isolate yourself from any direct bindings to the parent scope and instead pass them in. So arguably you should have something more like:

scope: { show: '=' }
template: '<div class="item" ng-transclude ng-show="show"></div>'

// And in the HTML call it like this:

<div my-directive show="list.length">

As for why ng-repeat is working fine, if I had to hazard another guess it is because it is part of the transclusion step. Since you are transcluding inside myDirective it needs to determining what content to transclude in advance. Thus ng-repeat is not happening inside the directive scope but outside of it before being injected into the template through the transclusion step.

I do agree the behaviour is a bit non-intuitive, and I wonder if it wouldn't be better to ensure that binding to the parent scope always "just worked" regardless of an isolate scope or not. As a rule of thumb I try to design my directives to be completely ignorante (isolated) from the controller/scope that contains them. If necessary I'll write a controller inside the directive itself to manage the internal state of the directive.

@shairez
Copy link
Contributor

shairez commented May 15, 2013

Agree, especially when you have multiple nested components, where you have to declare the "show" property on each one of them...

So maybe when angular see an element directives with attribute directives, it should transclude just them..

Something like:

<my-panel ng-show="someVar"></my-panel>

Before my-panel's template is compiled, it should transclude the other attributes on the HTML declaration.

Is that possible?

@robinboehm
Copy link
Contributor

"I do agree the behaviour is a bit non-intuitive"
Me too, that dont feels right.
I understand the technical background, but for from the developers point of view it should be possible.

There is also a Google Group for this:
https://groups.google.com/forum/#!msg/angular/FqvC0ciG08w/k7KkyJu7zNoJ

Lets find a solution...

@0x-r4bbit
Copy link
Contributor

@shairez Unfortunately in that case, you have to wrap it in a div which has the ng-show directive.

@tam4s
Copy link

tam4s commented Jun 13, 2013

+1 to support ng-show on custom directives with isolated scope

@gaevoy
Copy link

gaevoy commented Jun 15, 2013

+1 It would be helpful somewhat declare inside directive statement that you want to use ng-show ng-hide on it.

@jrencz
Copy link

jrencz commented Jun 23, 2013

+1

@keir
Copy link
Contributor

keir commented Jun 24, 2013

+1; I just ran into this.

@kevinsbennett
Copy link

+1... I don't think anybody placing attribute expressions on an element directive (at the point of use) is wanting them to run in the context of the isolate scope.

@jyboudreau
Copy link

+1

3 similar comments
@amorphius
Copy link

+1

@jim-cooper
Copy link

+1

@rob-balfre
Copy link

+1

@scamden
Copy link

scamden commented Jul 12, 2013

seems unnecessary to mention but i also would intuitively expect the isolate scope not to affect the other directives

@arvearve
Copy link

👍

2 similar comments
@chiefy
Copy link

chiefy commented Jul 23, 2013

+1

@devmondo
Copy link

👍

@Foxandxss
Copy link
Member

I agree with all of you, but if we had an isolated scope per attribute instead of the whole element, I am not sure of how easy would be having directives with options (via other attributes).

I think that trying to do the hell of workarounds just for avoiding an extra div is overkill.

@scamden
Copy link

scamden commented Aug 7, 2013

But it's not just to avoid an extra div, how about trying to set focus into an input element that has type ahead. It's really hard to do it not hackily if you're using isolate scope. I guess the point is this affects more than just Ng-show

Sent from Mailbox for iPhone

On Wed, Aug 7, 2013 at 2:36 AM, Foxandxss notifications@github.com
wrote:

I agree with all of you, but if we had an isolated scope per attribute instead of the whole element, I am not sure of how easy would be having directives with options (via other attributes).

I think that trying to do the hell of workarounds just for avoiding an extra div is overkill.

Reply to this email directly or view it on GitHub:
#2500 (comment)

@Foxandxss
Copy link
Member

Ah!, you got me 👍

@chrisnicola
Copy link
Contributor

I'm wondering if simply providing a way to reference to the parent scope (even though it technically isn't a parent) would help. $outerScope or $externalScope or something to indicate it isn't a parent in the inheritance sense.

@Foxandxss
Copy link
Member

@chrisnicola something like $parent.foo ?

@sveilleux
Copy link

+1

@spamdaemon
Copy link

This one keeps driving me nuts - I keep running into this and the only way to fix it is to add another div or make the scope not-isolated.

@scottmboring
Copy link

Adding the following into the link function solves the problem. It's an extra step for the component creator, but makes the component more intuitive.

    function link($scope, $element, attr, ctrl) {

        if (attr.hasOwnProperty("ngShow")) {
            $scope.$watch("ngShow", function (value) {
                if (value) {
                    $element.show();
                }
                else {
                    $element.hide();
                }
            });
        }

You'll also need to setup scope bindings for ngShow

        scope: {
            ngShow: "="
        }

@rgraffconnect
Copy link

Other languages use OOP inheritance to solve issues like this, for example, if you had BaseComponent and that had a property called show, then in CustomComponent, which extends BaseComponent, show would already exist. And... you could set the value of show from an external "scope". Maybe directives with isolate scopes should automatically inherit the "ng" attributes unless otherwise specified in the directive. That's how I might solve this problem.

@rgraffconnect
Copy link

OH yeah and... +1 ;)

@kubino
Copy link

kubino commented Oct 28, 2013

and another +1

btford added a commit that referenced this issue Nov 8, 2013
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
Fixes issue with isolate scope leaking all over the place into other directives on the same element.

Isolate scope is now available only to the isolate directive that requested it and its template.

A non-isolate directive should not get the isolate scope of an isolate directive on the same element,
instead they will receive the original scope (which is the parent scope of the newly created isolate scope).

Paired with Tobias.

BREAKING CHANGE: Directives without isolate scope do not get the isolate scope from an isolate directive on the same element. If your code depends on this behavior (non-isolate directive needs to access state from within the isolate scope), change the isolate directive to use scope locals to pass these explicitly.

// before
<input ng-model="$parent.value" ng-isolate>

.directive('ngIsolate', function() {
  return {
    scope: {},
    template: '{{value}}'
  };
});

// after
<input ng-model="value" ng-isolate>

.directive('ngIsolate', function() {
  return {
    scope: {value: '=ngModel'},
    template: '{{value}}
  };
});

Closes angular#1924
Closes angular#2500
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
Fixes issue with isolate scope leaking all over the place into other directives on the same element.

Isolate scope is now available only to the isolate directive that requested it and its template.

A non-isolate directive should not get the isolate scope of an isolate directive on the same element,
instead they will receive the original scope (which is the parent scope of the newly created isolate scope).

Paired with Tobias.

BREAKING CHANGE: Directives without isolate scope do not get the isolate scope from an isolate directive on the same element. If your code depends on this behavior (non-isolate directive needs to access state from within the isolate scope), change the isolate directive to use scope locals to pass these explicitly.

// before
<input ng-model="$parent.value" ng-isolate>

.directive('ngIsolate', function() {
  return {
    scope: {},
    template: '{{value}}'
  };
});

// after
<input ng-model="value" ng-isolate>

.directive('ngIsolate', function() {
  return {
    scope: {value: '=ngModel'},
    template: '{{value}}
  };
});

Closes angular#1924
Closes angular#2500
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
@stsvilik
Copy link

Would it be more elegant to have another type of scope declaration that would allow access to isolate scope it it already exists or original scope if it doesn't?

app.directive('MyDir', function(){
return {
scope: 'inherit' //This would grab any scope available on the element already
};
});

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