NTP5: Fine tuning of Apps implementation.

chrome/browser/resources/ntp_search/apps_page.js

Index: chrome/browser/resources/ntp_search/apps_page.js
diff --git a/chrome/browser/resources/ntp_search/apps_page.js b/chrome/browser/resources/ntp_search/apps_page.js
index 2366e4c720b524e5213ddb4c30c7ea86ca970c2e..ecf88e6063000929f2f809b84c7fd9a85238a6a9 100644
--- a/chrome/browser/resources/ntp_search/apps_page.js
+++ b/chrome/browser/resources/ntp_search/apps_page.js
@@ -576,14 +576,18 @@ cr.define('ntp', function() {
     // wants the non-default behavior.
     config: {
       // The width of a cell.
-      cellWidth: 77,
+      cellWidth: 70,
       // The start margin of a cell (left or right according to text direction).
       cellMarginStart: 12,
       // The maximum number of Tiles to be displayed.
-      maxTileCount: 20
+      maxTileCount: 20,
+      // Whether the TilePage content will be scrollable.
+      scrollable: true,
     },
 
     initialize: function() {
+      TilePage.prototype.initialize.apply(this, arguments);
+
       this.classList.add('apps-page');
 
       this.addEventListener('cardselected', this.onCardSelected_);
@@ -595,8 +599,6 @@ cr.define('ntp', function() {
                             this.onCardChangeEnded_);
 
       this.addEventListener('tilePage:tile_added', this.onTileAdded_);
-
-      this.content_.addEventListener('scroll', this.onScroll_.bind(this));
     },
 
     /**
@@ -605,28 +607,30 @@ cr.define('ntp', function() {
      */
     insertAndHighlightApp: function(appData) {
       ntp.getCardSlider().selectCardByValue(this);
-      this.content_.scrollTop = this.content_.scrollHeight;
-      this.insertApp(appData, true);
+      var app = this.insertApp(appData, true);
+      this.content_.scrollTop = app.tileCell.offsetTop;

[Dan Beam, 2012/11/30 23:41:45]

this might be a bit jerky... have you tried when this changes stuff?

[pedro (no code reviews), 2012/12/01 00:02:29]

What do you mean by "changes stuff"?

I've tested this and it does properly scroll to the proper place, revealing the
App that has just been added.

[Dan Beam, 2012/12/01 00:21:16]

sorry, "changes stuff" -> "an app is inserted".  if you've tried it and it
doesn't look so bad, that's fine, I just imagined it jerking to the new app's
spot rather than smoothly scrolling to it (desired, I'd think)

     },
 
     /**
-     * Similar to appendApp, but it respects the app_launch_ordinal field of
-     * |appData|.
+     * Inserts an App into the TilePage, preserving the alphabetical order.
      * @param {Object} appData The data that describes the app.
      * @param {boolean} animate Whether to animate the insertion.
      */
     insertApp: function(appData, animate) {
       var index = this.tiles_.length;
       for (var i = 0; i < this.tiles_.length; i++) {
-        if (appData.app_launch_ordinal <
-            this.tiles_[i].appData.app_launch_ordinal) {
+        if (appData.title.toLocaleLowerCase() <
+            this.tiles_[i].appData.title.toLocaleLowerCase()) {
           index = i;
           break;
         }
       }
 
-      this.addTileAt(new App(appData), index);
+      var app = new App(appData);
+      this.addTileAt(app, index);
       this.renderGrid_();
+
+      return app;
     },
 
     /**
@@ -684,14 +688,10 @@ cr.define('ntp', function() {
       }
     },
 
-    /**
-     * A handler for when the apps page is scrolled (then we need to reposition
-     * the bubbles.
-     * @private
-     */
-    onScroll_: function(e) {
-      if (!this.selected)
-        return;
+    /** @override */
+    onScroll: function() {
+      TilePage.prototype.onScroll.apply(this, arguments);
+
       for (var i = 0; i < this.tiles_.length; i++) {
         var app = this.tiles_[i];
         assert(app instanceof App);