feat(ngRemove): directive to remove a portion of the DOM tree (similar to ngHide)

[OrenAvissar]

I added a directive called ngRemove that I think could be useful. This works similar to ngHide except instead of setting the display: none css it will remove the element from the DOM.

There is already a comment in the code about refactoring ngHide "//TODO(misko): refactor to remove element from the DOM". It seems to me ngShow/ngHide imply setting visibility rather than removing an element.

Some example cases when ngRemove is helpful:

When using bootstrap in the situation shown in the HTML snippet below the bootstrap css uses selectors like .btn-group > .btn:first-child and .btn-group > .btn:last-child to properly format a series of buttons. In this example if you just hide the last button #btnCategories using display: none the previous button #btnUpload won't be styled correctly -- the #btnCategories button needs to be removed from the DOM to trigger the :last-child selector on the #btnUpload button.

<div class="btn-group">
  <button id="btnHome" class="btn btn-primary"><i class="icon-home icon-white"></i> Home</button>
  <button id="btnUpload" class="btn btn-primary"><i class="icon-upload icon-white"></i> Upload Document</button>
  <button id="btnCategories" class="btn btn-primary"><i class="icon-edit icon-white"></i> Manage Categories</button>
</div>

When using alternate inputs in a form (depending on application settings) it seems cleaner to remove the unused alternates instead of hiding them (in some cases they may share the same name or id attributes). As an example, the code below will use radio inputs if the application is set to only allow a single selection and checkbox inputs if multiple selections are allowed.

<div id="answer-options" class="controls" ng-init="answerData={}">
  <label class="{{slideData.answers.selectone && 'radio' || 'checkbox'}}" ng-repeat="answerOption in slideData.answers.options">
    <input ng-remove="!slideData.answers.selectone" type="radio" ng-model="answerData.radio" value="{{$index}}" />
    <input ng-remove="slideData.answers.selectone" type="checkbox" ng-model="answerData[$index]" />
    {{answerOption.text}}
  </label>
</div>
<button type="button" ng-click="submitQuestion(answerData)" class="btn">{{slideData.submittext || "Submit"}}</button>

[ganarajpr]

This isnt two way binding isnt it? Why would I use this over ui-if from Angular UI?

[OrenAvissar]

There isn't two way binding -- it's a very simple directive that just removes the element from the DOM. ui-if would do the same thing. I proposed it here because I'm not using AngularUI in my project and think this directive belongs in the angular core (at the same level as ngShow/ngHide). I'd be interested in any ideas to improve this.

[olostan]

As I understand, there is no possibility to bring removed element back? That can cause problems: for example, ngResource returns empty object before it fetch action object from backend. So if you have something like

<div ng-remove='user.hasEmail'>{{user.mail}}</div>

this directive will remove this DIV without waiting until user would be loaded.

[ganarajpr]

@olostan Agreed. That was my point about it requiring two way data-binding.

[OrenAvissar]

Thanks @ganarajpr and @olostan for the feedback. I ported the implementation from ui-if in AngularUI (by @tigbro) to handle two-way binding. I still need to do more testing and update the unit test. Please let me know if anything else comes to mind.

var ngRemoveDirective = [function() {
  return {
    transclude: 'element',
    priority: 1000,
    terminal: true,
    restrict: 'A',
    compile: function (element, attr, transclude) {
      return function ($scope, $element, $attr) {
        var childElement, childScope;
        $scope.$watch($attr.ngRemove, function ngRemoveWatchAction(value) {
          if (childElement) {
            childElement.remove();
            childElement = undefined;
          }
          if (childScope) {
            childScope.$destroy();
            childScope = undefined;
          }
          if (!toBoolean(value)) {
            childScope = $scope.$new();
            transclude(childScope, function (clone) {
              childElement = clone;
              $element.after(clone);
            });
          }
        });
      }
    }
  }
}];

[ganarajpr]

@OrenAvissar But that is not a port of ui-if. That is a complete copy? I dont know why they havent moved ui-if to angular core.. But there might be a reason..

[OrenAvissar]

@ganarajpr Semantics aside, the purpose of this request is to create a starting point for the possibility of adding this functionality to angular, which may or may not be the final implementation. I think @tigbro did a fine job -- he gets all the credit.

[tbosch]

Hi,
well, credits where credits belong I just took parts of the ng-repeat directive to create the if directive. So the real credit goes to the AngularJS-team :-)
Tobias

[lrlopez]

I'd really like to have an ng-if directive right into the core. I know that you can use ui-if instead, but I think the are lot of general use-cases (i.e. CSS pseudo-elements like :first-child) that are worth those extra bytes.

[vojtajina]

This implementation looks good to me. I think we should add it to the core, as it's more useful than ng-switch to me.

Changing ng-hide/ng-show to remove the element from DOM is possible, but when removing the element, we should remove the scope as well (to save watchers). In order to do scope removing, ng-show/ng-hide has to create a new scope and that would totally break existing apps.

Should we drop off ng-switch ? Because really ng-switch is just a different syntax of this thing.

[ganarajpr]

I think dropping off ng-switch wouldnt be backward compatible. But you could mark it as deprecated or something. Having said that, I think ng-switch and ng-if are akin to switch and if in programming languages. Though one can use one of them to get the effect of another they are both used in different situations? So maybe there is a space for both?

[IgorMinar]

+1 for merging this after these changes:

• rename to ng-if
• in docs, clearly state that a new scope is created which could be a problem in some cases (when biding to primitives)
• in docs explain the difference when compared to ngShow/ngHide

[vojtajina]

• squash the whole thing into a single commit
• add @restrict A and @scope to the docs

[vojtajina]

• sign CLA

[OrenAvissar]

I think ng-show/ng-hide are good the way they are to enable reading/modifying the state of an element while it's hidden. I agree with @ganarajpr about ng-switch. It's capable of the same thing but used differently.

@IgorMinar, @vojtajina I'll work on those changes tonight. I signed the CLA when I submitted the pull request. I'll go through it again -- maybe I missed something.

[OrenAvissar]

I renamed the directive to ng-if and updated the documentation.

I noticed that similar directives have support for ng-animate so I added it to ng-if as well, along with the unit tests. It looks like the docs in animator.js would need to be updated too -- I figure the person working on it can update that. I can remove the animation part if preferred.

I also noted in the docs that besides creating a new scope the original compiled state is used whenever elements are recreated, so any DOM changes made after compilation will be lost when an element is removed. I also added a test case for this.

I'm still kind of new to git, so I can use some help for squashing the branch to a single commit without totally messing things up :). Would the best way be to clone the latest version of angular, apply the changes for ngIf, and then do a force push to this branch? I'm the only one working on this branch so reseting shouldn't be a problem.

Please let me know if I need to submit the CLA again.

[vojtajina]

Awesome @OrenAvissar you don't need to sign the CLA multiple times.

[petebacondarwin]

typo compliled -> compiled

[OrenAvissar]

Thanks, I'll fix the typo and update the commit.

[petebacondarwin]

Apart from the comments on the documentation this looks good to merge, yes?

[frolic]

👍

[OrenAvissar]

I updated the docs and included a section about binding to primitives in the parent scope. Hopefully it's clear. The link recommended by @petebacondarwin explains it well so I also linked it in the docs.

[petebacondarwin]

Landed as: 2f96fbd! Thanks Oren

[L42y]

Awesome, love this!

[OrenAvissar]

Great, thank you to everyone for contributing!