HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Am I using Angular directives correctly?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
correctlyangularusingdirectives

Problem

I'm coming from Backbone and trying to wrap my head around Angular for a new project and would like to get some feedback on whether I'm using it correctly or not.

Plunker

What I'm trying to accomplish is this: I have a list of items that can be very easily rendered with an ngRepeat. Each item starts off in an inactive state. When a user selects one of the items it enters into an expanded active state that shows some extra details and controls for working with the currently selected item. The markup for these two states is different enough that adding & removing some CSS classes won't cut it, so I have separate templates for each state.

The approach I've taken is encapsulating this functionality into a directive that can render the two templates. When the item is initially rendered the inactive template is used. When an item is selected (in the example, by checking a checkbox) then the active template is compiled and the resulting markup put into the element's HTML. When the checkbox is unchecked the same happens for the inactive template.

The ngShow/ngHide directives won't work for me in this case because there could potentially be lots of these things on the page and my understanding is that ngShow/ngHide will render the markup for both states and then show/hide based on some model property. Ultimately only one item will be active at any given time so rendering all that markup for each item only to display one at a time seems wasteful. I also wanted more control over the transition between the inactive & active states.

Is this solution something that would be considered idiomatic Angular or an egregious abuse of directives? Am I exposing myself to any bugs?

The directive:

```
module.directive("myDirective", ['$compile', '$http', '$templateCache', function($compile, $http, $templateCache) {

function getTemplate(isActive) {
var templateLoader,
baseUrl = 'templates/',
templateMap = {
active: 'checked.html',
i

Solution

If the only reason is to avoid a few more DOM elements, this sounds like an overkill to me. It is so much easier to use ng-show or hiding/showing depending on the class like it is done in TodoMVC:


    ...
     ... 
     ... 


Then in CSS:

#todo-list li.editing .view {
    display: none;
}
#todo-list li.editing .edit {
    display: block;
}


Also it does not feel right to do DOM manipulation with .html() jQuery-style, maybe use dynamic template property instead.

If having additional elements in DOM is really a concern,
there are also ng-if and ng-switch.
Though I personally prefer ng-show/ng-hide exactly because
this way I don't have to rebuild the DOM.

To put this into perspective, I have tried using ngRouter on my site 33hotels.com, which does rebuild the view every time the route is changed. That felt in considerable delay. Instead I have all my Views inside my DOM at all times and only hide/show them depending on the route. The performance difference is dramatic - it now feels instant.

Finally, both templates have common fragment, which does not make code feel DRY. Upon any change you'll have to edit each of them. I would simply keep the common part outside any directive, and put the dynamic part inside ng-whatever.

Also note that the HTML tag `` has semantic meaning, not presentational. That is, it marks your main header. Any presentation adjustment should be done via CSS.

Code Snippets

<li ng-class="{editing: todo == editedTodo}">
    ...
    <div class="view"> ... </div>
    <input class="edit"> ... </div>
</li>
#todo-list li.editing .view {
    display: none;
}
#todo-list li.editing .edit {
    display: block;
}

Context

StackExchange Code Review Q#51438, answer score: 2

Revisions (0)

No revisions yet.