Merge lp://qastaging/~makyo/juju-gui/drag-interrupted-1099921 into lp://qastaging/juju-gui/experimental

Reviewers: mp+146228_code.launchpad.net,

Message:
Please take a look.

Description:
Dragging through annotation updates

Previously, dragging a service would fail when an annotation update was
received due to updated objects. This branch and parts of Ben's
ViewModel update have helped fix this. This has been tested in one and
two browser windows (previously, if the same service was dragged in two
browser windows, similar problems would occur).

https://code.launchpad.net/~makyo/juju-gui/drag-interrupted-1099921/+merge/146228

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/7221087/

Affected files:
   A [revision details]
   M app/views/topology/relation.js
   M app/views/topology/service.js

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: app/views/topology/relation.js
=== modified file 'app/views/topology/relation.js'
--- app/views/topology/relation.js 2013-01-31 02:59:16 +0000
+++ app/views/topology/relation.js 2013-02-01 18:49:00 +0000
@@ -149,20 +149,32 @@
       * Update relation line endpoints for a given service.
       *
       * @method updateLinkEndpoints
- * @param {Object} service The service module that has been moved.
+ * @param {Object} evt The event facade that was fired. This should
have
+ * a 'service' property mixed in when fired.
       */
      updateLinkEndpoints: function(evt) {
        var self = this;
        var service = evt.service;
        Y.each(Y.Array.filter(self.relations, function(relation) {
- return relation.source.id === service.id ||
- relation.target.id === service.id;
+ if (relation.source.id === service.id) {
+ if (service.inDrag === 2) {
+ relation.source = service;
+ }
+ return true;
+ } else if (relation.target.id === service.id) {
+ if (service.inDrag === 2) {
+ relation.target = service;
+ }
+ return true;
+ }
+ return false;
        }), function(relation) {
          var rel_group = d3.select('#' + relation.id);
          var connectors = relation.source
                    .getConnectorPair(relation.target);
          var s = connectors[0];
          var t = connectors[1];
+ console.log(s, t);
          rel_group.select('line')
                .attr('x1', s[0])
                .attr('y1', s[1])

Index: app/views/topology/service.js
=== modified file 'app/views/topology/service.js'
--- app/views/topology/service.js 2013-02-01 15:40:54 +0000
+++ app/views/topology/service.js 2013-02-01 18:49:00 +0000
@@ -311,7 +311,7 @@
       **/
      drag: function(box, self, pos, includeTransition) {
        var topo = self.get('component');
- var selection = d3.select(this);
+ var selection = topo.vis.select('#' + box.modelId);

        if (topo.buildingRelation) {
          topo.fire('addRelationDrag...

Extra console.log()s already removed locally.

https://codereview.appspot.com/7221087/diff/1/app/views/topology/relation.js
File app/views/topology/relation.js (right):

https://codereview.appspot.com/7221087/diff/1/app/views/topology/relation.js#newcode177
app/views/topology/relation.js:177: console.log(s, t);
Removed locally.

https://codereview.appspot.com/7221087/diff/1/app/views/topology/service.js
File app/views/topology/service.js (right):

https://codereview.appspot.com/7221087/diff/1/app/views/topology/service.js#newcode369
app/views/topology/service.js:369: console.log('uodate');
Removed locally.

https://codereview.appspot.com/7221087/

LGTM, one minor below.

https://codereview.appspot.com/7221087/diff/1/app/views/topology/relation.js
File app/views/topology/relation.js (right):

https://codereview.appspot.com/7221087/diff/1/app/views/topology/relation.js#newcode160
app/views/topology/relation.js:160: if (service.inDrag === 2) {
Now that this is being used a number of places we might want to use a
define for '2'

views.DRAG_START = 1;
views.DRAG_ACTIVE = 2;

or something similar and replace that. Magic numbers (and I know its my
precedent) smell worse and worse the more they are used.

https://codereview.appspot.com/7221087/

The changes made here look good. Land as is after making Ben's changes.
  Thanks for the fix.

Following the detailed instructions in the bug I was not able to
reproduce the problem on uistage or locally using trunk or these
changes, which is a little disconcerting.

https://codereview.appspot.com/7221087/diff/1/app/views/topology/relation.js
File app/views/topology/relation.js (right):

https://codereview.appspot.com/7221087/diff/1/app/views/topology/relation.js#newcode160
app/views/topology/relation.js:160: if (service.inDrag === 2) {
On 2013/02/04 16:30:18, bcsaller wrote:
> Magic numbers (and I know its my
> precedent) smell worse and worse the more they are used.

I totally concur!

https://codereview.appspot.com/7221087/