Merge lp:~benji/juju-gui/charm-dnd-2 into lp:juju-gui/experimental

[Benji York]

Make charms able to be dragged to the environment.

https://codereview.appspot.com/10500044/

[lp:~benji/juju-gui/charm-dnd-2]

745. By Benji York on 2013-06-24

remove unneeded dependencies

746. By Benji York on 2013-06-24

checkpoint

747. By Benji York on 2013-06-24

tests

748. By Benji York on 2013-06-24

pre-review fixes

749. By Benji York on 2013-06-24

more pre-review fixes

[Jeff Pihach (hatch)]

This is such a cool feature thanks for implementing it. I added a
comment about deploying to the position that the user dropped as a
follow-up branch as it's probably a bit too much work to tack onto this
one :-)

My comments below are mostly just cleanup and few small improvement
requests. If you have any questions don't hesitate to ask!

https://codereview.appspot.com/10500044/diff/5001/app/app.js
File app/app.js (right):

https://codereview.appspot.com/10500044/diff/5001/app/app.js#newcode582
app/app.js:582: Y.on('initiateDeploy', function(charm) {
as per comment in topology/service.js this should be:

this.views.environment.instance.topo.on('initiateDeploy', function() {
... });

Environment is a bubble target for the topology so it might be
preferable to do

this.env.on('initiateDeploy', ...
-or-
this.env.on('Topology:initiateDeploy', ...

I'd even entertain the idea of renaming the event to be
'deployFromCharm'

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

https://codereview.appspot.com/10500044/diff/5001/app/views/topology/service.js#newcode387
app/views/topology/service.js:387: canvasDropHandler: function() {
- possible followup branch

This should take note of the drop position and place the service there
instead of the default deploy location.

https://codereview.appspot.com/10500044/diff/5001/app/views/topology/service.js#newcode388
app/views/topology/service.js:388: // XXX Why do we have to dig so deep
just to get to the event? And why
Typically libraries will wrap native objects with their own convenience
layer and then provide access to the original one on a private property.
As for why it's not passed to the handler....not sure Makyo might know.

https://codereview.appspot.com/10500044/diff/5001/app/views/topology/service.js#newcode395
app/views/topology/service.js:395: Y.fire('initiateDeploy', charm);
This should not be firing on Y, but firing from this class so that the
app can listen on this class for the event.

this.fire('initiateDeploy', charm);

https://codereview.appspot.com/10500044/diff/5001/app/widgets/charm-token.js
File app/widgets/charm-token.js (right):

https://codereview.appspot.com/10500044/diff/5001/app/widgets/charm-token.js#newcode112
app/widgets/charm-token.js:112: if (!container) {
should no longer be necessary as per my comments in the test script.

https://codereview.appspot.com/10500044/diff/5001/app/widgets/charm-token.js#newcode117
app/widgets/charm-token.js:117: this._makeDraggable(container,
container, charmData);
this._makeDraggable(container.one('a'), container, charmData) will get
you to a visibile UI while dragging without attaching to every child
element.

https://codereview.appspot.com/10500044/diff/5001/app/widgets/charm-token.js#newcode119
app/widgets/charm-token.js:119: container.all('*').each(function(child)
{
This can be removed as per comment above

https://codereview.appspot.com/10500044/diff/5001/app/widgets/charm-token.js#newcode133
app/widgets/charm-token.js:133: var outerContainer =
container.ancestor('.yui3-charmtoken')
this is the bounding box this.get('boundingBox').addClass('yui3-u');

Although I'...

[Gary Poster (gary)]

On Jun 24, 2013, at 6:37 PM, Jeff Pihach <email address hidden> wrote:

> This is such a cool feature thanks for implementing it. I added a
> comment about deploying to the position that the user dropped as a
> follow-up branch as it's probably a bit too much work to tack onto this
> one :-)

Yes, that would be fine. If you do that, please create it as a high priority card in the inspector story.

Thank you,

Gary

>
> My comments below are mostly just cleanup and few small improvement
> requests. If you have any questions don't hesitate to ask!
>
>
> https://codereview.appspot.com/10500044/diff/5001/app/app.js
> File app/app.js (right):
>
> https://codereview.appspot.com/10500044/diff/5001/app/app.js#newcode582
> app/app.js:582: Y.on('initiateDeploy', function(charm) {
> as per comment in topology/service.js this should be:
>
> this.views.environment.instance.topo.on('initiateDeploy', function() {
> ... });
>
> Environment is a bubble target for the topology so it might be
> preferable to do
>
> this.env.on('initiateDeploy', ...
> -or-
> this.env.on('Topology:initiateDeploy', ...
>
> I'd even entertain the idea of renaming the event to be
> 'deployFromCharm'
>
> https://codereview.appspot.com/10500044/diff/5001/app/views/topology/service.js
> File app/views/topology/service.js (right):
>
> https://codereview.appspot.com/10500044/diff/5001/app/views/topology/service.js#newcode387
> app/views/topology/service.js:387: canvasDropHandler: function() {
> - possible followup branch
>
> This should take note of the drop position and place the service there
> instead of the default deploy location.
>
> https://codereview.appspot.com/10500044/diff/5001/app/views/topology/service.js#newcode388
> app/views/topology/service.js:388: // XXX Why do we have to dig so deep
> just to get to the event? And why
> Typically libraries will wrap native objects with their own convenience
> layer and then provide access to the original one on a private property.
> As for why it's not passed to the handler....not sure Makyo might know.
>
> https://codereview.appspot.com/10500044/diff/5001/app/views/topology/service.js#newcode395
> app/views/topology/service.js:395: Y.fire('initiateDeploy', charm);
> This should not be firing on Y, but firing from this class so that the
> app can listen on this class for the event.
>
> this.fire('initiateDeploy', charm);
>
> https://codereview.appspot.com/10500044/diff/5001/app/widgets/charm-token.js
> File app/widgets/charm-token.js (right):
>
> https://codereview.appspot.com/10500044/diff/5001/app/widgets/charm-token.js#newcode112
> app/widgets/charm-token.js:112: if (!container) {
> should no longer be necessary as per my comments in the test script.
>
> https://codereview.appspot.com/10500044/diff/5001/app/widgets/charm-token.js#newcode117
> app/widgets/charm-token.js:117: this._makeDraggable(container,
> container, charmData);
> this._makeDraggable(container.one('a'), container, charmData) will get
> you to a visibile UI while dragging without attaching to every child
> element.
>
> https://codereview.appspot.com/10500044/diff/5001/app/widgets/charm-token.js#newcode119
> app/widgets/charm-token.js:119: c...

[Richard Harding (rharding)]

Comments below. Jeff has some great feedback and I agree that there's
some work around the container/boundingBox that'll make things a bit
easier.

https://codereview.appspot.com/10500044/diff/1/app/widgets/charm-container.js
File app/widgets/charm-container.js (right):

https://codereview.appspot.com/10500044/diff/1/app/widgets/charm-container.js#newcode191
app/widgets/charm-container.js:191: _getDropZone: function() {
please make a valueFn so that it's done once and stored. It won't be
changing.

https://codereview.appspot.com/10500044/diff/5001/app/widgets/charm-token.js
File app/widgets/charm-token.js (right):

https://codereview.appspot.com/10500044/diff/5001/app/widgets/charm-token.js#newcode100
app/widgets/charm-token.js:100: element.on('dragstart',
Should we be catching this event to clean up later? This class is using
the Y.Event.EventTracker and you can do a
this.addEvent(element.on('dragstart...

Then it'll auto get removed and cleaned up on the widgets destroy.

https://codereview.appspot.com/10500044/diff/5001/app/widgets/charm-token.js#newcode129
app/widgets/charm-token.js:129: renderUI: function(container) {
I'm not sure why we need the container here. If you pass the boundingBox
into the widget cfg on init you'll have the correct space for the
content box and such.

https://codereview.appspot.com/10500044/diff/5001/app/widgets/charm-token.js#newcode135
app/widgets/charm-token.js:135: this._addDraggability(outerContainer);
This should only be run if the charm-token is instructed to be
draggable. This widget is shared among fullscreen, sidebar, and the
related charms side data uses. It doesn't make sense to have them
draggable in all cases. An ATTR for isDraggable should be added and then
set from the sidebar related views I'd think.

https://codereview.appspot.com/10500044/

[lp:~benji/juju-gui/charm-dnd-2]

750. By Benji York on 2013-06-25

review changes

751. By Benji York on 2013-06-25

lint fixes

[Richard Harding (rharding)]

LGTM with the discussion on using boundingBox via cfg vs the container
and the simpler remove below.

https://codereview.appspot.com/10500044/diff/18001/test/test_charm_token_drag_and_drop.js
File test/test_charm_token_drag_and_drop.js (right):

https://codereview.appspot.com/10500044/diff/18001/test/test_charm_token_drag_and_drop.js#newcode44
test/test_charm_token_drag_and_drop.js:44:
outerContainer.remove().destroy(true);
you can just remove(true) and get it destroyed fully.

https://codereview.appspot.com/10500044/

[Benjamin Saller (bcsaller)]

Thanks for this, going to be a really nice feature when it lands. I
think you need to merge trunk and maybe look at this again, the ghost
handling and the inspector stuff changes and possibly simplifies some of
this.

https://codereview.appspot.com/10500044/diff/18001/app/app.js
File app/app.js (right):

https://codereview.appspot.com/10500044/diff/18001/app/app.js#newcode584
app/app.js:584: }, this.charmPanel);
We can't invoke the function above in cfg.deploy? we already switch the
impl based on the feature flag.

https://codereview.appspot.com/10500044/diff/18001/app/store/env/fakebackend.js
File app/store/env/fakebackend.js (right):

https://codereview.appspot.com/10500044/diff/18001/app/store/env/fakebackend.js#newcode1232
app/store/env/fakebackend.js:1232: var importImpl;
This looks like a story for another day.. Not sure what you must have
hit here to need this. :)

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

https://codereview.appspot.com/10500044/diff/18001/app/views/topology/service.js#newcode390
app/views/topology/service.js:390: // is it not just passed into the
handler?
I blame YUI :)

https://codereview.appspot.com/10500044/diff/18001/app/views/topology/service.js#newcode394
app/views/topology/service.js:394: var charmData =
Y.JSON.parse(evt.dataTransfer.getData('charmData'));
Why JSON parse at all if its a single string?

https://codereview.appspot.com/10500044/diff/18001/app/views/topology/service.js#newcode396
app/views/topology/service.js:396: Y.fire('initiateDeploy', charm);
If you merge trunk we also have db.services.ghostDeploy(charm) which
creates the ghost model from the charm. The rest of the system has been
changed to react to that, render the proper model, set up the inspector,
etc.

https://codereview.appspot.com/10500044/diff/18001/app/widgets/charm-token.js
File app/widgets/charm-token.js (right):

https://codereview.appspot.com/10500044/diff/18001/app/widgets/charm-token.js#newcode84
app/widgets/charm-token.js:84:
evt.dataTransfer.setDragImage(dragImage._node, 0, 0);
cute with the image

https://codereview.appspot.com/10500044/diff/18001/app/widgets/charm-token.js#newcode112
app/widgets/charm-token.js:112: var charmData =
Y.JSON.stringify(this.charmAttributes);
You only need the id here, no? If thats true then no need for JSON

https://codereview.appspot.com/10500044/diff/18001/test/test_notifier_widget.js
File test/test_notifier_widget.js (right):

https://codereview.appspot.com/10500044/diff/18001/test/test_notifier_widget.js#newcode97
test/test_notifier_widget.js:97: }, 500);
That must have been a hard one to track down.

https://codereview.appspot.com/10500044/

[lp:~benji/juju-gui/charm-dnd-2]

752. By Benji York on 2013-06-26

review changes

753. By Benji York on 2013-06-26

fix lint

=== modified file 'app/app.js'
--- app/app.js	2013-06-25 19:27:16 +0000
+++ app/app.js	2013-06-26 13:48:25 +0000
@@ -593,6 +593,12 @@
                                 this.on_database_changed, this);
       }
       this.addSubApplications(cfg);
+
+      // When someone wants a charm to be deployed they fire initiateDeploy
+      // and we show the charm panel to configure/deploy the service.
+      Y.on('initiateDeploy', function(charm) {
+        this.deploy(charm);
+      }, this.charmPanel);
     },
 
     /**

=== modified file 'app/store/env/fakebackend.js'
--- app/store/env/fakebackend.js	2013-06-25 19:36:29 +0000
+++ app/store/env/fakebackend.js	2013-06-26 13:48:25 +0000
@@ -28,7 +28,8 @@
 YUI.add('juju-env-fakebackend', function(Y) {
 
   var models = Y.namespace('juju.models');
-  var UNAUTHENTICATEDERROR = {error: 'Please log in.'};
+  var UNAUTHENTICATED_ERROR = {error: 'Please log in.'};
+  var VALUE_ERROR = {error: 'Unparsable environment data.'};
 
   /**
     Loops through the charm endpoint data to determine whether we have a
@@ -163,7 +164,7 @@
     */
     nextChanges: function() {
       if (!this.get('authenticated')) {
-        return UNAUTHENTICATEDERROR;
+        return UNAUTHENTICATED_ERROR;
       }
       var result;
       if (Y.Object.isEmpty(this.changes.services) &&
@@ -205,7 +206,7 @@
     */
     nextAnnotations: function() {
       if (!this.get('authenticated')) {
-        return UNAUTHENTICATEDERROR;
+        return UNAUTHENTICATED_ERROR;
       }
       var result;
       if (Y.Object.isEmpty(this.annotations.services) &&
@@ -293,7 +294,7 @@
     */
     deploy: function(charmId, callback, options) {
       if (!this.get('authenticated')) {
-        return callback(UNAUTHENTICATEDERROR);
+        return callback(UNAUTHENTICATED_ERROR);
       }
       if (!options) {
         options = {};
@@ -502,7 +503,7 @@
      */
     destroyService: function(serviceName) {
       if (!this.get('authenticated')) {
-        return UNAUTHENTICATEDERROR;
+        return UNAUTHENTICATED_ERROR;
       }
       var service = this.db.services.getById(serviceName);
       if (!service) {
@@ -539,7 +540,7 @@
      */
     getService: function(serviceName) {
       if (!this.get('authenticated')) {
-        return UNAUTHENTICATEDERROR;
+        return UNAUTHENTICATED_ERROR;
       }
       var service = this.db.services.getById(serviceName);
       if (!service) {
@@ -567,7 +568,7 @@
      */
     getCharm: function(charmName, callback) {
       if (!this.get('authenticated')) {
-        return callback(UNAUTHENTICATEDERROR);
+        return callback(UNAUTHENTICATED_ERROR);
       }
       var formatCharm = function(charm) {
         // Simulate a delay in the charm loading for testing.
@@ -597,7 +598,7 @@
     */
     addUnit: function(serviceName, numUnits) {
       if (!this.get('authenticated')) {
-        return UNAUTHENTICATEDERROR;
+        return UNAUTHENTICATED_ERROR;
       }
       if (Y.Lang.isUndefined(numUnits)) {
         numUnits = 1;
@@ -742,7 +743,7 @@
           warning, error;
 
       if (!this.get('authenticated')) {
-        return UNAUTHENTICATEDERROR;
+        return UNAUTHENTICATED_ERROR;
       }
 
       if (service) {
@@ -775,7 +776,7 @@
           warning, error;
 
       if (!this.get('authenticated')) {
-        return UNAUTHENTICATEDERROR;
+        return UNAUTHENTICATED_ERROR;
       }
       if (service) {
         if (service.get('exposed')) {
@@ -805,7 +806,7 @@
     */
     addRelation: function(endpointA, endpointB) {
       if (!this.get('authenticated')) {
-        return UNAUTHENTICATEDERROR;
+        return UNAUTHENTICATED_ERROR;
       }
       if ((typeof endpointA !== 'string') ||
           (typeof endpointB !== 'string')) {
@@ -886,7 +887,7 @@
     */
     removeRelation: function(endpointA, endpointB) {
       if (!this.get('authenticated')) {
-        return UNAUTHENTICATEDERROR;
+        return UNAUTHENTICATED_ERROR;
       }
       if ((typeof endpointA !== 'string') ||
           (typeof endpointB !== 'string')) {
@@ -960,7 +961,7 @@
      */
     updateAnnotations: function(entityName, annotations) {
       if (!this.get('authenticated')) {
-        return UNAUTHENTICATEDERROR;
+        return UNAUTHENTICATED_ERROR;
       }
       var entity = this.db.resolveModelByName(entityName);
       var existing;
@@ -993,7 +994,7 @@
      */
     getAnnotations: function(entityName) {
       if (!this.get('authenticated')) {
-        return UNAUTHENTICATEDERROR;
+        return UNAUTHENTICATED_ERROR;
       }
       var entity = this.db.resolveModelByName(entityName);
 
@@ -1015,7 +1016,7 @@
      */
     removeAnnotations: function(entityName, keys) {
       if (!this.get('authenticated')) {
-        return UNAUTHENTICATEDERROR;
+        return UNAUTHENTICATED_ERROR;
       }
       var entity = this.db.resolveModelByName(entityName);
       var annotations;
@@ -1058,7 +1059,7 @@
     */
     setConfig: function(serviceName, config) {
       if (!this.get('authenticated')) {
-        return UNAUTHENTICATEDERROR;
+        return UNAUTHENTICATED_ERROR;
       }
       var service = this.db.services.getById(serviceName);
       if (!service) {
@@ -1096,7 +1097,7 @@
       var constraints = {};
 
       if (!this.get('authenticated')) {
-        return UNAUTHENTICATEDERROR;
+        return UNAUTHENTICATED_ERROR;
       }
       var service = this.db.services.getById(serviceName);
       if (!service) {
@@ -1138,7 +1139,7 @@
      */
     resolved: function(unitName, relationName) {
       if (!this.get('authenticated')) {
-        return UNAUTHENTICATEDERROR;
+        return UNAUTHENTICATED_ERROR;
       }
       var unit = this.db.units.getById(unitName);
       if (!unit) {
@@ -1217,7 +1218,7 @@
           };
 
       if (!this.get('authenticated')) {
-        return UNAUTHENTICATEDERROR;
+        return UNAUTHENTICATED_ERROR;
       }
 
       serviceList.each(function(s) {
@@ -1260,11 +1261,17 @@
    */
     importEnvironment: function(jsonData, callback) {
       if (!this.get('authenticated')) {
-        return callback(UNAUTHENTICATEDERROR);
-      }
-      var data = JSON.parse(jsonData),
-          version = 0,
-          importImpl;
+        return callback(UNAUTHENTICATED_ERROR);
+      }
+      var data;
+      try {
+        data = JSON.parse(jsonData);
+      } catch (e) {
+        console.log('error parsing environment data');
+        return callback(VALUE_ERROR);
+      }
+      var version = 0;
+      var importImpl;
       // Dispatch to the correct version after inspecting the JSON data.
       if (data.meta && data.meta.exportFormat) {
         version = data.meta.exportFormat;

=== modified file 'app/subapps/browser/views/editorial.js'
--- app/subapps/browser/views/editorial.js	2013-06-18 13:08:42 +0000
+++ app/subapps/browser/views/editorial.js	2013-06-26 13:48:25 +0000
@@ -115,7 +115,8 @@
 
           var containerCfg = {
             additionalChildConfig: {
-              size: this.get('isFullscreen') ? 'large' : 'small'
+              size: this.get('isFullscreen') ? 'large' : 'small',
+              isDraggable: !this.get('isFullscreen')
             }
           };
 

=== modified file 'app/views/charm-panel.js'
--- app/views/charm-panel.js	2013-06-20 15:37:45 +0000
+++ app/views/charm-panel.js	2013-06-26 13:48:25 +0000
@@ -856,7 +856,7 @@
         charmId: charm.get('id')
       });
       // Since we are showing the configure/deploy panel ex nihilo, we want the
-      // panel to disappear when the deploy completes or is candled, but just
+      // panel to disappear when the deploy completes or is canceled, but just
       // this once (i.e., they should work normally if the user opens the panel
       // via a charm search).
       panels.configuration.once('panelRemoved', hide);

=== modified file 'app/views/topology/importexport.js'
--- app/views/topology/importexport.js	2013-06-25 20:49:59 +0000
+++ app/views/topology/importexport.js	2013-06-26 13:48:25 +0000
@@ -98,7 +98,14 @@
               reader.readAsText(file);
             });
           } else {
-            env.importEnvironment(evt._event.dataTransfer.getData('Text'));
+            // If this is not a drop event generated by the application (all
+            // of which have a dataType), then it was caused by dropping
+            // something external to the app, in which case it might be an
+            // environment configuration, so process it as such.
+            var dataType = evt._event.dataTransfer.getData('dataType');
+            if (dataType === undefined) {
+              env.importEnvironment(evt._event.dataTransfer.getData('Text'));
+            }
           }
           evt.preventDefault();
           evt.stopPropagation();

=== modified file 'app/views/topology/service.js'
--- app/views/topology/service.js	2013-06-23 22:54:49 +0000
+++ app/views/topology/service.js	2013-06-26 13:48:25 +0000
@@ -77,7 +77,8 @@
           mouseout: 'serviceStatusMouseOut'
         },
         '.zoom-plane': {
-          click: 'canvasClick'
+          click: 'canvasClick',
+          drop: 'canvasDropHandler'
         },
         // Menu/Controls
         '.view-service': {
@@ -378,6 +379,25 @@
     },
 
     /**
+     * Handle deploying a services by dropping a charm onto the canvas.
+     *
+     * @method canvasDropHandler
+     * @static
+     * @return {undefined} Nothing.
+     */
+    canvasDropHandler: function() {
+      var evt = d3.event._event;  // So well hidden.
+      var dataType = evt.dataTransfer.getData('dataType');
+      if (dataType === 'charm-token-drag-and-drop') {
+        // The charm data was JSON encoded because the dataTransfer mechanism
+        // only allows for string values.
+        var charmData = Y.JSON.parse(evt.dataTransfer.getData('charmData'));
+        var charm = new models.Charm(charmData);
+        Y.fire('initiateDeploy', charm);
+      }
+    },
+
+    /**
      * Clear any stateful actions (menus, etc.) when a clearState event is
      * received.
      *

=== modified file 'app/widgets/charm-token.js'
--- app/widgets/charm-token.js	2013-06-14 13:14:27 +0000
+++ app/widgets/charm-token.js	2013-06-26 13:48:25 +0000
@@ -38,6 +38,18 @@
     TEMPLATE: Y.namespace('juju.views').Templates['charm-token'],
 
     /**
+    * Default general initializer method.
+    *
+    * @method initializer
+    * @param {Object} cfg the config for the widget.
+    * @return {undefined} Nothing.
+    */
+    initializer: function(charmAttributes) {
+      // This passed-in config is made up of charm attributes.
+      this.charmAttributes = charmAttributes;
+    },
+
+    /**
       Setter for the boundingBox attribute
 
       **Override vs YUI to prevent node id setting based on BrowserCharm**
@@ -56,15 +68,71 @@
     },
 
     /**
+     * Generate a function that records drag and drop data when a drag starts.
+     *
+     * @method _makeDragStartHandler
+     * @param {Node} dragImage The node to show during the drag.
+     * @param {String} charmData The JSON encoded charm attributes.
+     * @return {undefined} Nothing.
+     */
+    _makeDragStartHandler: function(dragImage, charmData) {
+      return function(evt) {
+        evt = evt._event; // We want the real event.
+        evt.dataTransfer.effectAllowed = 'copy';
+        evt.dataTransfer.setData('charmData', charmData);
+        evt.dataTransfer.setData('dataType', 'charm-token-drag-and-drop');
+        evt.dataTransfer.setDragImage(dragImage._node, 0, 0);
+      };
+    },
+
+    /**
+     * Make an element draggable.
+     *
+     * @method _makeDraggable
+     * @param {Node} element The node which is to be made draggable.
+     * @param {Node} dragImage The node which will be displayed during
+     *   dragging.
+     * @param {String} charmData The JSON encoded charm attributes.
+     * @return {undefined} Nothing.
+     */
+    _makeDraggable: function(element, dragImage, charmData) {
+      element.setAttribute('draggable', 'true');
+      this.addEvent(element.on('dragstart',
+          this._makeDragStartHandler(dragImage, charmData)));
+    },
+
+    /**
+     * Make the charm token draggable.
+     *
+     * @method _addDraggability
+     * @param {Node} container he node which contains the charm list.
+     * @return {undefined}  Nothing; side-effects only.
+    */
+    _addDraggability: function(container) {
+      // Since the browser's dataTransfer mechanism only accepts string values
+      // we have to JSON encode the charm data.
+      var charmData = Y.JSON.stringify(this.charmAttributes);
+      this._makeDraggable(container, container, charmData);
+      this._makeDraggable(container.one('a'), container, charmData);
+    },
+
+    /**
      * Create the nodes required by this widget and attach them to the DOM.
      *
+     * @param {Node} container The contaner to render into.  Mainly for
+     *   testing.
      * @method renderUI
      */
-    renderUI: function() {
+    renderUI: function(container) {
       var content = this.TEMPLATE(this.getAttrs());
-      var container = this.get('contentBox');
+      // This way we can pass in a container for easier testing.
+      container = container || this.get('contentBox');
       container.setHTML(content);
-      container.ancestor('.yui3-charmtoken').addClass('yui3-u');
+      var outerContainer = container.ancestor('.yui3-charmtoken')
+        .addClass('yui3-u');
+      if (this.get('isDraggable')) {
+        this._addDraggability(outerContainer);
+      }
     }
 
   }, {
@@ -147,6 +215,17 @@
        */
       size: {
         value: 'small'
+      },
+      /**
+       * Should the charm token be draggable?
+       *
+       * @attribute isDraggable
+       * @default true
+       * @type {Boolean}
+       *
+       */
+      isDraggable: {
+        value: true
       }
     }
   });

=== modified file 'test/index.html'
--- test/index.html	2013-06-20 22:15:49 +0000
+++ test/index.html	2013-06-26 13:48:25 +0000
@@ -60,8 +60,12 @@
   <script src="test_charm_panel.js"></script>
   <script src="test_charm_store.js"></script>
   <script src="test_charm_token.js"></script>
+<<<<<<< TREE
 
   <!-- FIXME: tests flicker, add container. -->
+=======
+  <script src="test_charm_token_drag_and_drop.js"></script>
+>>>>>>> MERGE-SOURCE
   <script src="test_charm_view.js"></script>
 
   <script src="test_console.js"></script>

=== added file 'test/test_charm_token_drag_and_drop.js'
--- test/test_charm_token_drag_and_drop.js	1970-01-01 00:00:00 +0000
+++ test/test_charm_token_drag_and_drop.js	2013-06-26 13:48:25 +0000
@@ -0,0 +1,147 @@
+/*
+This file is part of the Juju GUI, which lets users view and manage Juju
+environments within a graphical interface (https://launchpad.net/juju-gui).
+Copyright (C) 2012-2013 Canonical Ltd.
+
+This program is free software: you can redistribute it and/or modify it under
+the terms of the GNU Affero General Public License version 3, as published by
+the Free Software Foundation.
+
+This program is distributed in the hope that it will be useful, but WITHOUT
+ANY WARRANTY; without even the implied warranties of MERCHANTABILITY,
+SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Affero
+General Public License for more details.
+
+You should have received a copy of the GNU Affero General Public License along
+with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+'use strict';
+
+describe('charm token drag and drop', function() {
+  var Y, container, outerContainer, CharmToken, token;
+
+  before(function(done) {
+    Y = YUI(GlobalConfig).use([
+      'browser-charm-token',
+      'juju-tests-utils'
+    ],
+    function(Y) {
+      CharmToken = Y.juju.widgets.browser.CharmToken;
+      done();
+    });
+
+  });
+
+  beforeEach(function() {
+    outerContainer = Y.namespace('juju-tests.utils').makeContainer()
+      .addClass('yui3-charmtoken');
+    container = Y.namespace('juju-tests.utils').makeContainer();
+    outerContainer.append(container);
+  });
+
+  afterEach(function() {
+    outerContainer.remove().destroy(true);
+    if (token) {
+      token.destroy();
+    }
+  });
+
+  it('makes each charm token draggable', function() {
+    var cfg = {
+      id: 'test',
+      name: 'some-charm',
+      description: 'some description',
+      recent_commit_count: 1,
+      recent_download_count: 3,
+      tested_providers: ['ec2']
+    };
+    token = new CharmToken(cfg);
+    var draggable = [];
+    token._makeDraggable = function(element, dragImage, charmData) {
+      draggable.push(Y.JSON.parse(charmData).id);
+    };
+    token.renderUI(container);
+    // Many elements are made draggable for a single token token.
+    assert.isTrue(draggable.length > 1);
+    // There is only a single token represented by all the draggable elements.
+    draggable = Y.Array.dedupe(draggable);
+    assert.equal(draggable.length, 1);
+    // All of the charm tokens are made draggable.
+    assert.deepEqual(draggable, ['test']);
+  });
+
+  it('can make an element draggable', function() {
+    token = new CharmToken();
+    var setAttributeCalled, onCalled;
+    var element = {
+      setAttribute: function(name, value) {
+        assert.equal(name, 'draggable');
+        assert.equal(value, 'true');
+        setAttributeCalled = true;
+      },
+      on: function(when, callable) {
+        assert.equal(when, 'dragstart');
+        onCalled = true;
+        return {detach: function() {}};
+      }
+    };
+    var dragImage = {};
+    token._makeDragStartHandler = function(dragImage, charmData) {
+      assert.equal(element, element);
+      assert.equal(dragImage, dragImage);
+      assert.equal(charmData, charmData);
+    };
+    token._makeDraggable(element, dragImage, 'charm-id');
+    assert.isTrue(setAttributeCalled);
+    assert.isTrue(onCalled);
+  });
+
+  it('can set up drag and drop configuration', function() {
+    token = new CharmToken();
+    var setDataCalled, setDragImageCalled;
+    var dragImage = {_node: {id: 'the real drag image'}};
+    var charmData = 'data';
+    var handler = token._makeDragStartHandler(dragImage, charmData);
+    var dragDataSet = [];
+    var evt = {
+      _event: {
+        dataTransfer: {
+          setData: function(name, value) {
+            dragDataSet.push([name, value]);
+            setDataCalled = true;
+          },
+          setDragImage: function(provideDragImage, x, y) {
+            assert.equal(provideDragImage, dragImage._node);
+            assert.equal(x, 0);
+            assert.equal(y, 0);
+            setDragImageCalled = true;
+          }
+        }
+      }
+    };
+    handler(evt);
+    assert.equal(evt._event.dataTransfer.effectAllowed, 'copy');
+    assert.equal(Y.JSON.stringify(dragDataSet),
+        '[["charmData","data"],["dataType","charm-token-drag-and-drop"]]');
+    assert.isTrue(setDataCalled);
+    assert.isTrue(setDragImageCalled);
+  });
+
+  it('respects the isDraggable switch', function() {
+    token = new CharmToken({contentBox: container});
+    token.set('isDraggable', false);
+    var dragEnabled = false;
+    token._addDraggability = function() {
+      dragEnabled = true;
+    };
+    token.renderUI(container);
+    // Since we set isDraggable to false, _addDraggability was not called.
+    assert.isFalse(dragEnabled);
+    // If we set isDraggable to true, _addDraggability will be called.
+    token.set('isDraggable', true);
+    token.renderUI(container);
+    assert.isTrue(dragEnabled);
+  });
+
+});

=== modified file 'test/test_notifier_widget.js'
--- test/test_notifier_widget.js	2013-05-17 14:51:05 +0000
+++ test/test_notifier_widget.js	2013-06-26 13:48:25 +0000
@@ -89,34 +89,34 @@
 
   it('should destroy notifications after N milliseconds', function(done) {
     makeNotifier('mytitle', 'mymessage', 1);
-    // A timeout of 250 milliseconds is used so that we ensure the destroying
-    // animation can be completed.
+    // A timeout is used so that we ensure the destroying animation can be
+    // completed.
     setTimeout(function() {
       assertNumNotifiers(0);
       done();
-    }, 250);
+    }, 500);
   });
 
   it('should destroy notifications on click', function(done) {
     makeNotifier();
     notifierBox.one('*').simulate('click');
-    // A timeout of 250 milliseconds is used so that we ensure the destroying
-    // animation can be completed.
+    // A timeout is used so that we ensure the destroying animation can be
+    // completed.
     setTimeout(function() {
       assertNumNotifiers(0);
       done();
-    }, 250);
+    }, 500);
   });
 
   it('should prevent notification removal on mouse enter', function(done) {
     makeNotifier('mytitle', 'mymessage', 1);
     notifierBox.one('*').simulate('mouseover');
-    // A timeout of 250 milliseconds is used so that we ensure the node is not
-    // preserved by the destroying animation.
+    // A timeout is used so that we ensure the destroying animation can be
+    // completed.
     setTimeout(function() {
       assertNumNotifiers(1);
       done();
-    }, 250);
+    }, 500);
   });
 
 });

=== modified file 'test/test_service_module.js'
--- test/test_service_module.js	2013-06-19 15:44:25 +0000
+++ test/test_service_module.js	2013-06-26 13:48:25 +0000
@@ -369,4 +369,20 @@
     assert.deepPropertyVal(boxes, 'wordpress.model', wordpress);
   });
 
+  it('should deploy a service on charm token drop events', function(done) {
+    d3.event._event = {dataTransfer: {
+      getData: function(name) {
+        if (name === 'dataType') {
+          return 'charm-token-drag-and-drop';
+        } else if (name === 'charmData') {
+          return '{"id": "cs:foo/bar-1"}';
+        }
+      }}};
+    var eventHandle = Y.on('initiateDeploy', function(charm) {
+      eventHandle.detach();
+      done();
+    });
+    serviceModule.canvasDropHandler();
+  });
+
 });