https://codereview.appspot.com/6821088/diff/1/app/views/environment.js File app/views/environment.js (right):
https://codereview.appspot.com/6821088/diff/1/app/views/environment.js#newcode28 app/views/environment.js:28: this.addRelationDragStart.call(this, box, context); why do you need to use the 'call' method and pass 'this'? You could simply call 'this.addRelationDragStart(box, context)', no? https://codereview.appspot.com/6821088/diff/1/app/views/environment.js#newcode133 app/views/environment.js:133: self.rectMousemove.call(node.getDOMNode(), d, self); why to you set the scope to getDOMNode and pass self ('this') as parameter? You could pass the domNode as parameter instead... self.rectMousemove(node.getDOMNode(), d); https://codereview.appspot.com/6821088/diff/1/app/views/environment.js#newcode176 app/views/environment.js:176: self.rectMousemove.call(node.getDOMNode(), d, self); It seems we have mousemove more than once in this file. You could extract it to a common function in this closure. https://codereview.appspot.com/6821088/diff/1/app/views/environment.js#newcode209 app/views/environment.js:209: self.backgroundClicked.call(self); You dont need to use 'call'. You can do... self.backgroundClicked(); https://codereview.appspot.com/6821088/diff/1/app/views/environment.js#newcode446 app/views/environment.js:446: rectMousemove: function(d, self) { According the the rest of the code, the scope here is domNode. If you dont change the scope you can do (not tested... :) )... // ************************* self.rectMousemove(node.getDOMNode()); rectMousemove: function(domNode) { var mouse = d3.mouse(domNode); d3.event.x = mouse[0]; d3.event.y = mouse[1]; this.addRelationDrag(this.get('addRelationStart_service'), domNode); }, // ************************* btw, you dont use the 'd' parameter. https://codereview.appspot.com/6821088/diff/1/app/views/environment.js#newcode1231 app/views/environment.js:1231: possible_relations = Y.Array.map( The indentation in this block is really strange. Maybe you could just declare the variable and then assign the value to it in another line. https://codereview.appspot.com/6821088/diff/1/app/views/environment.js#newcode1529 app/views/environment.js:1529: view.startRelation.call(view, service); It seems you like to use the 'call' function. :) https://codereview.appspot.com/6821088/diff/1/undocumented File undocumented (left): https://codereview.appspot.com/6821088/diff/1/undocumented#oldcode116 undocumented:116: app/views/environment.js click On 2012/11/07 15:10:41, gary.poster wrote: > The People of the Future thank you. Hahahaha! https://codereview.appspot.com/6821088/ -- https://code.launchpad.net/~benji/juju-gui/add-rel-improvements-3/+merge/133104 Your team Juju GUI Hackers is subscribed to branch lp:juju-gui. -- Mailing list: https://launchpad.net/~yellow Post to : [email protected] Unsubscribe : https://launchpad.net/~yellow More help : https://help.launchpad.net/ListHelp
