Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I need some advice on writing clean code for an Angular directive. I feel things could be written better, and would love some feedback and suggestions.

'use strict';

angular.module('aCollectiveApp.dashboard')
    .directive('adminDashboard', ($document) => {
        return {
            restrict: 'EA',
            link: (scope, elem, attrs) => {

                let dropdownElem = angular.element(elem.parent().children()[2]),
                    isOpen = false;

                elem.bind('click', (e) => {

                    e.stopPropagation();
                    isOpen = true;

                    if (dropdownElem.hasClass('active')) {
                        dropdownElem.removeClass('active');
                    } else {
                        dropdownElem.addClass('active');
                    }
                });

                $document.bind('click', () => {
                    if (isOpen) {
                        dropdownElem.removeClass('active');
                    } else {
                        isOpen = false;
                    }
                })
            }
        };
    })

.directive('circleMenu', () => {
    return {
        restrict: 'EA',
        link: (scope, elem, attrs) => {

            let menu = angular.element(elem.parent().children()[0]);

            elem.bind('mouseenter', () => {

                // Slide Out Button
                elem.addClass('fade-out');

                // Slide In Menu
                menu.addClass('fade-in');
            })

            menu.bind('mouseleave', () => {
                elem.removeClass('fade-out');
                menu.removeClass('fade-in');
            });

        }
    }
})
share|improve this question
    
I have small comments but it's related to using jquery, you can use toggleClass instead of checking for presence of class, and hover instead of mouseenter & mouseleave also you can use removeClass without checking as it has no side effect if no class found – Ihab Khattab Apr 22 at 15:50
    
thank @ihabkhattab, though i'm just trying to avoid using jQuery at the moment, only jQlite that Angular provides – anon Apr 22 at 18:28

Your Answer

 
discard

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

Browse other questions tagged or ask your own question.