[NTP] Allow reordering of apps via drag and drop.

chrome/browser/resources/ntp/apps.js

Index: chrome/browser/resources/ntp/apps.js
diff --git a/chrome/browser/resources/ntp/apps.js b/chrome/browser/resources/ntp/apps.js
index 4dd8bc12ce14a1589f4118fd95c06d46691860d4..f88d3fc1eacb42e3c48e380e9bf11c302ddf8909 100644
--- a/chrome/browser/resources/ntp/apps.js
+++ b/chrome/browser/resources/ntp/apps.js
@@ -45,6 +45,15 @@ function getAppsCallback(data) {
     return a.app_launch_index - b.app_launch_index;
   });
 
+  // Determines if the web store link should be detached and place in the
+  // top right of the screen.
+  apps.detachWebstoreEntry =
+      !apps.showPromo && data.apps.length >= MAX_APPS_PER_ROW[layoutMode];
+
+  apps.data = data.apps;
+  if (!apps.detachWebstoreEntry)
+    apps.data.push('web-store-entry');
+
   clearClosedMenu(apps.menu);
   data.apps.forEach(function(app) {
     appsSectionContent.appendChild(apps.createElement(app));
@@ -84,12 +93,15 @@ function getAppsCallback(data) {
     appsPromoLink.setAttribute('ping', appsPromoPing);
   maybeDoneLoading();
 
-  if (isDoneLoading()) {
-    if (!apps.showPromo && data.apps.length >= MAX_APPS_PER_ROW[layoutMode])
-      webStoreEntry.classList.add('loner');
-    else
-      webStoreEntry.classList.remove('loner');
+  // Disable the animations when the app launcher is being (re)initailized.
+  apps.layout(true);
 
+  if (apps.detachWebstoreEntry)
+    webStoreEntry.classList.add('loner');
+  else
+    webStoreEntry.classList.remove('loner');
+
+  if (isDoneLoading()) {
     updateMiniviewClipping(appsMiniview);
     layoutSections();
   }
@@ -261,6 +273,12 @@ var apps = (function() {
     }
   });
 
+  // Moves the element at position |from| in array |arr| to position |to|.
+  function arrayMove(arr, from, to) {
+    var element = arr.splice(from, 1);
+    arr.splice(to, 0, element[0]);
+  }
+
   return {
     loaded: false,
 
@@ -268,6 +286,179 @@ var apps = (function() {
 
     showPromo: false,
 
+    detachWebstoreEntry: false,
+
+    // The list of app ids, in order, of each app in the launcher.
+    data_: null,
+    get data() { return this.data_; },
+    set data(data) {
+      var ids = [];

[arv (Not doing code reviews), 2011/01/22 00:52:14]

this.data_ = data.map(function(app) {
  return app.id;
});

[jstritar, 2011/01/24 01:00:42]

Done.

+      data.forEach(function(app) {
+        ids.push(app.id);
+      });
+      this.data_ = ids;
+      this.invalidate();
+    },
+
+    dirty_: true,
+    invalidate: function() {
+      this.dirty_ = true;
+    },
+
+    visible_: true,
+    get visible() {
+      return this.visible_;
+    },
+    set visible(visible) {
+      this.visible_ = visible;
+      this.invalidate();
+    },
+
+    // DragAndDropDelegate
+
+    dragContainer: $('apps-content'),
+    transitionsDuration: 200,
+
+    get dragItem() { return this.dragItem_; },
+    set dragItem(dragItem) {
+      if (this.dragItem_ != dragItem) {
+        this.dragItem_ = dragItem;
+        this.invalidate();
+      }
+    },
+
+    // These are the dimensions of each item in the app launcher. These need
+    // to be in sync with the rules in apps.css.

[Aaron Boodman, 2011/01/22 23:42:57]

Is it possible to get them via offsetHeight/offsetWidth instead? Seems like the
margins could be gottten via getComputedStyle().

[jstritar, 2011/01/24 01:00:42]

Done.

+    get dimensions() {
+      return {
+        itemHeight: 136,
+        itemWidth: 124,
+        marginWidth: 3,
+        marginHeight: 5,
+        borderWidth: 0
+      };
+    },
+
+    // Gets the item under the mouse event |e|. Returns null if there is no
+    // item or if the item is not draggable.
+    getItem: function(e) {
+      var item = findAncestorByClass(e.target, 'app');
+
+      // You can't drag the web store launcher.
+      if (item.classList.contains('web-store-entry'))
+        return null;
+
+      return item;
+    },
+
+    // Returns true if |position| is a valid place to drop an app.
+    canDropOn: function(position) {
+      var appCount = this.data.length;
+      if (!this.detachWebstoreEntry)
+        appCount--;
+      return position >= 0 && position < appCount;
+    },
+
+    setDragPlaceholder: function(position) {
+      var appId = this.dragItem.querySelector('a').getAttribute('app-id');
+      var current = this.data.indexOf(appId);
+
+      if (current == position || current < 0)
+        return;
+
+      arrayMove(this.data, current, position);
+      this.invalidate();
+      this.layout();
+    },
+
+    saveDrag: function() {

[Aaron Boodman, 2011/01/22 23:42:57]

Method isn't called from dragdrop.js ?

[jstritar, 2011/01/24 01:00:42]

Oops.... that could be a problem! Done.

+      this.invalidate();
+      this.layout();
+
+      // Wait until the transitions are complete before notifying the browser.
+      // Otherwise, the apps will be re-rendered while still transitioning.
+      setTimeout(function() {
+        chrome.send('reorderApps', this.data.filter(function(id) {
+          return id != 'web-store-entry';
+        }));
+      }, this.transitionsDuration + 10);
+    },
+
+    layout: function(disable_animations) {

[Aaron Boodman, 2011/01/22 23:42:57]

It sucks that it is different, but naming in JavaScript is disableAnimations for
locals.

[Aaron Boodman, 2011/01/22 23:42:57]

Style nit: I don't much care for the boolean flags to methods.

You could do:

layoutWithAnimations()
layoutWithoutAnimations()

Or:

layout({disableAnimation:true});

[jstritar, 2011/01/24 01:00:42]

Done. Went with layout({disableAnimation:true}) in case we add more options.

[jstritar, 2011/01/24 01:00:42]

Done.

+      if (!this.dirty_)
+        return;
+
+      try {
+        if (disable_animations)
+          this.dragContainer.setAttribute('launcher-animations', false);
+        var d0 = Date.now();
+        this.applyAppRects();
+        this.dirty_ = false;
+        logEvent('apps.layout: ' + (Date.now() - d0));
+
+      } finally {
+        if (disable_animations) {
+          // We need to re-enable animations asynchronously, so that the
+          // animations are still disabled for this layout update.
+          setTimeout(function() {
+            this.dragContainer.setAttribute('launcher-animations', true);

[arv (Not doing code reviews), 2011/01/22 00:52:14]

How about introducing a variable to dragContainer. Then you do not need to bind.

[jstritar, 2011/01/24 01:00:42]

Done.

+          }.bind(this), 0);
+        }
+      }
+    },
+
+    applyAppRects: function() {

[Aaron Boodman, 2011/01/22 23:42:57]

Kind of an odd name for this method. What about layoutImpl_? The Impl suffix is
a common convention for a method that does the real work of another public
facing method.

[jstritar, 2011/01/24 01:00:42]

Done.

+      var apps = this.data;
+      var rects = this.getAppLayoutRects(apps.length);
+      var appsContent = $('apps-content');
+      var d = this.dimensions;
+      var h = d.itemHeight + 2 * d.marginHeight + 2 * d.borderWidth;
+      var maxRows = 0;
+
+      for (var i = 0; i < apps.length; i++) {
+        var t = appsContent.querySelector('[app-id='+apps[i]+']').parentNode;

[Aaron Boodman, 2011/01/22 23:42:57]

Avoid single variable letter names unless they are extremely obvious (eg: i, j,
x, y, w, h), or have well-known mathematical meanings (eg: a^2 + b^2 = c^2).

[jstritar, 2011/01/24 01:00:42]

Done.

+
+        // If the node is being dragged, don't try to place it in the grid.
+        if (t == this.dragItem)
+          continue;
+
+        maxRows = Math.max(maxRows, rects[i].row);
+
+        t.style.left = rects[i].left + 'px';
+        t.style.top = rects[i].top + 'px';
+        t.style.right = '';

[Aaron Boodman, 2011/01/22 23:42:57]

It looks like maybe you were going to use 'right', but then decided not to. It
is never set to anything. You can remove this line, and another one in
dragdropcontroller.js.

[jstritar, 2011/01/24 01:00:42]

Done.

+        t.style.display = this.visible ? '' : 'none';

[Aaron Boodman, 2011/01/22 23:42:57]

If this.visible is false, does it mean that the section is just collapsed, or
that it is completely hidden.

If completely hidden (eg in menu mode), it seems like doing this layout is not
even necessary.

[jstritar, 2011/01/24 01:00:42]

Done.

+
+        var innerStyle = t.firstElementChild.style;
+        innerStyle.left = innerStyle.top = '';

[Aaron Boodman, 2011/01/22 23:42:57]

This seems odd, when are these ever used?

[jstritar, 2011/01/24 01:00:42]

Oops... must have brought this in from the most visited drag and drop code.
Removed.

+      }
+
+      // Set the container's height manually now that we use absolute
+      // positioning.
+      var lastRect = rects[rects.length - 1];
+      appsContent.style.height = (++maxRows * h) + 'px';
+    },
+
+    getAppLayoutRects: function(appCount) {
+      var d = this.dimensions;

[Aaron Boodman, 2011/01/22 23:42:57]

I don't think it's obvious what d stands for. How about just typing out
'dimensions'. Or if you make the change I suggested above, you could cache a
dimensions object in the apps object once it is calculated once and just use
that.

[jstritar, 2011/01/24 01:00:42]

Done.

+      var availableWidth = this.dragContainer.offsetWidth;
+      var rtl = isRtl();
+      var rects = [];
+      var w = d.itemWidth + 2 * d.borderWidth + 2 * d.marginWidth;
+      var h = d.itemHeight + 2 * d.borderWidth + 2 * d.marginHeight;
+
+      for (var i = 0; i < appCount; i++) {
+        var row = Math.floor((w * i) / availableWidth);
+        var top = row * h;
+        var left = (w * i) % availableWidth;
+
+        // Reflect the X axis if an RTL language is active.
+        if (rtl) left = availableWidth - left - w;

[arv (Not doing code reviews), 2011/01/22 00:52:14]

line break?

[jstritar, 2011/01/24 01:00:42]

Done.

+        rects[i] = {left: left, top: top, row: row};
+      }
+      return rects;
+    },
+
     createElement: function(app) {
       var div = createElement(app);
       var a = div.firstChild;
@@ -344,7 +535,7 @@ var apps = (function() {
         'name': localStrings.getString('web_store_title'),
         'launch_url': localStrings.getString('web_store_url')
       });
-      elm.setAttribute('app-id', 'web-store-entry');
+      elm.classList.add('web-store-entry');
       return elm;
     },
 
@@ -364,3 +555,6 @@ var apps = (function() {
     }
   };
 })();
+
+// Enable drag and drop reordering of the app launcher.
+var appDragAndDrop = new DragAndDropController(apps);