Permalink
Browse files

Some small JSHint fixes

  • Loading branch information...
arian authored and cpojer committed Feb 24, 2011
1 parent 749af0f commit af48c8d589f43f32212f9bb8ff68a127e6a3ba6c
Showing with 13 additions and 12 deletions.
  1. +6 −6 Source/Element/Element.Dimensions.js
  2. +7 −6 Source/Types/Event.js
@@ -218,28 +218,28 @@ var styleString = Element.getComputedStyle;
function styleNumber(element, style){
return styleString(element, style).toInt() || 0;
-};
+}
function borderBox(element){
return styleString(element, '-moz-box-sizing') == 'border-box';
-};
+}
function topBorder(element){
return styleNumber(element, 'border-top-width');
-};
+}
function leftBorder(element){
return styleNumber(element, 'border-left-width');
-};
+}
function isBody(element){
return (/^(?:body|html)$/i).test(element.tagName);
-};
+}
function getCompatElement(element){
var doc = element.getDocument();
return (!doc.compatMode || doc.compatMode == 'CSS1Compat') ? doc.html : doc.body;
-};
+}
}).call(this);
View
@@ -23,12 +23,14 @@ var Event = new Type('Event', function(event, win){
var type = event.type,
target = event.target || event.srcElement,
page = {},
- client = {};
+ client = {},
+ related = null,
+ rightClick, wheel, code, key;
@kassens

kassens Feb 24, 2011

Member

Shouldn't we leave collecting variables to the beginning of the function up to the compressor? IMHO, it makes the code harder to read.

@arian

arian Feb 24, 2011

Owner

The problem basically was:

if (condition){
    var wheel = 'hi';
}

return {wheel: wheel};
@michaelficarra

michaelficarra Feb 24, 2011

Just noticed this line note after posting a comment on the original commit. I fully agree with kassens here. I don't see any problems with your "problematic" code; care to elaborate?

My original comment: "Why hoist your own variable declarations? That's why JS does it for you, it's a feature. It allows you to declare you variables closer to where you use them, resulting in pretty big readability gains."

@cpojer

cpojer Feb 24, 2011

Owner

The point is that variables were referenced outside of the block they are defined in. While it might not be an issue in JavaScript it is still better to avoid such style.

@michaelficarra

michaelficarra Feb 24, 2011

Ah, I see. I didn't notice that they were all used again much further down. I'd agree now that this is a better practice.

@ibolmo

ibolmo Feb 24, 2011

Owner

I'm with kassens, though, multiline var is horrible practice.

@kassens

kassens Feb 24, 2011

Member

Sorry, I thought those were local variables, if they are used at the end the initial definition makes sense, but we could probably just use this.wheel = ...; instead of the Object.append at the end. I'll see if that makes the code cleaner.

@digitarald

digitarald Feb 26, 2011

Contributor

I agree, why all these iterations in Object.append when we write the variable onces and put it in this right way?

@cpojer

cpojer Feb 26, 2011

Owner

do it and send a pull request :)

while (target && target.nodeType == 3) target = target.parentNode;
if (type.indexOf('key') != -1){
- var code = event.which || event.keyCode;
- var key = Object.keyOf(Event.Keys, code);
+ code = event.which || event.keyCode;
+ key = Object.keyOf(Event.Keys, code);
if (type == 'keydown'){
var fKey = code - 111;
if (fKey > 0 && fKey < 13) key = 'f' + fKey;
@@ -45,10 +47,9 @@ var Event = new Type('Event', function(event, win){
y: (event.pageY != null) ? event.pageY - win.pageYOffset : event.clientY
};
if ((/DOMMouseScroll|mousewheel/).test(type)){
- var wheel = (event.wheelDelta) ? event.wheelDelta / 120 : -(event.detail || 0) / 3;
+ wheel = (event.wheelDelta) ? event.wheelDelta / 120 : -(event.detail || 0) / 3;
}
- var rightClick = (event.which == 3) || (event.button == 2),
- related = null;
+ rightClick = (event.which == 3) || (event.button == 2);
if ((/over|out/).test(type)){
related = event.relatedTarget || event[(type == 'mouseover' ? 'from' : 'to') + 'Element'];
var testRelated = function(){

0 comments on commit af48c8d

Please sign in to comment.