Combining attributes with expressions (e.g. ng-show conditions) in a directive with replace = true fails

[koningskristof]

If both the div in the parent view does have a ng-show and the div of a directive with replace = true does have a ng-show, those conditions are bad merged.

Bug in action: http://jsfiddle.net/koningskristof/x69qwm8q/1/

[pkozlowski-opensource]

Yeh, here is the reproduce scenario with the latest AngularJS: http://jsfiddle.net/0usmoyjm/
So, this is one more manifestation of a corner-case created by replace: true - this is why it was depreciated and people are discouraged from using this attribute.

I'm afraid that this might be won't fix in 1.x as I'm not even sure what should be a semantic of a merged thing.

A side note: having attribute directives that got they own template isn't that common, I guess this should be an element directive.

My best advice here: turn it into an element directive and remove replace: true.

[caitp]

attribute merging: it breaks things.

Sadly, this is not likely to be fixed in 1.x. However, there is a solution to this issue in angular 2, which is putting this stuff in the "template" attribute. The API for that is awful, but we could conceivably do it in 1.x with ng-tpl-<<attrName>> putting <<attrName>> into the replace template attributes rather than merging them. You'd still have some issues, but it could work.

[gkalpak]

You'd still have some issues, but it could work.

@caitp: Unrelated to this issue, but I proposed this approach on a similar issue with ngMaterial (angular/material#848 (comment)). Do you have any specific issues in mind that need to be considered ?

[caitp]

@gkalpak the only issue I can think of with it is where you have 3 levels of component directives, and each one uses the own-attribute and template-attribute notations, I think you'd still end up with merging in those cases

[frfancha]

Merging attributes when these ones have strings to be evaluated in scope can't be correct anyway.
The most typical/useful examples I can think of are ngHide and ngIf.
The ngHide (or ngIf) in the parent document must be evaluated in the parent scope, and the ngHide (or ngIf) from the template must be evaluated in the scope of the directive.
They can't be "merged", they are two different things.
As recommended by caitp, we have decided to stop using replace=true, then the semantic is clear and correct.

[angrytoro]

maybe it caused by the code

scope.$watch(attr.ngShow, function ngShowWatchAction(value) {
        // we're adding a temporary, animation-specific class for ng-hide since this way
        // we can control when the element is actually displayed on screen without having
        // to have a global/greedy CSS selector that breaks when other animations are run.
        // Read: https://github.com/angular/angular.js/issues/9103#issuecomment-58335845
        $animate[value ? 'removeClass' : 'addClass'](element, NG_HIDE_CLASS, {
          tempClasses: NG_HIDE_IN_PROGRESS_CLASS
        });
});

the value is always a string. so $animate[value ? 'removeClass' : 'addClass'] always to be $animate['removeClass']

[matsko]

@angrytoro the code that you posted is to prevent other animations from overriding the hide style when the .ng-hide class is present on the element.

[Narretz]

Essentially the same as #5695