Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have created a script for going a multiple select option in Angular without using a select box that you have to hold control in order to select multiple items.

Demo: http://jsfiddle.net/qwertynl/xfw9f/

Code on GitHub

Here is my code:

Javascript:

var switcharoo = angular.module('switcharoo', []).directive('multiSelect', function(){
    return {
        restrict: 'E',
        scope: {
            items: '=',
            default: '=',
            leftTitle: '@',
            rightTitle: '@'
        },
        templateUrl: "switcharoo.html",
        link: function(scope)   {
            scope.switchItem = function(item)   {
                var index = scope.default.indexOf(item);
                if(index == -1) {
                    //add it in
                    scope.default.push(item);
                }
                else    {
                    //remove it
                    scope.default.splice(index, 1);
                }
            }
        }
    };
})

switcharoo.directive('switchitem', function() {
    return {
        restrict: 'E',
        scope: {
            value: '='
        },
        template: '<div>{{value}}</div>'
    };
});

HTML Template:

<style>
    .switchBox .entBox {
        overflow:auto;height:8em; width:190px;border:1px solid black;float:left;
        -webkit-user-select: none;
        -khtml-user-select: none;
        -moz-user-select: none;
        -ms-user-select: none;
        -o-user-select: none;
        user-select: none;
    }
    .switchBox .entBox div:hover {
        background-color: #908f8f;
    }
    .switchBox .eBox2.entBox {
        background-color: #ccc9c9;
    }
</style>

<label>
    <table>
        <tr>
            <th>{{leftTitle}}</th>
            <th>{{rightTitle}}</th>
        </tr>
        <tr class="switchBox">
            <td>
                <div class="entBox eBox1">
                    <switchitem ng-repeat="(key, value) in items" ng-if="default.indexOf(key) == -1" value="value" ng-click="switchItem(key)"></switchitem>
                </div>
            </td>
            <td>
                <div class="entBox eBox2">
                    <switchitem ng-repeat="(key, value) in items" ng-if="default.indexOf(key) > -1" value="value" ng-click="switchItem(key)"></switchitem>
                </div>
            </td>
        </tr>
    </table>
</label>

Is there anything I can do to make my code slicker?

share|improve this question
add comment

1 Answer 1

I really like your code, I only have 2 minor observations:

  • I understand the part where default has the default items. I am not sure that after initialization you should keep referring to the default variable ( from a naming perspective ), maybe call it selection instead of default ?

  • Related to the previous point, ng-if="default.indexOf(key) == -1" and ng-if="default.indexOf(key) > -1" only make sense during initialization. In my mind, it would be ideal to provide a isSelected( key ) that you can use for the ng-if. Not to mention that it will remove a > out of your HTML attribute and look better in Markup ;)

share|improve this answer
    
Yea, I realize now that it should be called "selected" and not default. I'll try using a function instead for the ng-if –  qwertynl Feb 6 at 19:47
    
Does this look better? jsfiddle.net/qwertynl/xfw9f/15 –  qwertynl Feb 6 at 19:51
    
If you go for slickness, then I would go for return ~scope.selected.indexOf(item); other than that, looks good to me. –  konijn Feb 6 at 19:54
    
What does the tilde (~) do? –  qwertynl Feb 6 at 19:54
    
Bitwise NOT, converts -1 to 0, convert anything else to not-zero, –  konijn Feb 6 at 19:56
show 3 more comments

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.