This is my first angular code. I've been working with jQuery for a while, so I don't have the same approach. I'm looking for advice and code improvement.
The following code has 3 directives:
ceBoxCreator
: Will create a div that will be draggable and resizable on a click.ceDrag
: The element binded to this directive will be draggable.ceResize
: The element binded to this directive will be resizable.
(function() {
var contentEditor = angular.module("contentEditor", []);
// To create a empty resizable and draggable box
contentEditor.directive("ceBoxCreator", function($document, $compile) {
return {
restrict: 'A',
link: function($scope, $element, $attrs) {
$element.on("mousedown", function($event) {
var newNode = $compile('<div class="contentEditorBox" ce-drag ce-resize></div>')($scope);
newNode.css({
position: "absolute",
top: $event.pageY - 25 + "px",
left: $event.pageX - 25 + "px",
});
angular.element($document[0].body).append(newNode);
});
}
}
});
// To manage the drag
contentEditor.directive("ceDrag", function($document) {
return function($scope, $element, $attr) {
var startX = 0, startY = 0;
var newElement = angular.element('<div class="draggable"></div>');
$element.append(newElement);
newElement.on("mousedown", function($event) {
$event.preventDefault();
// To keep the last selected box in front
angular.element(document.querySelectorAll(".contentEditorBox")).css("z-index", "0");
$element.css("z-index", "1");
startX = $event.pageX - $element[0].offsetLeft;
startY = $event.pageY - $element[0].offsetTop;
$document.on("mousemove", mousemove);
$document.on("mouseup", mouseup);
});
function mousemove($event) {
y = $event.pageY - startY;
x = $event.pageX - startX;
$element.css({
top: y + "px",
left: x + "px"
});
}
function mouseup() {
$document.off("mousemove", mousemove);
$document.off("mouseup", mouseup);
}
};
});
// To manage the resize
contentEditor.directive("ceResize", function($document) {
return function($scope, $element, $attr) {
// Function to manage resize up event
var resizeUp = function($event) {
var top = $event.pageY;
var height = $element[0].offsetTop + $element[0].offsetHeight - $event.pageY;
if ($event.pageY < $element[0].offsetTop + $element[0].offsetHeight - 50) {
$element.css({
top: top + "px",
height: height + "px"
});
} else {
$element.css({
top: $element[0].offsetTop + $element[0].offsetHeight - 50 + "px",
height: "50px"
});
}
};
// Function to manage resize right event
var resizeRight = function($event) {
var width = $event.pageX - $element[0].offsetLeft;
if ($event.pageX > $element[0].offsetLeft + 50) {
$element.css({
width: width + "px"
});
} else {
$element.css({
width: "50px",
});
}
};
// Function to manage resize down event
var resizeDown = function($event) {
var height = $event.pageY - $element[0].offsetTop;
if ($event.pageY > $element[0].offsetTop + 50) {
$element.css({
height: height + "px"
});
} else {
$element.css({
height: "50px"
});
}
};
// Function to manage resize left event
var resizeLeft = function($event) {
var left = $event.pageX;
var width = $element[0].offsetLeft + $element[0].offsetWidth - $event.pageX;
if ($event.pageX < $element[0].offsetLeft + $element[0].offsetWidth - 50) {
$element.css({
left: left + "px",
width: width + "px"
});
} else {
$element.css({
left: $element[0].offsetLeft + $element[0].offsetWidth - 50 + "px",
width: "50px"
});
}
};
// Create a div to catch resize up event
var newElement = angular.element('<div class="n-resize"></div>');
$element.append(newElement);
newElement.on("mousedown", function() {
$document.on("mousemove", mousemove);
$document.on("mouseup", mouseup);
function mousemove($event) {
$event.preventDefault();
resizeUp($event);
}
function mouseup() {
$document.off("mousemove", mousemove);
$document.off("mouseup", mouseup);
}
});
// Create a div to catch resize right event
newElement = angular.element('<div class="e-resize"></div>');
$element.append(newElement);
newElement.on("mousedown", function() {
$document.on("mousemove", mousemove);
$document.on("mouseup", mouseup);
function mousemove($event) {
$event.preventDefault();
resizeRight($event);
}
function mouseup() {
$document.off("mousemove", mousemove);
$document.off("mouseup", mouseup);
}
});
// Create a div to catch resize down event
newElement = angular.element('<div class="s-resize"></div>');
$element.append(newElement);
newElement.on("mousedown", function() {
$document.on("mousemove", mousemove);
$document.on("mouseup", mouseup);
function mousemove($event) {
$event.preventDefault();
resizeDown($event);
}
function mouseup() {
$document.off("mousemove", mousemove);
$document.off("mouseup", mouseup);
}
});
// Create a div to catch resize left event
newElement = angular.element('<div class="w-resize"></div>');
$element.append(newElement);
newElement.on("mousedown", function() {
$document.on("mousemove", mousemove);
$document.on("mouseup", mouseup);
function mousemove($event) {
$event.preventDefault();
resizeLeft($event);
}
function mouseup() {
$document.off("mousemove", mousemove);
$document.off("mouseup", mouseup);
}
});
// Create a div to catch resize up left event
newElement = angular.element('<div class="nw-resize"></div>');
$element.append(newElement);
newElement.on("mousedown", function() {
$document.on("mousemove", mousemove);
$document.on("mouseup", mouseup);
function mousemove($event) {
$event.preventDefault();
resizeUp($event);
resizeLeft($event);
}
function mouseup() {
$document.off("mousemove", mousemove);
$document.off("mouseup", mouseup);
}
});
// Create a div to catch resize up right event
newElement = angular.element('<div class="ne-resize"></div>');
$element.append(newElement);
newElement.on("mousedown", function() {
$document.on("mousemove", mousemove);
$document.on("mouseup", mouseup);
function mousemove($event) {
$event.preventDefault();
resizeUp($event);
resizeRight($event);
}
function mouseup() {
$document.off("mousemove", mousemove);
$document.off("mouseup", mouseup);
}
});
// Create a div to catch resize down right event
newElement = angular.element('<div class="se-resize"></div>');
$element.append(newElement);
newElement.on("mousedown", function() {
$document.on("mousemove", mousemove);
$document.on("mouseup", mouseup);
function mousemove($event) {
$event.preventDefault();
resizeDown($event);
resizeRight($event);
}
function mouseup() {
$document.off("mousemove", mousemove);
$document.off("mouseup", mouseup);
}
});
// Create a div to catch resize down left event
newElement = angular.element('<div class="sw-resize"></div>');
$element.append(newElement);
newElement.on("mousedown", function() {
$document.on("mousemove", mousemove);
$document.on("mouseup", mouseup);
function mousemove($event) {
$event.preventDefault();
resizeDown($event);
resizeLeft($event);
}
function mouseup() {
$document.off("mousemove", mousemove);
$document.off("mouseup", mouseup);
}
});
};
});
})();
To see what the code does: jsFiddle
For example:
There is code repetition on the
ceResize
directive of themousemove
andmouseup
event. If I put this function at the root of the directive, it cannot be accessed because it's not the correct scope.There is HTML code in the JS file. Is it possible to extract it and keep the events available?
I've heard of things like
injector
,scope
,link
(in directives),templateURL
, etc... But those are still vague concepts to me. And it seems like there is multiple ways to do the same thing. But is there "best ways" to use Angular ?Also why is there two different prototypes for a directive :
One with a function that returns a JSON object :
module.directive(directiveName, function(/* Some attributes (services?) */) {
return {
/* Some JSON attributes */
}
});
And another one that returns a function :
module.directive(directiveName, function(/* Some attributes (services?) */) {
return function(/* Some attributes (services?) */) {
/* Some attributes and functions to handle events */
}
});
This are the points I can think of, but feel free to review the code.
Note: I don't know why using JSFiddle the resize up and left are weird. It's working great on my computer. Anyway, this isn't the point here, just help me to understand how we are supposed to code using AngularJS.
-
1\$\begingroup\$ Great question, just wanted to say that I see the same behaviour on jsbin.com, so I suspect your code is at fault, not jsfiddle. jsbin.com/sopapo/1/edit \$\endgroup\$konijn– konijn2014年09月10日 19:58:27 +00:00Commented Sep 10, 2014 at 19:58
-
\$\begingroup\$ this jfiddle do not work in ff 44.0.2 \$\endgroup\$user98482– user984822016年02月23日 11:03:14 +00:00Commented Feb 23, 2016 at 11:03
-
\$\begingroup\$ @ghdj My bad, I updated the JSFiddle. \$\endgroup\$Elfayer– Elfayer2016年02月23日 11:28:41 +00:00Commented Feb 23, 2016 at 11:28
1 Answer 1
From a once over:
- It makes more sense from a UX experience to create the box on
click
then onmousedown
- There are a number of indenting inconsistencies, use TidyUp in jsFiddle to fix that
- Naming is good
- Flow of the code is good
- Commenting is good
- I don't like the constants
25
and50
all over the place, name them and manage them You have a few place where you can apply DRY
You set the
top
andleft
a number of times, simply create a helper functionfunction placeNode(node, top, left) { node.css({ position: "absolute", top: top + "px", left: left + "px", }); }
You set css values a number of times by comparing values
var width = $event.pageX - $element[0].offsetLeft; if ($event.pageX > $element[0].offsetLeft + 50) { $element.css({ width: width + "px" }); } else { $element.css({ width: "50px", }); }
It is better to first deduce the value of
width
and then set it in 1 go:var margin = 50, leftest = $element[0].offsetLeft + margin, width = $event.pageX > leftest ? $event.pageX - $element[0].offsetLeft : margin; $element.css({ width: width + "px" });
- There is a ton of copy pastage when the resize divs are generated, use 1 common function that can generate a div with a class name and event handler(s).
All in all I thought your code looked okay, if repetitive, so I took a stab at both fixing your code ( for upper and left resize ) and making it less repetitive ( you can find this in action here ):
(function() {
var contentEditor = angular.module("contentEditor", []);
function placeNode(node, top, left) {
node.css({
position: "absolute",
top: top + "px",
left: left + "px",
});
}
// To create a empty resizable and draggable box
contentEditor.directive("ceBoxCreator", function($document, $compile) {
return {
restrict: 'A',
link: function($scope, $element, $attrs) {
$element.on("click", function($event) {
var newNode = $compile('<div class="contentEditorBox" ce-drag ce-resize></div>')($scope);
placeNode(newNode, $event.pageY - 25, $event.pageX - 25);
angular.element($document[0].body).append(newNode);
});
}
}
});
// To manage the drag
contentEditor.directive("ceDrag", function($document) {
return function($scope, $element, $attr) {
var startX = 0,
startY = 0;
var newElement = angular.element('<div class="draggable"></div>');
$element.append(newElement);
newElement.on("mousedown", function($event) {
event.preventDefault();
// To keep the last selected box in front
angular.element(document.querySelectorAll(".contentEditorBox")).css("z-index", "0");
$element.css("z-index", "1");
startX = $event.pageX - $element[0].offsetLeft;
startY = $event.pageY - $element[0].offsetTop;
$document.on("mousemove", mousemove);
$document.on("mouseup", mouseup);
});
function mousemove($event) {
placeNode( $element , $event.pageY - startY , $event.pageX - startX );
}
function mouseup() {
$document.off("mousemove", mousemove);
$document.off("mouseup", mouseup);
}
};
});
// To manage the resizers
contentEditor.directive("ceResize", function($document) {
return function($scope, $element, $attr) {
//Reference to the original
var $mouseDown;
// Function to manage resize up event
var resizeUp = function($event) {
var margin = 50,
lowest = $mouseDown.top + $mouseDown.height - margin,
top = $event.pageY > lowest ? lowest : $event.pageY,
height = $mouseDown.top - top + $mouseDown.height;
$element.css({
top: top + "px",
height: height + "px"
});
};
// Function to manage resize right event
var resizeRight = function($event) {
var margin = 50,
leftest = $element[0].offsetLeft + margin,
width = $event.pageX > leftest ? $event.pageX - $element[0].offsetLeft : margin;
$element.css({
width: width + "px"
});
};
// Function to manage resize down event
var resizeDown = function($event) {
var margin = 50,
uppest = $element[0].offsetTop + margin,
height = $event.pageY > uppest ? $event.pageY - $element[0].offsetTop : margin;
$element.css({
height: height + "px"
});
};
// Function to manage resize left event
function resizeLeft ($event) {
var margin = 50,
rightest = $mouseDown.left + $mouseDown.width - margin,
left = $event.pageX > rightest ? rightest : $event.pageX,
width = $mouseDown.left - left + $mouseDown.width;
$element.css({
left: left + "px",
width: width + "px"
});
};
var createResizer = function createResizer( className , handlers ){
newElement = angular.element( '<div class="' + className + '"></div>' );
$element.append(newElement);
newElement.on("mousedown", function($event) {
$document.on("mousemove", mousemove);
$document.on("mouseup", mouseup);
//Keep the original event around for up / left resizing
$mouseDown = $event;
$mouseDown.top = $element[0].offsetTop;
$mouseDown.left = $element[0].offsetLeft
$mouseDown.width = $element[0].offsetWidth;
$mouseDown.height = $element[0].offsetHeight;
function mousemove($event) {
event.preventDefault();
for( var i = 0 ; i < handlers.length ; i++){
handlers[i]( $event );
}
}
function mouseup() {
$document.off("mousemove", mousemove);
$document.off("mouseup", mouseup);
}
});
}
createResizer( 'sw-resize' , [ resizeDown , resizeLeft ] );
createResizer( 'ne-resize' , [ resizeUp , resizeRight ] );
createResizer( 'nw-resize' , [ resizeUp , resizeLeft ] );
createResizer( 'se-resize' , [ resizeDown , resizeRight ] );
createResizer( 'w-resize' , [ resizeLeft ] );
createResizer( 'e-resize' , [ resizeRight ] );
createResizer( 'n-resize' , [ resizeUp ] );
createResizer( 's-resize' , [ resizeDown ] );
};
});
})();
-
\$\begingroup\$ But where exactly was the bug? Also, have you seen this? \$\endgroup\$RubberDuck– RubberDuck2014年09月11日 14:19:01 +00:00Commented Sep 11, 2014 at 14:19
-
1\$\begingroup\$ @RubberDuck The bug is ignoring the original mousedown event. I would do something fancier for that challenge ;) \$\endgroup\$konijn– konijn2014年09月11日 14:24:33 +00:00Commented Sep 11, 2014 at 14:24
-
\$\begingroup\$ Thanks for answering ! Do you have some advice about the architecture of the directives (directive prototype, link, scope, templateUrl, etc) ? \$\endgroup\$Elfayer– Elfayer2014年09月12日 15:30:38 +00:00Commented Sep 12, 2014 at 15:30
-
\$\begingroup\$ @Elfayer I am no great Angular expert, all I can tell you is that your Angular code is understandable/maintainable and has no obvious flaws. \$\endgroup\$konijn– konijn2014年09月14日 14:44:53 +00:00Commented Sep 14, 2014 at 14:44