4
\$\begingroup\$

I changed the normal way of using Angular. I'm using my controllers just to talk to my API routes and am using directives to manipulate the data and DOM as well.

I am using the object literal pattern where I have my INIT method that initializes global variables (object) and then calls other methods, bind clicks methods, etc. So, the logic is all the directives.

I'm finding it very confusing and would like the opinion of someone better.

'use strict';
var eventSidebarDirective = {
 /**
 * Initialize sidebar directive and return the link
 * calling all nested methods.
 *
 */
 init: function() {
 return {
 link: function(scope) {
 scope.$watch('events', function() {
 if (scope.events === undefined) {
 return;
 }
 /**
 * Every time the user access the event page, this methods
 * will be called.
 *
 */
 EventSidebar.init();
 });
 },
 restrict: 'M'
 };
 }
};
var EventSidebar = {
 init: function() {
 this.element = $('[data-event-sidebar]');
 this.indicator = $('[data-event-sidebar-to]');
 /**
 * To manipulate the element, bind a click in the indicator.
 *
 */
 this.indicator.bind('click', this._toggle.bind(this));
 },
 _toggle: function(e) {
 var state = $(e.currentTarget).data('event-sidebar-to');
 this.element
 .css({
 /**
 * If the window width is less than 768px, i just want toggle
 * between display block and none.
 *
 */
 display: 768 >= $(window).width()
 ? (state
 ? 'block'
 : 'none')
 : false
 })
 .animate({
 /**
 * But if the window.width is bigger the 768px, i want show
 * or hide the sidebar.
 *
 */
 right: 768 <= $(window).width()
 ? (state
 ? '0'
 : '-500')
 : false
 });
 }
};
angular
 .module('adminApp')
 .directive('eventSidebarDirective', eventSidebarDirective.init.bind(eventSidebarDirective));
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Aug 20, 2015 at 0:04
\$\endgroup\$
0

1 Answer 1

3
\$\begingroup\$

Yes, you're right. Your code is very hard to read.

  1. I think you don't need that init stuff. Just remove it and add initialization to the controller.
  2. You don't need jQuery for your directive. This is possible with Angular JS / pure Js. Use ng-show or ng-if for showing hiding your sidebar and use ng-class to add your class conditionally depending on a variable from your controller to do things differently for large screens.
  3. Animation could be done with ngAnimate and animate.css.

There are some points that could be improved in my code:

  • Add options so it's better reusable.
  • Add default options for example start visible = false. with angular.module('adminApp').constants(...) then you can angular.extend(...) these in the controller of your directive.
  • Add an isolated scope for the directive so it can't modify the scope of your app. Somthing like scope: {} would be OK or {scope: { options: '@' } } for the sidebar options.
  • Remove restrict: M because it's not recommended to use directives as comments (see angular docs).

Please have a look at the demo below or in this jsfiddle.

function EventSidebarDirective($window) {
 var directive = {
 restrict: 'EAM',
 templateUrl: 'components/sidebar/template.html',
 controllerAs: 'ctrl',
 controller: function ($scope) {
 var self = this, // self needed because toggle is not bound to this ctrl.
 	screenWidth;
 
 angular.extend(this, {
 	visible: false,
 	toggle: function () {
 screenWidth = $window.innerWidth; //$(window).width();
 self.visible = !self.visible; // toggle visibility
 self.largeScreen = ( screenWidth >= 768 );
				}
 });
 },
 link: function (scope, element, attr, ctrl) {
			 
 }
 };
 return directive;
}
EventSidebarDirective.$inject = ['$window']; // only need with-out ng-annotate for minification
angular.module('adminApp', ['ngAnimate'])
 .directive('eventSidebar', EventSidebarDirective);
.sidebar-content {
 width: 200px;
 height: 200px;
 background: lightgray;
}
.sidebar-content-animation.ng-hide-remove {
 -webkit-animation: fadeInLeft 1s;
 -moz-animation: fadeInLeft 1s;
 -ms-animation: fadeInLeft 1s;
 -o-animation: fadeInLeft 1s;
 animation: fadeInLeft 1s;
}
.sidebar-content-animation.ng-hide-add {
 -webkit-animation: fadeOutUp 1s;
 -moz-animation: fadeOutUp 1s;
 -ms-animation: fadeOutUp 1s;
 -o-animation: fadeOutUp 1s;
 animation: fadeOutUp 1s;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/angular.js/1.4.4/angular.js"></script>
<link href="https://cdnjs.cloudflare.com/ajax/libs/animate.css/3.4.0/animate.min.css" rel="stylesheet"/>
<script src="https://cdnjs.cloudflare.com/ajax/libs/angular.js/1.4.4/angular-animate.js"></script>
<div ng-app="adminApp">
 <script type="text/ng-template" id="components/sidebar/template.html">
 <div class="sidebar-content" 
 ng-show="ctrl.visible" 
 ng-class="{'sidebar-content-animation': ctrl.largeScreen}">
 I'm a sidebar
 </div>
 <a href="#" ng-click="ctrl.toggle()">toggle sidebar</a>
 </script>
 <event-sidebar></event-sidebar>
</div>

answered Aug 21, 2015 at 16:33
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.